On 1/24/21 2:57 AM, Justin Iurman wrote: >> De: "Jakub Kicinski" <k...@kernel.org> >> À: "Justin Iurman" <justin.iur...@uliege.be> >> Cc: netdev@vger.kernel.org, da...@davemloft.net, "alex aring" >> <alex.ar...@gmail.com> >> Envoyé: Dimanche 24 Janvier 2021 05:54:44 >> Objet: Re: [PATCH net 1/1] uapi: fix big endian definition of ipv6_rpl_sr_hdr > >> On Thu, 21 Jan 2021 23:00:44 +0100 Justin Iurman wrote: >>> Following RFC 6554 [1], the current order of fields is wrong for big >>> endian definition. Indeed, here is how the header looks like: >>> >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ >>> | Next Header | Hdr Ext Len | Routing Type | Segments Left | >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ >>> | CmprI | CmprE | Pad | Reserved | >>> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ >>> >>> This patch reorders fields so that big endian definition is now correct. >>> >>> [1] https://tools.ietf.org/html/rfc6554#section-3 >>> >>> Signed-off-by: Justin Iurman <justin.iur...@uliege.be> >> >> Are you sure? This looks right to me. > > AFAIK, yes. Did you mean the old (current) one looks right, or the new one? > If you meant the old/current one, well, I don't understand why the big endian > definition would look like this: > > #elif defined(__BIG_ENDIAN_BITFIELD) > __u32 reserved:20, > pad:4, > cmpri:4, > cmpre:4; > > When the RFC defines the header as follows: > > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | CmprI | CmprE | Pad | Reserved | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > The little endian definition looks fine. But, when it comes to big endian, > you define fields as you see them on the wire with the same order, right? So > the current big endian definition makes no sense. It looks like it was a > wrong mix with the little endian conversion. > >>> diff --git a/include/uapi/linux/rpl.h b/include/uapi/linux/rpl.h >>> index 1dccb55cf8c6..708adddf9f13 100644 >>> --- a/include/uapi/linux/rpl.h >>> +++ b/include/uapi/linux/rpl.h >>> @@ -28,10 +28,10 @@ struct ipv6_rpl_sr_hdr { >>> pad:4, >>> reserved1:16; >>> #elif defined(__BIG_ENDIAN_BITFIELD) >>> - __u32 reserved:20, >>> + __u32 cmpri:4, >>> + cmpre:4, >>> pad:4, >>> - cmpri:4, >>> - cmpre:4; >>> + reserved:20; >>> #else >>> #error "Please fix <asm/byteorder.h>" >>> #endif
cross-checking with other headers - tcp and vxlan-gpe - this patch looks correct.
