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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 15 Mar 2021 09:12:15 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Alex Elder <elder@...aro.org>
Cc:     Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>,
        stranche@...eaurora.org, David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, sharathv@...eaurora.org,
        bjorn.andersson@...aro.org, evgreen@...omium.org,
        cpratapa@...eaurora.org, David Laight <David.Laight@...lab.com>,
        Vladimir Oltean <olteanv@...il.com>, elder@...nel.org,
        Netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v4 0/6] net: qualcomm: rmnet: stop using C bit-fields

On Mon, Mar 15, 2021 at 6:36 AM Alex Elder <elder@...aro.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@...com>