netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] i40e: Fix VF VLAN offloading when port VLAN is configured
@ 2023-09-07 15:44 Ivan Vecera
  2023-09-07 18:52 ` Jesse Brandeburg
  0 siblings, 1 reply; 3+ messages in thread
From: Ivan Vecera @ 2023-09-07 15:44 UTC (permalink / raw)
  To: netdev
  Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Jeff Kirsher, Catherine Sullivan,
	Greg Rose, moderated list:INTEL ETHERNET DRIVERS, open list

If port VLAN is configured on a VF then any other VLANs on top of this VF
are broken.

During i40e_ndo_set_vf_port_vlan() call the i40e driver reset the VF and
iavf driver asks PF (using VIRTCHNL_OP_GET_VF_RESOURCES) for VF capabilities
but this reset occurs too early, prior setting of vf->info.pvid field
and because this field can be zero during i40e_vc_get_vf_resources_msg()
then VIRTCHNL_VF_OFFLOAD_VLAN capability is reported to iavf driver.

This is wrong because iavf driver should not report VLAN offloading
capability when port VLAN is configured as i40e does not support QinQ
offloading.

Fix the issue by moving VF reset after setting of vf->port_vlan_id
field.

Without this patch:
$ echo 1 > /sys/class/net/enp2s0f0/device/sriov_numvfs
$ ip link set enp2s0f0 vf 0 vlan 3
$ ip link set enp2s0f0v0 up
$ ip link add link enp2s0f0v0 name vlan4 type vlan id 4
$ ip link set vlan4 up
...
$ ethtool -k enp2s0f0v0 | grep vlan-offload
rx-vlan-offload: on
tx-vlan-offload: on
$ dmesg -l err | grep iavf
[1292500.742914] iavf 0000:02:02.0: Failed to add VLAN filter, error IAVF_ERR_INVALID_QP_ID

With this patch:
$ echo 1 > /sys/class/net/enp2s0f0/device/sriov_numvfs
$ ip link set enp2s0f0 vf 0 vlan 3
$ ip link set enp2s0f0v0 up
$ ip link add link enp2s0f0v0 name vlan4 type vlan id 4
$ ip link set vlan4 up
...
$ ethtool -k enp2s0f0v0 | grep vlan-offload
rx-vlan-offload: off [requested on]
tx-vlan-offload: off [requested on]
$ dmesg -l err | grep iavf

Fixes: f9b4b6278d51ff ("i40e: Reset the VF upon conflicting VLAN configuration")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 8ea1a238dcefe1..d3d6415553ed67 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -4475,9 +4475,7 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev, int vf_id,
 		goto error_pvid;
 
 	i40e_vlan_stripping_enable(vsi);
-	i40e_vc_reset_vf(vf, true);
-	/* During reset the VF got a new VSI, so refresh a pointer. */
-	vsi = pf->vsi[vf->lan_vsi_idx];
+
 	/* Locked once because multiple functions below iterate list */
 	spin_lock_bh(&vsi->mac_filter_hash_lock);
 
@@ -4563,6 +4561,10 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev, int vf_id,
 	 */
 	vf->port_vlan_id = le16_to_cpu(vsi->info.pvid);
 
+	i40e_vc_reset_vf(vf, true);
+	/* During reset the VF got a new VSI, so refresh a pointer. */
+	vsi = pf->vsi[vf->lan_vsi_idx];
+
 	ret = i40e_config_vf_promiscuous_mode(vf, vsi->id, allmulti, alluni);
 	if (ret) {
 		dev_err(&pf->pdev->dev, "Unable to config vf promiscuous mode\n");
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] i40e: Fix VF VLAN offloading when port VLAN is configured
  2023-09-07 15:44 [PATCH net] i40e: Fix VF VLAN offloading when port VLAN is configured Ivan Vecera
@ 2023-09-07 18:52 ` Jesse Brandeburg
  2023-09-15  8:42   ` [Intel-wired-lan] " Romanowski, Rafal
  0 siblings, 1 reply; 3+ messages in thread
From: Jesse Brandeburg @ 2023-09-07 18:52 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: Tony Nguyen, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Jeff Kirsher, Catherine Sullivan, Greg Rose,
	moderated list:INTEL ETHERNET DRIVERS, open list, Ahmed Zaki,
	Madhu Chittim

On 9/7/2023 8:44 AM, Ivan Vecera wrote:
> If port VLAN is configured on a VF then any other VLANs on top of this VF
> are broken.
> 
> During i40e_ndo_set_vf_port_vlan() call the i40e driver reset the VF and
> iavf driver asks PF (using VIRTCHNL_OP_GET_VF_RESOURCES) for VF capabilities
> but this reset occurs too early, prior setting of vf->info.pvid field
> and because this field can be zero during i40e_vc_get_vf_resources_msg()
> then VIRTCHNL_VF_OFFLOAD_VLAN capability is reported to iavf driver.
> 
> This is wrong because iavf driver should not report VLAN offloading
> capability when port VLAN is configured as i40e does not support QinQ
> offloading.
> 
> Fix the issue by moving VF reset after setting of vf->port_vlan_id
> field.
> 
> Without this patch:
> $ echo 1 > /sys/class/net/enp2s0f0/device/sriov_numvfs
> $ ip link set enp2s0f0 vf 0 vlan 3
> $ ip link set enp2s0f0v0 up
> $ ip link add link enp2s0f0v0 name vlan4 type vlan id 4
> $ ip link set vlan4 up
> ...
> $ ethtool -k enp2s0f0v0 | grep vlan-offload
> rx-vlan-offload: on
> tx-vlan-offload: on
> $ dmesg -l err | grep iavf
> [1292500.742914] iavf 0000:02:02.0: Failed to add VLAN filter, error IAVF_ERR_INVALID_QP_ID
> 
> With this patch:
> $ echo 1 > /sys/class/net/enp2s0f0/device/sriov_numvfs
> $ ip link set enp2s0f0 vf 0 vlan 3
> $ ip link set enp2s0f0v0 up
> $ ip link add link enp2s0f0v0 name vlan4 type vlan id 4
> $ ip link set vlan4 up
> ...
> $ ethtool -k enp2s0f0v0 | grep vlan-offload
> rx-vlan-offload: off [requested on]
> tx-vlan-offload: off [requested on]
> $ dmesg -l err | grep iavf
> 
> Fixes: f9b4b6278d51ff ("i40e: Reset the VF upon conflicting VLAN configuration")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Change looks reasonable to me and since it fixes your reproducer above,
then excellent! Thank you!

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [Intel-wired-lan] [PATCH net] i40e: Fix VF VLAN offloading when port VLAN is configured
  2023-09-07 18:52 ` Jesse Brandeburg
@ 2023-09-15  8:42   ` Romanowski, Rafal
  0 siblings, 0 replies; 3+ messages in thread
From: Romanowski, Rafal @ 2023-09-15  8:42 UTC (permalink / raw)
  To: Brandeburg, Jesse, ivecera, netdev@vger.kernel.org
  Cc: Catherine Sullivan, moderated list:INTEL ETHERNET DRIVERS,
	open list, Greg Rose, Eric Dumazet, Nguyen, Anthony L,
	Jeff Kirsher, Jakub Kicinski, Paolo Abeni, David S. Miller

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jesse Brandeburg
> Sent: Thursday, September 7, 2023 8:53 PM
> To: ivecera <ivecera@redhat.com>; netdev@vger.kernel.org
> Cc: Catherine Sullivan <catherine.sullivan@intel.com>; moderated list:INTEL
> ETHERNET DRIVERS <intel-wired-lan@lists.osuosl.org>; open list <linux-
> kernel@vger.kernel.org>; Greg Rose <gregory.v.rose@intel.com>; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Jeff Kirsher <jeffrey.t.kirsher@intel.com>;
> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David
> S. Miller <davem@davemloft.net>
> Subject: Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF VLAN offloading when
> port VLAN is configured
> 
> On 9/7/2023 8:44 AM, Ivan Vecera wrote:
> > If port VLAN is configured on a VF then any other VLANs on top of this
> > VF are broken.
> >
> > During i40e_ndo_set_vf_port_vlan() call the i40e driver reset the VF
> > and iavf driver asks PF (using VIRTCHNL_OP_GET_VF_RESOURCES) for VF
> > capabilities but this reset occurs too early, prior setting of
> > vf->info.pvid field and because this field can be zero during
> > i40e_vc_get_vf_resources_msg() then VIRTCHNL_VF_OFFLOAD_VLAN
> capability is reported to iavf driver.
> >
> > This is wrong because iavf driver should not report VLAN offloading
> > capability when port VLAN is configured as i40e does not support QinQ
> > offloading.
> >
> > Fix the issue by moving VF reset after setting of vf->port_vlan_id
> > field.
> >
> > Without this patch:
> > $ echo 1 > /sys/class/net/enp2s0f0/device/sriov_numvfs
> > $ ip link set enp2s0f0 vf 0 vlan 3
> > $ ip link set enp2s0f0v0 up
> > $ ip link add link enp2s0f0v0 name vlan4 type vlan id 4 $ ip link set
> > vlan4 up ...
> > $ ethtool -k enp2s0f0v0 | grep vlan-offload
> > rx-vlan-offload: on
> > tx-vlan-offload: on
> > $ dmesg -l err | grep iavf
> > [1292500.742914] iavf 0000:02:02.0: Failed to add VLAN filter, error
> > IAVF_ERR_INVALID_QP_ID
> >
> > With this patch:
> > $ echo 1 > /sys/class/net/enp2s0f0/device/sriov_numvfs
> > $ ip link set enp2s0f0 vf 0 vlan 3
> > $ ip link set enp2s0f0v0 up
> > $ ip link add link enp2s0f0v0 name vlan4 type vlan id 4 $ ip link set
> > vlan4 up ...
> > $ ethtool -k enp2s0f0v0 | grep vlan-offload
> > rx-vlan-offload: off [requested on]
> > tx-vlan-offload: off [requested on]
> > $ dmesg -l err | grep iavf
> >
> > Fixes: f9b4b6278d51ff ("i40e: Reset the VF upon conflicting VLAN
> > configuration")
> > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> 
> Change looks reasonable to me and since it fixes your reproducer above, then
> excellent! Thank you!
> 
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan


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



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-09-15  8:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-07 15:44 [PATCH net] i40e: Fix VF VLAN offloading when port VLAN is configured Ivan Vecera
2023-09-07 18:52 ` Jesse Brandeburg
2023-09-15  8:42   ` [Intel-wired-lan] " Romanowski, Rafal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).