public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed

From: Al Viro <viro@ZenIV•linux.org.uk>
To: Jose Abreu <joabreu@synopsys•com>
Cc: "David S. Miller" <davem@davemloft•net>, netdev@vger•kernel.org
Subject: [stmmac][bug?] endianness of Flexible RX Parser code
Date: Sat, 4 Aug 2018 02:19:52 +0100	[thread overview]
Message-ID: <20180804011952.GD30522@ZenIV.linux.org.uk> (raw)

	The values passed in struct tc_u32_sel ->mask and ->val are
32bit net-endian.  Your tc_fill_entry() does this:

        data = sel->keys[0].val;
        mask = sel->keys[0].mask;

...
                entry->frag_ptr = frag;
                entry->val.match_en = (mask << (rem * 8)) &
                        GENMASK(31, rem * 8);
                entry->val.match_data = (data << (rem * 8)) &
                        GENMASK(31, rem * 8);
                entry->val.frame_offset = real_off;
                entry->prio = prio;

                frag->val.match_en = (mask >> (rem * 8)) &
                        GENMASK(rem * 8 - 1, 0);
                frag->val.match_data = (data >> (rem * 8)) &
                        GENMASK(rem * 8 - 1, 0);
                frag->val.frame_offset = real_off + 1;
                frag->prio = prio;
                frag->is_frag = true;

and that looks very odd.  rem here is offset modulo 4.  Suppose offset is
equal to 5, val contains {V0, V1, V2, V3} and mask - {M0, M1, M2, M3}.
Then on little-endian host we get
entry->val.match_en:	{0, M0, M1, M2}
entry->val.match_data:	{0, V0, V1, V2}
entry->val.frame_offset = 1;
frag->val.match_en:	{M3, 0, 0, 0}
frag->val.match_data:	{V3, 0, 0, 0}
frag->val.frame_offset = 2;
and on big-endian
entry->val.match_en:	{M1, M2, M3, 0}
entry->val.match_data:	{V1, V2, V3, 0}
entry->val.frame_offset = 1;
frag->val.match_en:	{0, 0, 0, M0}
frag->val.match_data:	{0, 0, 0, V0}
frag->val.frame_offset = 2;

Little-endian variant looks like we mask octets 5, 6, 7 and 8 with
M0..M3 resp. and want V0..V3 in those.  On big-endian, though, we
look at the octets 11, 4, 5 and 6 instead.

I don't know the hardware (and it might be pulling any kind of weird
endianness-dependent stunts), but that really smells like a bug.
It looks like that code is trying to do something like

        data = ntohl(sel->keys[0].val);
        mask = ntohl(sel->keys[0].mask);
	shift = rem * 8;

	entry->val.match_en = htonl(mask >> shift);
	entry->val.match_data = htonl(data >> shift);
	entry->val.frame_offset = real_off;
	...
	frag->val.match_en = htonl(mask << (32 - shift));
	frag->val.match_data = htonl(data << (32 - shift));
	entry->val.frame_offset = real_off + 1;

Comments?

next             reply	other threads:[~2018-08-04  3:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-04  1:19 Al Viro [this message]
2018-08-06  8:56 ` [stmmac][bug?] endianness of Flexible RX Parser code Jose Abreu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180804011952.GD30522@ZenIV.linux.org.uk \
    --to=viro@zeniv$(echo .)linux.org.uk \
    --cc=davem@davemloft$(echo .)net \
    --cc=joabreu@synopsys$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox