[xdp-hints] Re: [RFC bpf-next 10/23] ice: Implement VLAN tag hint

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Larysa Zaremba <larysa.zaremba@intel.com>
Date: Thu, 24 Aug 2023 21:26:49 +0200

> Implement .xmo_rx_vlan_tag callback to allow XDP code to read
> packet's VLAN tag.
> 
> At the same time, use vlan_tci instead of vlan_tag in touched code,
> because vlan_tag is misleading.
> 
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c     | 22 ++++++++++++++++
>  drivers/net/ethernet/intel/ice/ice_txrx.c     |  6 ++---
>  drivers/net/ethernet/intel/ice/ice_txrx.h     |  1 +
>  drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 26 +++++++++++++++++++
>  drivers/net/ethernet/intel/ice/ice_txrx_lib.h |  4 +--
>  drivers/net/ethernet/intel/ice/ice_xsk.c      |  6 ++---
>  6 files changed, 57 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 557c6326ff87..aff4fa1a75f8 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -6007,6 +6007,23 @@ ice_fix_features(struct net_device *netdev, netdev_features_t features)
>  	return features;
>  }
>  
> +/**
> + * ice_set_rx_rings_vlan_proto - update rings with new stripped VLAN proto
> + * @vsi: PF's VSI
> + * @vlan_ethertype: VLAN ethertype (802.1Q or 802.1ad) in network byte order
> + *
> + * Store current stripped VLAN proto in ring packet context,
> + * so it can be accessed more efficiently by packet processing code.
> + */
> +static void
> +ice_set_rx_rings_vlan_proto(struct ice_vsi *vsi, __be16 vlan_ethertype)

@vsi can be const (I hope).
Line can be broken on arguments, not type (I hope).

> +{
> +	u16 i;
> +
> +	ice_for_each_alloc_rxq(vsi, i)
> +		vsi->rx_rings[i]->pkt_ctx.vlan_proto = vlan_ethertype;
> +}
> +
>  /**
>   * ice_set_vlan_offload_features - set VLAN offload features for the PF VSI
>   * @vsi: PF's VSI
> @@ -6049,6 +6066,11 @@ ice_set_vlan_offload_features(struct ice_vsi *vsi, netdev_features_t features)
>  	if (strip_err || insert_err)
>  		return -EIO;
>  
> +	if (enable_stripping)
> +		ice_set_rx_rings_vlan_proto(vsi, htons(vlan_ethertype));
> +	else
> +		ice_set_rx_rings_vlan_proto(vsi, 0);

Ternary?

> +
>  	return 0;
>  }
>  
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
> index 4e6546d9cf85..4fd7614f243d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> @@ -1183,7 +1183,7 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
>  		struct sk_buff *skb;
>  		unsigned int size;
>  		u16 stat_err_bits;
> -		u16 vlan_tag = 0;
> +		u16 vlan_tci;
>  
>  		/* get the Rx desc from Rx ring based on 'next_to_clean' */
>  		rx_desc = ICE_RX_DESC(rx_ring, ntc);
> @@ -1278,7 +1278,7 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
>  			continue;
>  		}
>  
> -		vlan_tag = ice_get_vlan_tag_from_rx_desc(rx_desc);
> +		vlan_tci = ice_get_vlan_tci(rx_desc);

Unrelated: I never was a fan of scattering rx_desc parsing across
several files, I remember I moved it to process_skb_fields() in both ice
(Hints series) and iavf (libie), maybe do that here as well? Or way too
out of context?

>  
>  		/* pad the skb if needed, to make a valid ethernet frame */
>  		if (eth_skb_pad(skb))

[...]

Thanks,
Olek