Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload

[Alex Elder]

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.

					-Alex