On Thu, Sep 14, 2023 at 06:25:04PM +0200, Alexander Lobakin wrote:
> From: Larysa Zaremba <larysa.zaremba@xxxxxxxxx>
> 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@xxxxxxxxx>
> > ---
> > 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).
I will try to make it const.
> Line can be broken on arguments, not type (I hope).
>
This is how we break the lines everywhere in this file though :/
> > +{
> > + 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?
Would look ugly in this particular case, I think, too long expressions and no
return values.
>
> > +
> > 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?
A little bit too unrelated to the purpose of the series, but a thing we must do
in the future.
>
> >
> > /* pad the skb if needed, to make a valid ethernet frame */
> > if (eth_skb_pad(skb))
>
> [...]
>
> Thanks,
> Olek
