Message ID | 20171201190024.5547-3-jacob.e.keller@intel.com |
---|---|
State | Superseded |
Headers | show |
Series | None | expand |
> -----Original Message----- > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf > Of Jacob Keller > Sent: Friday, December 1, 2017 11:00 AM > To: Intel Wired LAN <intel-wired-lan@lists.osuosl.org> > Cc: Kwan, Ngai-mint <ngai-mint.kwan@intel.com> > Subject: [Intel-wired-lan] [PATCH 2/6] fm10k: fix "failed to kill vid" message for > VF > > From: Ngai-Mint Kwan <ngai-mint.kwan@intel.com> > > When a VF is under PF VLAN assignment: > > ip link set <pf> vf <#> vlan <vid> > > This will remove all previous entries in the VLAN table including those > generated by VLAN interfaces created on the VF. The issue arises when > the VF is under PF VLAN assignment and one or more of these VLAN > interfaces of the VF are deleted. When deleting these VLAN interfaces, > the following message will be generated in "dmesg": > > failed to kill vid 0081/<vid> for device <vf> > > This is due to the fact that "ndo_vlan_rx_kill_vid" exits with an error. > The handler for this ndo is "fm10k_update_vid". Any calls to this > function while under PF VLAN management will exit prematurely and, thus, > it will generate the failure message. > > Additionally, since "fm10k_update_vid" exits prematurely, none of the > VLAN update is performed. So, even though the actual VLAN interfaces of > the VF will be deleted, the active_vlans bitmask is not cleared. When > the VF is no longer under PF VLAN assignment, the driver mistakenly > restores the previous entries of the VLAN table based on an > unsynchronized list of active VLANs. > > The solution to this issue involves checking the VLAN update action type > before exiting "fm10k_update_vid". If the VLAN update action type is to > "add", this action will not be permitted while the VF is under PF VLAN > assignment and the VLAN update is abandoned like before. > > However, if the VLAN update action type is to "kill", then we need to > also clear the active_vlans bitmask. However, we don't need to actually > queue any messages to the PF, because the MAC and VLAN tables have > already been cleared, and the PF would silently ignore these requests > anyways. > > Signed-off-by: Ngai-Mint Kwan <ngai-mint.kwan@intel.com> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> > --- > drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c > b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c > index 4b5272f13d40..f8e2d1804a17 100644 > --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c > +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c > @@ -934,8 +934,12 @@ static int fm10k_update_vid(struct net_device > *netdev, u16 vid, bool set) > if (vid >= VLAN_N_VID) > return -EINVAL; > > - /* Verify we have permission to add VLANs */ > - if (hw->mac.vlan_override) > + /* Verify that we have permission to add VLANs. If this is a request > + * to remove a VLAN, we still want to allow the user to remove the > + * VLAN device. In that case, we need to clear the bit in the > + * active_vlans bitmask. > + */ > + if (set && hw->mac.vlan_override) > return -EACCES; > > /* update active_vlans bitmask */ > @@ -954,6 +958,12 @@ static int fm10k_update_vid(struct net_device > *netdev, u16 vid, bool set) > rx_ring->vid &= ~FM10K_VLAN_CLEAR; > } > > + /* If our VLAN has been overridden, there is no reason to send VLAN > + * removal requests as they will be silently ignored. > + */ > + if (hw->mac.vlan_override) > + return 0; > + > /* Do not remove default VLAN ID related entries from VLAN and MAC > * tables > */ > -- > 2.15.0 > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c index 4b5272f13d40..f8e2d1804a17 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c @@ -934,8 +934,12 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set) if (vid >= VLAN_N_VID) return -EINVAL; - /* Verify we have permission to add VLANs */ - if (hw->mac.vlan_override) + /* Verify that we have permission to add VLANs. If this is a request + * to remove a VLAN, we still want to allow the user to remove the + * VLAN device. In that case, we need to clear the bit in the + * active_vlans bitmask. + */ + if (set && hw->mac.vlan_override) return -EACCES; /* update active_vlans bitmask */ @@ -954,6 +958,12 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set) rx_ring->vid &= ~FM10K_VLAN_CLEAR; } + /* If our VLAN has been overridden, there is no reason to send VLAN + * removal requests as they will be silently ignored. + */ + if (hw->mac.vlan_override) + return 0; + /* Do not remove default VLAN ID related entries from VLAN and MAC * tables */