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;
Sender: subashab=codeaurora.org@mg.codeaurora.org
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Thu, 04 Mar 2021 20:44:54 -0700
From:   subashab@codeaurora.org
To:     Alex Elder <elder@linaro.org>
Cc:     stranche@codeaurora.org, davem@davemloft.net, kuba@kernel.org,
        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
Subject: Re: [PATCH net-next 0/6] net: qualcomm: rmnet: stop using C
 bit-fields
In-Reply-To: <20210304223431.15045-1-elder@linaro.org>
References: <20210304223431.15045-1-elder@linaro.org>
Message-ID: <3a4a4c26494c12f9961c50e2d4b83c99@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Precedence: bulk

On 2021-03-04 15:34, 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.
> 
> 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(-)

Can you share what all tests have been done with these patches