When an ice port in a vlan-filtering bridge goes promiscuous (typical for
bond slaves), the driver installs a per-VLAN ICE_SW_LKUP_PROMISC_VLAN rule
for every VID on top of the broad ICE_SW_LKUP_DFLT VSI Rx rule. Each rule
consumes one of the ~32K Flow Lookup Unit (FLU) entries the device shares
across PFs, so a wide trunk (vid 2-4094) over several PFs overruns the
pool: firmware rejects further Add Switch Rules with ENOSPC (AQ 0x10) and
the DFLT Rx rule itself fails to install:
ice 0000:5c:00.1: Failed to set VSI 14 as the default forwarding
VSI, error -5
ice 0000:5c:00.1 ens1f1: Error -5 setting default VSI 14 Rx rule
Once a switch context is overrun the retries can also come back as ENOENT
(AQ 0x2), which has misled triage toward a perceived recipe binding defect
rather than a capacity issue.
The DFLT rule already catches every packet on the port regardless of VLAN
tag, so the per-VLAN promisc expansion is redundant while it is installed.
Skip it at the two sites that drive it, ice_set_promisc() and
ice_vlan_rx_add_vid(), keyed on ice_is_vsi_dflt_vsi() rather than the
netdev IFF_PROMISC flag so a failed or LAG-suppressed DFLT install still
falls back to the per-VLAN rules.
IFF_ALLMULTI and IFF_PROMISC can reach ice_vsi_sync_fltr() in separate
passes (a bridge join sets them through separate calls), so the allmulti
pass may expand the per-VID rules before the DFLT rule exists. Drop those
now-redundant rules right after ice_set_dflt_vsi() installs the DFLT rule;
ice_vsi_exit_dflt_promisc() reinstates them when promisc is cleared.
ice_vsi_sync_fltr() subscribed multicast promiscuity only inside the
"default VSI not yet in use" branch, so a promiscuous VSI that finds the
default VSI rule already present (owned by another VSI, or preserved
across a switchdev session) ended up in unicast promisc with no multicast
subscription. Issue ice_set_promisc(ICE_MCAST_PROMISC_BITS) whenever the
netdev is promiscuous; it is idempotent and returns 0 if the rule is
already present.
Fixes: 1273f89578f2 ("ice: Fix broken IFF_ALLMULTI handling")
Signed-off-by: Petr Oros <[email protected]>
---
v4 (review comments from Marcin Szycik):
- Reworded the ice_clear_promisc() and the per-VID cleanup comments.
- Flipped the ice_vsi_clear_vlan_mc_promisc() guard to an early return to
reduce indentation, and noted that the clear is best-effort.
No functional change.
v3: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v1:
https://lore.kernel.org/all/89efbea9831175e6f57e9fe8557f7a0e48e050b7.1781786935.git.po...@redhat.com/
---
drivers/net/ethernet/intel/ice/ice_main.c | 109 +++++++++++++++++-----
1 file changed, 88 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
b/drivers/net/ethernet/intel/ice/ice_main.c
index b43d420ece99ca..f1fef26eeadac9 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -274,7 +274,8 @@ static int ice_set_promisc(struct ice_vsi *vsi, u8
promisc_m)
if (vsi->type != ICE_VSI_PF)
return 0;
- if (ice_vsi_has_non_zero_vlans(vsi)) {
+ /* skip per-VID expansion; the DFLT Rx rule already covers every VID */
+ if (ice_vsi_has_non_zero_vlans(vsi) && !ice_is_vsi_dflt_vsi(vsi)) {
promisc_m |= (ICE_PROMISC_VLAN_RX | ICE_PROMISC_VLAN_TX);
status = ice_fltr_set_vlan_vsi_promisc(&vsi->back->hw, vsi,
promisc_m);
@@ -304,9 +305,20 @@ static int ice_clear_promisc(struct ice_vsi *vsi, u8
promisc_m)
return 0;
if (ice_vsi_has_non_zero_vlans(vsi)) {
- promisc_m |= (ICE_PROMISC_VLAN_RX | ICE_PROMISC_VLAN_TX);
+ u8 vlan_promisc_m = promisc_m | ICE_PROMISC_VLAN_RX |
+ ICE_PROMISC_VLAN_TX;
+ int vid0_status;
+
+ /* the vid=0 rule may be in either recipe (the recipe used to
+ * set it is not recorded), so clear both; clearing an absent
+ * rule returns 0
+ */
status = ice_fltr_clear_vlan_vsi_promisc(&vsi->back->hw, vsi,
- promisc_m);
+ vlan_promisc_m);
+ vid0_status = ice_fltr_clear_vsi_promisc(&vsi->back->hw,
+ vsi->idx, promisc_m,
0);
+ if (!status)
+ status = vid0_status;
} else {
status = ice_fltr_clear_vsi_promisc(&vsi->back->hw, vsi->idx,
promisc_m, 0);
@@ -317,6 +329,61 @@ static int ice_clear_promisc(struct ice_vsi *vsi, u8
promisc_m)
return status;
}
+/**
+ * ice_vsi_exit_dflt_promisc - drop the default VSI Rx rule on promisc off
+ * @vsi: the VSI leaving promiscuous mode
+ *
+ * For an IFF_ALLMULTI VSI with VLANs the per-VID multicast rules are
+ * reinstated before the default rule is cleared so coverage never lapses;
+ * the then redundant vid=0 rule is dropped best-effort. The callees log
+ * their own failures, so error returns are not re-logged here.
+ *
+ * Return: 0 on success, negative on error with the default rule left in place.
+ */
+static int ice_vsi_exit_dflt_promisc(struct ice_vsi *vsi)
+{
+ struct ice_vsi_vlan_ops *vlan_ops = ice_get_compat_vsi_vlan_ops(vsi);
+ struct net_device *netdev = vsi->netdev;
+ struct ice_hw *hw = &vsi->back->hw;
+ bool restore_mc;
+ int err;
+
+ restore_mc = (vsi->current_netdev_flags & IFF_ALLMULTI) &&
+ ice_vsi_has_non_zero_vlans(vsi);
+
+ if (restore_mc) {
+ err = ice_fltr_set_vlan_vsi_promisc(hw, vsi,
+
ICE_MCAST_VLAN_PROMISC_BITS);
+ if (err && err != -EEXIST)
+ return err;
+ }
+
+ err = ice_clear_dflt_vsi(vsi);
+ if (err)
+ return err;
+
+ if (netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER)
+ vlan_ops->ena_rx_filtering(vsi);
+
+ if (restore_mc)
+ ice_fltr_clear_vsi_promisc(hw, vsi->idx, ICE_MCAST_PROMISC_BITS,
+ 0);
+
+ return 0;
+}
+
+/* Drop the per-VID multicast promisc rules made redundant by the default
+ * VSI Rx rule; best-effort, a leftover is harmless while that rule stands.
+ */
+static void ice_vsi_clear_vlan_mc_promisc(struct ice_vsi *vsi)
+{
+ if (!ice_vsi_has_non_zero_vlans(vsi))
+ return;
+
+ ice_fltr_clear_vlan_vsi_promisc(&vsi->back->hw, vsi,
+ ICE_MCAST_VLAN_PROMISC_BITS);
+}
+
/**
* ice_vsi_sync_fltr - Update the VSI filter list to the HW
* @vsi: ptr to the VSI
@@ -429,30 +496,31 @@ static int ice_vsi_sync_fltr(struct ice_vsi *vsi)
err = 0;
vlan_ops->dis_rx_filtering(vsi);
- /* promiscuous mode implies allmulticast so
- * that VSIs that are in promiscuous mode are
- * subscribed to multicast packets coming to
- * the port
+ /* a prior allmulti pass may have added per-VID
+ * rules now covered by the DFLT rule
*/
- err = ice_set_promisc(vsi,
- ICE_MCAST_PROMISC_BITS);
- if (err)
- goto out_promisc;
+ ice_vsi_clear_vlan_mc_promisc(vsi);
}
+
+ /* Promiscuous mode implies allmulticast. Subscribe
+ * the VSI to all multicast even when the default VSI
+ * rule is already in use and the block above is
+ * skipped (it may be owned by another VSI, or
+ * preserved across a switchdev session); the unicast
+ * catch-all does not cover the multicast subscription.
+ */
+ err = ice_set_promisc(vsi, ICE_MCAST_PROMISC_BITS);
+ if (err)
+ goto out_promisc;
} else {
/* Clear Rx filter to remove traffic from wire */
if (ice_is_vsi_dflt_vsi(vsi)) {
- err = ice_clear_dflt_vsi(vsi);
+ err = ice_vsi_exit_dflt_promisc(vsi);
if (err) {
- netdev_err(netdev, "Error %d clearing
default VSI %i Rx rule\n",
- err, vsi->vsi_num);
vsi->current_netdev_flags |=
IFF_PROMISC;
goto out_promisc;
}
- if (vsi->netdev->features &
- NETIF_F_HW_VLAN_CTAG_FILTER)
- vlan_ops->ena_rx_filtering(vsi);
}
/* disable allmulti here, but only if allmulti is not
@@ -3676,10 +3744,9 @@ int ice_vlan_rx_add_vid(struct net_device *netdev,
__be16 proto, u16 vid)
while (test_and_set_bit(ICE_CFG_BUSY, vsi->state))
usleep_range(1000, 2000);
- /* Add multicast promisc rule for the VLAN ID to be added if
- * all-multicast is currently enabled.
- */
- if (vsi->current_netdev_flags & IFF_ALLMULTI) {
+ /* skip the per-VID rule when the DFLT Rx rule already covers this VID
*/
+ if ((vsi->current_netdev_flags & IFF_ALLMULTI) &&
+ !ice_is_vsi_dflt_vsi(vsi)) {
ret = ice_fltr_set_vsi_promisc(&vsi->back->hw, vsi->idx,
ICE_MCAST_VLAN_PROMISC_BITS,
vid);
--
2.54.0
