Messages in this thread | | | Subject | Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload | From | Alex Elder <> | Date | Fri, 12 Feb 2021 13:06:56 -0600 |
| |
On 2/12/21 12:51 PM, Jakub Kicinski wrote: > On Fri, 12 Feb 2021 08:01:15 -0600 Alex Elder wrote: >> On 2/11/21 8:04 PM, Jakub Kicinski wrote: >>> On Fri, 12 Feb 2021 03:05:23 +0530 Sharath Chandra Vurukala wrote: >>>> +/* MAP CSUM headers */ >>>> +struct rmnet_map_v5_csum_header { >>>> + u8 next_hdr:1; >>>> + u8 header_type:7; >>>> + u8 hw_reserved:5; >>>> + u8 priority:1; >>>> + u8 hw_reserved_bit:1; >>>> + u8 csum_valid_required:1; >>>> + __be16 reserved; >>>> +} __aligned(1); >>> >>> Will this work on big endian? >> >> Sort of related to this point... >> >> I'm sure the response to this will be to add two versions >> of the definition, surrounded __LITTLE_ENDIAN_BITFIELD >> and __BIG_ENDIAN_BITFIELD tests. >> >> I really find this non-intuitive, and every time I >> look at it I have to think about it a bit to figure >> out where the bits actually lie in the word. >> >> I know this pattern is used elsewhere in the networking >> code, but that doesn't make it any easier for me to >> understand... >> >> Can we used mask, defined in host byte order, to >> specify the positions of these fields? >> >> I proposed a change at one time that did this and >> this *_ENDIAN_BITFIELD thing was used instead. >> >> I will gladly implement this change (completely >> separate from what's being done here), but thought >> it might be best to see what people think about it >> before doing that work. > > Most definitely agree, please convert.
KS, would you like me to do this to the existing code first?
I don't think it will take me very long. If it were a priority I could probably get it done by the end of today, but I'd want to ensure the result worked for the testing you do.
-Alex
| |