diff mbox series

[RFC,bpf-next,10/23] ice: Implement VLAN tag hint

Message ID 20230824192703.712881-11-larysa.zaremba@intel.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series XDP metadata via kfuncs for ice + mlx5 | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-0 success Logs for ShellCheck
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-1 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 pending Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-11 pending Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 pending Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-28 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-10 fail Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 fail Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-14 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 fail Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 fail Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on s390x with gcc
netdev/series_format fail Series longer than 15 patches (and no cover letter)
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1330 this patch: 1330
netdev/cc_maintainers warning 7 maintainers not CCed: hawk@kernel.org jesse.brandeburg@intel.com intel-wired-lan@lists.osuosl.org davem@davemloft.net anthony.l.nguyen@intel.com pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1353 this patch: 1353
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 134 lines checked
netdev/kdoc success Errors and warnings before: 1 this patch: 1
netdev/source_inline success Was 0 now: 0

Commit Message

Larysa Zaremba Aug. 24, 2023, 7:26 p.m. UTC
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(-)

Comments

Fijalkowski, Maciej Sept. 4, 2023, 4 p.m. UTC | #1
On Thu, Aug 24, 2023 at 09:26:49PM +0200, Larysa Zaremba wrote:
> 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.

misleading...because? ;)

> 
> 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(-)
>
Larysa Zaremba Sept. 4, 2023, 6:18 p.m. UTC | #2
On Mon, Sep 04, 2023 at 06:00:34PM +0200, Maciej Fijalkowski wrote:
> On Thu, Aug 24, 2023 at 09:26:49PM +0200, Larysa Zaremba wrote:
> > 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.
> 
> misleading...because? ;)
>

VLAN tag ofter refers to VLAN proto and VLAN TCI combined, while in the 
corrected code we clearly store only VLAN TCI.

Will add the above to the commit message.

> > 
> > 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(-)
> >
Alexander Lobakin Sept. 14, 2023, 4:25 p.m. UTC | #3
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
Larysa Zaremba Sept. 14, 2023, 4:28 p.m. UTC | #4
On Thu, Sep 14, 2023 at 06:25:04PM +0200, Alexander Lobakin wrote:
> 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).

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
Alexander Lobakin Sept. 14, 2023, 4:38 p.m. UTC | #5
From: Larysa Zaremba <larysa.zaremba@intel.com>
Date: Thu, 14 Sep 2023 18:28:07 +0200

> On Thu, Sep 14, 2023 at 06:25:04PM +0200, Alexander Lobakin wrote:
>> From: Larysa Zaremba <larysa.zaremba@intel.com>
>> Date: Thu, 24 Aug 2023 21:26:49 +0200

[...]

>>> +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 :/

I know and would really like us stop at least adding new such
occurrences when not needed :s

> 
>>> +{
>>> +	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.

	ice_set_rx_rings_vlan_proto(vsi, strip ? htons(vlan_ethertype) : 0);

?

[...]

>>> -		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.

Sure, +

> 
>>
>>>  
>>>  		/* pad the skb if needed, to make a valid ethernet frame */
>>>  		if (eth_skb_pad(skb))
>>
>> [...]
>>
>> Thanks,
>> Olek

Thanks,
Olek
Larysa Zaremba Sept. 14, 2023, 5:02 p.m. UTC | #6
On Thu, Sep 14, 2023 at 06:38:04PM +0200, Alexander Lobakin wrote:
> From: Larysa Zaremba <larysa.zaremba@intel.com>
> Date: Thu, 14 Sep 2023 18:28:07 +0200
> 
> > On Thu, Sep 14, 2023 at 06:25:04PM +0200, Alexander Lobakin wrote:
> >> From: Larysa Zaremba <larysa.zaremba@intel.com>
> >> Date: Thu, 24 Aug 2023 21:26:49 +0200
> 
> [...]
> 
> >>> +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 :/
> 
> I know and would really like us stop at least adding new such
> occurrences when not needed :s

I think with such minor stuff, it is more important to keep style consistent in 
files.

> 
> > 
> >>> +{
> >>> +	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.
> 
> 	ice_set_rx_rings_vlan_proto(vsi, strip ? htons(vlan_ethertype) : 0);
> 
> ?
> 
> [...]
> 
> >>> -		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.
> 
> Sure, +
> 
> > 
> >>
> >>>  
> >>>  		/* pad the skb if needed, to make a valid ethernet frame */
> >>>  		if (eth_skb_pad(skb))
> >>
> >> [...]
> >>
> >> Thanks,
> >> Olek
> 
> Thanks,
> Olek
Larysa Zaremba Sept. 18, 2023, 2:07 p.m. UTC | #7
On Thu, Sep 14, 2023 at 06:38:04PM +0200, Alexander Lobakin wrote:
> From: Larysa Zaremba <larysa.zaremba@intel.com>
> Date: Thu, 14 Sep 2023 18:28:07 +0200
> 
> > On Thu, Sep 14, 2023 at 06:25:04PM +0200, Alexander Lobakin wrote:
> >> From: Larysa Zaremba <larysa.zaremba@intel.com>
> >> Date: Thu, 24 Aug 2023 21:26:49 +0200
> 
> [...]
> 
> >>> +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 :/
> 
> I know and would really like us stop at least adding new such
> occurrences when not needed :s
> 
> > 
> >>> +{
> >>> +	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.
> 
> 	ice_set_rx_rings_vlan_proto(vsi, strip ? htons(vlan_ethertype) : 0);
> 
> ?

Have missed this one the first time, sorry, makes sense this way :D

> 
> [...]
> 
> >>> -		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.
> 
> Sure, +
> 
> > 
> >>
> >>>  
> >>>  		/* pad the skb if needed, to make a valid ethernet frame */
> >>>  		if (eth_skb_pad(skb))
> >>
> >> [...]
> >>
> >> Thanks,
> >> Olek
> 
> Thanks,
> Olek
diff mbox series

Patch

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)
+{
+	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);
+
 	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);
 
 		/* pad the skb if needed, to make a valid ethernet frame */
 		if (eth_skb_pad(skb))
@@ -1292,7 +1292,7 @@  int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int budget)
 
 		ice_trace(clean_rx_irq_indicate, rx_ring, rx_desc, skb);
 		/* send completed skb up the stack */
-		ice_receive_skb(rx_ring, skb, vlan_tag);
+		ice_receive_skb(rx_ring, skb, vlan_tci);
 
 		/* update budget accounting */
 		total_rx_pkts++;
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
index 4237702a58a9..41e0b14e6643 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -260,6 +260,7 @@  enum ice_rx_dtype {
 struct ice_pkt_ctx {
 	const union ice_32b_rx_flex_desc *eop_desc;
 	u64 cached_phctime;
+	__be16 vlan_proto;
 };
 
 struct ice_xdp_buff {
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
index b11cfaedb81c..10e7ec51f4ef 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
@@ -639,7 +639,33 @@  static int ice_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash,
 	return 0;
 }
 
+/**
+ * ice_xdp_rx_vlan_tag - VLAN tag XDP hint handler
+ * @ctx: XDP buff pointer
+ * @vlan_tci: destination address for VLAN tag
+ * @vlan_proto: destination address for VLAN protocol
+ *
+ * Copy VLAN tag (if was stripped) and corresponding protocol
+ * to the destination address.
+ */
+static int ice_xdp_rx_vlan_tag(const struct xdp_md *ctx, u16 *vlan_tci,
+			       __be16 *vlan_proto)
+{
+	const struct ice_xdp_buff *xdp_ext = (void *)ctx;
+
+	*vlan_proto = xdp_ext->pkt_ctx.vlan_proto;
+	if (!*vlan_proto)
+		return -ENODATA;
+
+	*vlan_tci = ice_get_vlan_tci(xdp_ext->pkt_ctx.eop_desc);
+	if (!*vlan_tci)
+		return -ENODATA;
+
+	return 0;
+}
+
 const struct xdp_metadata_ops ice_xdp_md_ops = {
 	.xmo_rx_timestamp		= ice_xdp_rx_hw_ts,
 	.xmo_rx_hash			= ice_xdp_rx_hash,
+	.xmo_rx_vlan_tag		= ice_xdp_rx_vlan_tag,
 };
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.h b/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
index 145883eec129..b7205826fea8 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
@@ -84,7 +84,7 @@  ice_build_ctob(u64 td_cmd, u64 td_offset, unsigned int size, u64 td_tag)
 }
 
 /**
- * ice_get_vlan_tag_from_rx_desc - get VLAN from Rx flex descriptor
+ * ice_get_vlan_tci - get VLAN TCI from Rx flex descriptor
  * @rx_desc: Rx 32b flex descriptor with RXDID=2
  *
  * The OS and current PF implementation only support stripping a single VLAN tag
@@ -92,7 +92,7 @@  ice_build_ctob(u64 td_cmd, u64 td_offset, unsigned int size, u64 td_tag)
  * one is found return the tag, else return 0 to mean no VLAN tag was found.
  */
 static inline u16
-ice_get_vlan_tag_from_rx_desc(union ice_32b_rx_flex_desc *rx_desc)
+ice_get_vlan_tci(const union ice_32b_rx_flex_desc *rx_desc)
 {
 	u16 stat_err_bits;
 
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index fdeddad9b639..eeb02f76b4a6 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -878,7 +878,7 @@  int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget)
 		struct xdp_buff *xdp;
 		struct sk_buff *skb;
 		u16 stat_err_bits;
-		u16 vlan_tag = 0;
+		u16 vlan_tci;
 
 		rx_desc = ICE_RX_DESC(rx_ring, ntc);
 
@@ -957,10 +957,10 @@  int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget)
 		total_rx_bytes += skb->len;
 		total_rx_packets++;
 
-		vlan_tag = ice_get_vlan_tag_from_rx_desc(rx_desc);
+		vlan_tci = ice_get_vlan_tci(rx_desc);
 
 		ice_process_skb_fields(rx_ring, rx_desc, skb);
-		ice_receive_skb(rx_ring, skb, vlan_tag);
+		ice_receive_skb(rx_ring, skb, vlan_tci);
 	}
 
 	rx_ring->next_to_clean = ntc;