All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <elder@ieee.org>
To: Jakub Kicinski <kuba@kernel.org>,
	Sharath Chandra Vurukala <sharathv@codeaurora.org>
Cc: davem@davemloft.net, elder@kernel.org, cpratapa@codeaurora.org,
	subashab@codeaurora.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload
Date: Fri, 12 Feb 2021 08:01:15 -0600	[thread overview]
Message-ID: <1c4e21bf-5903-bc45-6d6e-64b68e494542@ieee.org> (raw)
In-Reply-To: <20210211180459.500654b4@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

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

  reply	other threads:[~2021-02-12 14:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-11 21:35 [PATCH 0/3]net:qualcomm:rmnet:Enable Mapv5 Sharath Chandra Vurukala
2021-02-11 21:35 ` [PATCH 1/3] docs:networking: Add documentation for MAP v5 Sharath Chandra Vurukala
2021-02-11 21:35 ` [PATCH 2/3] net:ethernet:rmnet:Support for downlink MAPv5 csum offload Sharath Chandra Vurukala
2021-02-12  2:04   ` Jakub Kicinski
2021-02-12 14:01     ` Alex Elder [this message]
2021-02-12 17:49       ` subashab
2021-02-12 18:51       ` Jakub Kicinski
2021-02-12 19:06         ` Alex Elder
2021-02-12 20:11           ` subashab
2021-02-12  4:53   ` kernel test robot
2021-02-12  4:53     ` kernel test robot
2021-02-11 21:35 ` [PATCH 3/3] net:ethernet:rmnet: Add support for Mapv5 Uplink packet Sharath Chandra Vurukala
2021-02-12  2:25   ` kernel test robot
2021-02-12  2:25     ` kernel test robot

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=1c4e21bf-5903-bc45-6d6e-64b68e494542@ieee.org \
    --to=elder@ieee.org \
    --cc=cpratapa@codeaurora.org \
    --cc=davem@davemloft.net \
    --cc=elder@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sharathv@codeaurora.org \
    --cc=subashab@codeaurora.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.