[Intel-wired-lan] [PATCH iwl-next 2/2] ice: store max_frame and rx_buf_len only in ice_rx_ring

[Romanowski, Rafal]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces at osuosl.org> On Behalf Of Jacob
> Keller
> Sent: Tuesday, September 10, 2024 1:08 AM
> To: Intel Wired LAN <intel-wired-lan at lists.osuosl.org>; Nguyen, Anthony L
> <anthony.l.nguyen at intel.com>
> Cc: Keller, Jacob E <jacob.e.keller at intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel at intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 2/2] ice: store max_frame and
> rx_buf_len only in ice_rx_ring
> 
> The max_frame and rx_buf_len fields of the VSI set the maximum frame size for
> packets on the wire, and configure the size of the Rx buffer. In the hardware,
> these are per-queue configuration. Most VSI types use a simple method to
> determine the size of the buffers for all queues.
> 
> However, VFs may potentially configure different values for each queue.
> While the Linux iAVF driver does not do this, it is allowed by the virtchnl interface.
> 
> The current virtchnl code simply sets the per-VSI fields inbetween calls to
> ice_vsi_cfg_single_rxq(). This technically works, as these fields are only ever used
> when programming the Rx ring, and otherwise not checked again.
> However, it is confusing to maintain.
> 
> The Rx ring also already has an rx_buf_len field in order to access the buffer
> length in the hotpath. It also has extra unused bytes in the ring structure which
> we can make use of to store the maximum frame size.
> 
> Drop the VSI max_frame and rx_buf_len fields. Add max_frame to the Rx ring,
> and slightly re-order rx_buf_len to better fit into the gaps in the structure layout.
> 
> Change the ice_vsi_cfg_frame_size function so that it writes to the ring fields.
> Call this function once per ring in ice_vsi_cfg_rxqs(). This is done over calling it
> inside the ice_vsi_cfg_rxq(), because
> ice_vsi_cfg_rxq() is called in the virtchnl flow where the max_frame and
> rx_buf_len have already been configured.
> 
> Change the accesses for rx_buf_len and max_frame to all point to the ring
> structure. This has the added benefit that ice_vsi_cfg_rxq() no longer has the
> surprise side effect of updating ring->rx_buf_len based on the VSI field.
> 
> Update the virtchnl ice_vc_cfg_qs_msg() function to set the ring values directly,
> and drop references to the removed VSI fields.
> 
> This now makes the VF logic clear, as the ring fields are obviously per-queue. This
> reduces the required cognitive load when reasoning about this logic.
> 
> Note that removing the VSI fields does leave a 4 byte gap, but the ice_vsi
> structure has many gaps, and its layout is not as critical in the hot path.
> The structure may benefit from a more thorough repacking, but no attempt was
> made in this change.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice.h          |  3 ---
>  drivers/net/ethernet/intel/ice/ice_txrx.h     |  3 ++-
>  drivers/net/ethernet/intel/ice/ice_base.c     | 34 ++++++++++++++-------------
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c |  7 +++---
>  4 files changed, 23 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index ce8b5505b16d..7f606f9277f2 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -371,9 +371,6 @@ struct ice_vsi {


Tested-by: Rafal Romanowski <rafal.romanowski at intel.com>