Re: [PATCH net-next v4 0/6] net: qualcomm: rmnet: stop using C bit-fields

From: Alexander Duyck <alexander.duyck@gmail.com>
To: Alex Elder <elder@linaro.org>
Cc: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>,
	stranche@codeaurora.org, David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	sharathv@codeaurora.org, bjorn.andersson@linaro.org,
	evgreen@chromium.org, cpratapa@codeaurora.org,
	David Laight <David.Laight@aculab.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	elder@kernel.org, Netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next v4 0/6] net: qualcomm: rmnet: stop using C bit-fields
Date: Mon, 15 Mar 2021 09:12:15 -0700	[thread overview]
Message-ID: <CAKgT0UdUuDBRO38h4a0V-0MJg7ONwYTr-nzDgPsEKtmihseDEQ@mail.gmail.com> (raw)
In-Reply-To: <20210315133455.1576188-1-elder@linaro.org>

On Mon, Mar 15, 2021 at 6:36 AM Alex Elder <elder@linaro.org> wrote:
>
> The main reason for version 4 of this series is that a bug was
> introduced in version 3, and that is fixed.
>
> But a nice note from Vladimir Oltean got me thinking about the
> necessity of using accessors defined in <linux/bitfield.h>, and I
> concluded there was no need.  So this version simplifies things
> further, using bitwise AND and OR operators (rather than, e.g.,
> u8_get_bits()) to access all values encoded in bit fields.
>
> This version has been tested using IPv4 with checksum offload
> enabled and disabled.  Traffic over the link included ICMP (ping),
> UDP (iperf), and TCP (wget).
>
> Version 3 of this series used BIT() rather than GENMASK() to define
> single-bit masks, and bitwise AND operators to access them.
>
> Version 2 fixed bugs in the way the value written into the header
> was computed in version 1.
>
> The series was first posted here:
>   https://lore.kernel.org/netdev/20210304223431.15045-1-elder@linaro.org/
>
>                                         -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 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  | 10 +--
>  .../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, 69 insertions(+), 89 deletions(-)
>

Other than the minor nit I pointed out in patch 2 the set looks good to me.

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>