[Intel-wired-lan] [PATCH iwl-next v2 00/13] ice: use <linux/packing.h> for Tx and Rx queue context data

Jacob Keller jacob.e.keller at intel.com
Wed Aug 28 20:57:16 UTC 2024


This series refactors the Tx and Rx queue context data packing in the ice
driver to use the <linux/packing.h> API. This is intended to aid in the
future implementation of unpacking context data for live migration. While
it strictly does save a few bytes in the module according to bloat-o-meter,
the savings is quite minimal (~200 bytes).

While initially implementing the logic, it was discovered that the packing
code does not work properly if the provided buffer size is not a multiple
of 4 bytes. This is also fixed by this series, along with some general
improvements to the API including the addition of pack() and unpack().

First, several improvements and cleanups to the packing API are made:

An additional sanity check is added to packing() to ensure that the
bit indices do not point outside the bounds of the provided buffer.

The packing() implementation is adjusted to work correctly when given
an arbitrary buffer size.

The duplicate kernel-doc for packing() is removed from the header
file.

The packing API gaves the pack() and unpack() wrappers which simplify
the use of packing, and enable better const correctness checking.

The pack() and unpack() implementations are separated and packing()
becomes a simple wrapper that selects between the two operations. This is
intended to improve the logic as several conditionals can be dropped.

A suite of KUnit tests for the packing API have been added, based on
simple selftests originally developed by Vladimir.

Additional unit tests for the packing library are added.

A bug in the QUIRK_MSB_ON_THE_RIGHT of the packing API is fixed, along with
additional unit tests covering this behavior.

Following the improvements to the packing API, the ice driver is
refactored:

The int_q_state field is removed from the ice_tlan_ctx structure and
packing definitions. This field is not used, and can't even be properly
packed or unpacked as its size exceeds 8 bytes.

The existing packing code based on ice_set_ctx() is ripped out and replaced
with an implementation based on pack().

The sizes of fields in ice_tlan_ctx and ice_rlan_ctx are updated to better
reflect their actual size.

The setting of the prefetch enable bit in the Rx context is moved into
ice_setup_rx_ctx() where the rest of the Rx context is configured. This is
less surprising than having the ice_write_rxq_ctx() always overwrite that
field of the context.

The copy_rxq_ctx_to_hw() and ice_write_rxq_ctx() are updated to align with
modern kernel style guidelines. This includes removing an unnecessary NULL
check, updating the kdoc descriptions to align with the check-kdoc, and
making ice_copy_rxq_ctx_to_hw() void. This ensures the style for these
functions will match the style for the similar functions needed to enable
live migration in a future series.

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel at intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
---
Changes in v2:
- Fix netdev address
- Drop unnecessary fixes tag in prefetch enable patch
- Add a module description to the KUnit tests
- Mark the test cases array in packing_test.c static
- Prefer 'Return:' over 'Returns:' for kdoc
- Link to v1: https://lore.kernel.org/r/20240827-e810-live-migration-jk-prep-ctx-functions-v1-0-0442e2e42d32@intel.com

I opted to include the packing changes in this series to Intel Wired LAN,
because the changes are only required due to the new use with the ice
driver. I also do not believe any existing driver uses
QUIRK_MSB_ON_THE_RIGHT, so I don't think the fix for that quirk needs to go
through net or stable either.

The packing library changes were discussed here:
https://lore.kernel.org/netdev/a0338310-e66c-497c-bc1f-a597e50aa3ff@intel.com/

In addition, several of the changes are taken from Vladimir's github:
https://github.com/vladimiroltean/linux/tree/packing-selftests

Strictly, the changes for pack() and unpack() are not required by ice, but
Vladimir requested that we try to use the new API so that I am not adding
another driver which needs updating in the future. In addition, pack() and
unpack() enable const checking.

---
Jacob Keller (8):
      lib: packing: add KUnit tests adapted from selftests
      lib: packing: add additional KUnit tests
      lib: packing: fix QUIRK_MSB_ON_THE_RIGHT behavior
      ice: remove int_q_state from ice_tlan_ctx
      ice: use <linux/packing.h> for Tx and Rx queue context data
      ice: reduce size of queue context fields
      ice: move prefetch enable to ice_setup_rx_ctx
      ice: cleanup Rx queue context programming functions

Vladimir Oltean (5):
      lib: packing: refuse operating on bit indices which exceed size of buffer
      lib: packing: adjust definitions and implementation for arbitrary buffer lengths
      lib: packing: remove kernel-doc from header file
      lib: packing: add pack() and unpack() wrappers over packing()
      lib: packing: duplicate pack() and unpack() implementations

 drivers/net/ethernet/intel/ice/ice_common.h    |  13 +-
 drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h |  33 +-
 include/linux/packing.h                        |  32 +-
 drivers/net/ethernet/intel/ice/ice_base.c      |   6 +-
 drivers/net/ethernet/intel/ice/ice_common.c    | 333 ++++++--------------
 lib/packing.c                                  | 304 ++++++++++++------
 lib/packing_test.c                             | 412 +++++++++++++++++++++++++
 Documentation/core-api/packing.rst             |  71 +++++
 MAINTAINERS                                    |   1 +
 lib/Kconfig                                    |  12 +
 lib/Makefile                                   |   1 +
 11 files changed, 840 insertions(+), 378 deletions(-)
---
base-commit: 2c163922de69983e6ccedeb5c00dec85b6a17283
change-id: 20240812-e810-live-migration-jk-prep-ctx-functions-d46db279b2c6

Best regards,
-- 
Jacob Keller <jacob.e.keller at intel.com>



More information about the Intel-wired-lan mailing list