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

Alexander Lobakin
2023-09-14T16:26:28Z

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