lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 23 Dec 2020 20:22:01 +0100
From:   Horatiu Vultur <horatiu.vultur@...rochip.com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Rasmus Villemoes <rasmus.villemoes@...vas.dk>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net 1/2] net: mrp: fix definitions of MRP test packets

Hi Andrew,

The 12/23/2020 19:41, Andrew Lunn wrote:
> 
> > > @@ -96,7 +96,7 @@ struct br_mrp_ring_test_hdr {
> > >         __be16 state;
> > >         __be16 transitions;
> > >         __be32 timestamp;
> > > -};
> > > +} __attribute__((__packed__));
> >
> > Yes, I agree that this should be packed but it also needs to be 32 bit
> > alligned, so extra 2 bytes are needed.
> 
> The full structure is:
> 
> struct br_mrp_ring_test_hdr {
>         __be16 prio;
>         __u8 sa[ETH_ALEN];
>         __be16 port_role;
>         __be16 state;
>         __be16 transitions;
>         __be32 timestamp;
> };
> 
> If i'm looking at this correctly, the byte offsets are:
> 
> 0-1   prio
> 2-7   sa
> 8-9   port_role
> 10-11 state
> 12-13 transition
> 
> With packed you get
> 
> 14-17 timestamp
> 
> which is not 32 bit aligned.
> 
> Do you mean the whole structure must be 32 bit aligned? We need to add
> two reserved bytes to the end of the structure?

Sorry, I looked too fast over this. First some info, in front of the
'br_mrp_ring_test_hdr', there is 'br_mrp_tlv_hdr' which is 2
bytes. So the frame will look something like this:

... |---------|----------------|----------------------|------------| ....
... | MRP Ver | br_mrp_tlv_hdr | br_mrp_ring_test_hdr | Common TLV | ....
... |---------|----------------|----------------------|------------| ....

The standard says that for br_mrp_tlv_hdr + br_mrp_ring_test, 32 bit
alignment shall be ensured. So my understanding was that it needs to be
at word boundary(4 bytes).

So based on this, if we use packed then we get the following offsets
0	type
1	length
2-3	prio
4-9	sa
10-11	port_role
12-13	state
14-15	transition
16-19	timestamp.

Which is 20 bytes, that fits correctly. So my understanding is we need to
use packed, to remove the hole between transition and timestamp as
Rasmus suggested and should NOT use these 2 extra bytes that I
mentioned because it would not be aligned anymore.

Here you can find few more details about MRP[1]

> 
>     Andrew

[1] https://www.youtube.com/watch?v=R3vQYfwiJ2M

-- 
/Horatiu

Powered by blists - more mailing lists