Version 2 of this series fixes the code in the final patch that
encoded the RMNet checksum offload header inserted for outgoing
packets. It was erroneously using be16_encode_bits() to generate
the value to be written into the header, where it should have been
using u16_encode_bits(). Bjorn noticed this, as did the Intel
kernel test robot.
This version of these patches has been tested as follows:
- ICMP (ping) and TCP (wget), and UDP (iperf) packet transfer in
both directions over an RMNet link (using IPA, with IPv4 only).
This used QMAPv4, with checksum offload in both directions
enabled (and aggregation enabled for inbound data). Inbound
checksum values were verified good (for TCP and UDP). I
presume the TX checksum was acceptable but did nothing special
to confirm that. Checksum verification was enabled with:
ethtool -K rmnet_data0 tx on rx on
- Format of the new and old structures were compared, bit by bit,
after assigning various values to the fields using both the
old and new structure definitions (and access methods).
- The patches were all run through sparse. No new errors are
reported. (A "no newline at end of file" warning is reported
for "rmnet_vnd.c"; and ip_fast_csum() defined in ARM64
"checksum.h" produces a warning.)
I've added Reviewed-by from Bjorn to the first five patches, and
Reported-by from the kernel test robot on the last one
Below is description that was sent with version 1.
-Alex
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-el...@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-shara...@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(-)
--
2.27.0
