Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH net-next 0/6] net: qualcomm: rmnet: stop using C
 bit-fields
From:   Alex Elder <elder@linaro.org>
To:     subashab@codeaurora.org, stranche@codeaurora.org,
        davem@davemloft.net, kuba@kernel.org
Cc:     sharathv@codeaurora.org, bjorn.andersson@linaro.org,
        evgreen@chromium.org, cpratapa@codeaurora.org, elder@kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210304223431.15045-1-elder@linaro.org>
Message-ID: <cdfe3730-97e1-acb3-fa5e-7e016523eab8@linaro.org>
Date:   Thu, 4 Mar 2021 16:43:33 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210304223431.15045-1-elder@linaro.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/4/21 4:34 PM, Alex Elder wrote:
> This series converts data structures defined in <linux/if_rmnet.h>
> so they use integral field values with bitfield masks rather than
> rely on C bit-fields.

Whoops!  I forgot to check if net-next was open.  I'm very
sorry about that...

   http://vger.kernel.org/~davem/net-next.html

					-Alex

> I first proposed doing something like this long ago when my confusion
> about this code (and the memory layout it was supposed to represent)
> led me to believe it was erroneous:
>    https://lore.kernel.org/netdev/20190520135354.18628-1-elder@linaro.org/
> 
> It came up again recently, when Sharath Chandra Vurukala proposed
> a new structure in "if_rmnet.h", again using C bit-fields.  I asked
> whether the new structure could use field masks, and Jakub requested
> that this be done.
>    https://lore.kernel.org/netdev/1613079324-20166-1-git-send-email-sharathv@codeaurora.org/
> I volunteered to convert the existing RMNet code to use bitfield
> masks, and that is what I'm doing here.
> 
> The first three patches are more or less preparation work for the
> last three.
>    - The first marks two fields in an existing structure explicitly
>      big endian.  They are unused by current code, so this should
>      have no impact.
>    - The second simplifies some code that computes the value of a
>      field in a header in a somewhat obfuscated way.
>    - The third eliminates some trivial accessor macros, open-coding
>      them instead.  I believe the accessors actually do more harm
>      than good.
>    - The last three convert the structures defined in "if_rmnet.h"
>      so they are defined only with integral fields, each having
>      well-defined byte order.  Where sub-fields are needed, field
>      masks are defined so they can be encoded or extracted using
>      functions like be16_get_bits() or u8_encode_bits(), defined
>      in <linux/bitfield.h>.  The three structures converted are,
>      in order:  rmnet_map_header, rmnet_map_dl_csum_trailer, and
>      rmnet_map_ul_csum_header.
> 
> 					-Alex
> 
> Alex Elder (6):
>    net: qualcomm: rmnet: mark trailer field endianness
>    net: qualcomm: rmnet: simplify some byte order logic
>    net: qualcomm: rmnet: kill RMNET_MAP_GET_*() accessor macros
>    net: qualcomm: rmnet: use field masks instead of C bit-fields
>    net: qualcomm: rmnet: don't use C bit-fields in rmnet checksum trailer
>    net: qualcomm: rmnet: don't use C bit-fields in rmnet checksum header
> 
>   .../ethernet/qualcomm/rmnet/rmnet_handlers.c  | 11 ++--
>   .../net/ethernet/qualcomm/rmnet/rmnet_map.h   | 12 ----
>   .../qualcomm/rmnet/rmnet_map_command.c        | 11 +++-
>   .../ethernet/qualcomm/rmnet/rmnet_map_data.c  | 60 ++++++++---------
>   include/linux/if_rmnet.h                      | 65 +++++++++----------
>   5 files changed, 70 insertions(+), 89 deletions(-)
>