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

From: subashab
Date: Thu Mar 04 2021 - 22:45:08 EST


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@xxxxxxxxxx/

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