| Message ID | 20230907154457.3861711-1-ivecera@redhat.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] i40e: Fix VF VLAN offloading when port VLAN is configured | expand |
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>
> -----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>
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");
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(-)