git: 3faea48e1179 - main - ice: Make VF MAC filter requests idempotent

From: Kevin Bowling <kbowling_at_FreeBSD.org>
Date: Fri, 18 Sep 2026 00:33:16 UTC
The branch main has been updated by kbowling:

URL: https://cgit.FreeBSD.org/src/commit/?id=3faea48e117939ae41f92c8738784d8e38a78612

commit 3faea48e117939ae41f92c8738784d8e38a78612
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-19 05:52:43 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-09-18 00:32:52 +0000

    ice: Make VF MAC filter requests idempotent
    
    VF drivers replay their address filters after reset and may retry a
    request whose reply was lost.  The PF tracked only a count and
    incremented it after an idempotent hardware add, so duplicate replays
    eventually exhausted the quota.  It then rejected an entire address
    batch, including the administrator-assigned address.
    
    Track exact non-primary MAC filter membership within each VF quota.
    Validate a complete batch before changing hardware, charge only unique
    absent addresses, and update ownership after each successful operation.
    Preserve an administrator-assigned address when the VF is not permitted
    to change it.
    
    Validated on an E810-XXV with a host-attached iavf VF.  The configured
    filter quota was filled, then the complete set was replayed across VFR
    and PF reset without a duplicate warning or ADD_ETH_ADDR NACK.  Deleting
    an absent address was a no-op.  With allow-set-mac disabled, the guest
    could not remove its administrator-assigned filter, while multicast
    filter additions continued to succeed.
    
    MFC after:      2 weeks
    Sponsored by:   BBOX.io
    Differential Revision:  https://reviews.freebsd.org/D59026
---
 share/man/man4/ice.4  |  19 +++++--
 sys/dev/ice/ice_iov.c | 144 +++++++++++++++++++++++++++++++++++++-------------
 sys/dev/ice/ice_iov.h |   5 ++
 3 files changed, 126 insertions(+), 42 deletions(-)

diff --git a/share/man/man4/ice.4 b/share/man/man4/ice.4
index a4bb498edf55..0174f09af4e9 100644
--- a/share/man/man4/ice.4
+++ b/share/man/man4/ice.4
@@ -1143,11 +1143,20 @@ resource; this is used to prevent a VF from starving other VFs or the PF of
 filter resources.
 By default, this is set to 16.
 .It max-mac-filters Pq uint16_t
-Specify maximum number of MAC address filters that the VF can use.
-Each allowed MAC address requires a hardware filter which are a finite
-resource; this is used to prevent a VF from starving other VFs or the PF of
-filter resources.
-The VF's default mac address does not count towards this limit.
+Specify the maximum number of unique MAC address filters that the VF can
+request.
+Repeated requests for the same address count once.
+Each allowed MAC address requires a hardware filter, which is a finite
+resource; this limit prevents a VF from starving other VFs or the PF of filter
+resources.
+A PF-assigned
+.Dq mac-addr
+does not count towards this limit.
+When
+.Dq mac-addr
+is omitted, a VF-chosen address, including an address randomly generated by
+.Xr iavf 4 ,
+counts as a unique filter.
 By default, this is set to 64.
 .El
 .Pp
diff --git a/sys/dev/ice/ice_iov.c b/sys/dev/ice/ice_iov.c
index 68cfae7197db..315950b6f8f8 100644
--- a/sys/dev/ice/ice_iov.c
+++ b/sys/dev/ice/ice_iov.c
@@ -103,6 +103,7 @@ static void ice_vc_del_vlan_msg(struct ice_softc *sc, struct ice_vf *vf,
 static int ice_vc_select_vlans(struct ice_vf *vf, u16 *vids, u16 count,
 			       bool add, u16 *selected_count);
 static enum virtchnl_status_code ice_iov_err_to_virt_err(int ice_err);
+static int ice_vf_mac_filter_index(struct ice_vf *vf, const uint8_t *addr);
 static int ice_vf_validate_mac(struct ice_vf *vf, const uint8_t *addr);
 
 #ifdef DRIVER_FAILPOINTS
@@ -520,6 +521,19 @@ ice_iov_add_vf(struct ice_softc *sc, uint16_t vfnum, const nvlist_t *params)
 
 	vf->vlan_limit = nvlist_get_number(params, "max-vlan-allowed");
 	vf->mac_filter_limit = nvlist_get_number(params, "max-mac-filters");
+	if (vf->mac_filter_limit != 0) {
+		vf->mac_filters = mallocarray(vf->mac_filter_limit,
+		    sizeof(*vf->mac_filters), M_ICE, M_NOWAIT | M_ZERO);
+		if (vf->mac_filters == NULL) {
+			device_printf(sc->dev,
+			    "Unable to allocate VF-%d MAC filter memory\n",
+			    vfnum);
+			error = ENOMEM;
+			goto release_imap;
+		}
+	}
+	ICE_IOV_FAIL_POINT(sc, vfnum, add_after_mac_filter_memory, error,
+	    free_mac_filters);
 
 	vf->vf_flags |= VF_FLAG_VLAN_CAP;
 
@@ -528,29 +542,33 @@ ice_iov_add_vf(struct ice_softc *sc, uint16_t vfnum, const nvlist_t *params)
 	if (error) {
 		device_printf(sc->dev, "Unable to initialize VF %d VSI: %s\n",
 			      vfnum, ice_err_str(error));
-		goto release_imap;
+		goto free_mac_filters;
 	}
 	ICE_IOV_FAIL_POINT(sc, vfnum, add_after_vsi_init, error,
-	    release_imap);
+	    free_mac_filters);
 	error = ice_iov_configure_mac_anti_spoof(sc, vf);
 	if (error != 0)
-		goto release_imap;
+		goto free_mac_filters;
 
 	/* Add the broadcast address */
 	error = ice_add_vsi_mac_filter(vsi, broadcastaddr);
 	if (error) {
 		device_printf(sc->dev, "Unable to add broadcast filter VF %d VSI: %s\n",
 			      vfnum, ice_err_str(error));
-		goto release_imap;
+		goto free_mac_filters;
 	}
 	ICE_IOV_FAIL_POINT(sc, vfnum, add_after_broadcast_filter, error,
-	    release_imap);
+	    free_mac_filters);
 
 	atomic_set_32(&vf->vf_flags, VF_FLAG_ENABLED);
 	ice_iov_ready_vf(sc, vf);
 
 	return (0);
 
+free_mac_filters:
+	free(vf->mac_filters, M_ICE);
+	vf->mac_filters = NULL;
+	vf->mac_filter_cnt = 0;
 release_imap:
 	ice_resmgr_release_map(&sc->dev_imgr, vf->vf_imap,
 			       vf->num_irq_vectors);
@@ -594,6 +612,9 @@ ice_iov_uninit(struct ice_softc *sc)
 		vf = &sc->vfs[i];
 		atomic_store_rel_32(&vf->vf_flags, 0);
 		vsi = vf->vsi;
+		free(vf->mac_filters, M_ICE);
+		vf->mac_filters = NULL;
+		vf->mac_filter_cnt = 0;
 
 		/* Free VF interrupt reservation */
 		if (vf->vf_imap) {
@@ -1180,6 +1201,25 @@ ice_vf_validate_mac(struct ice_vf *vf, const uint8_t *addr)
 	return (0);
 }
 
+/**
+ * ice_vf_mac_filter_index - Find a VF-owned MAC filter
+ * @vf: VF tracking structure
+ * @addr: MAC address to find
+ *
+ * The administrator-assigned address does not consume the configurable VF
+ * filter quota and is therefore not stored in this array.
+ */
+static int
+ice_vf_mac_filter_index(struct ice_vf *vf, const uint8_t *addr)
+{
+
+	for (u16 i = 0; i < vf->mac_filter_cnt; i++) {
+		if (memcmp(vf->mac_filters[i].addr, addr, ETHER_ADDR_LEN) == 0)
+			return (i);
+	}
+	return (-1);
+}
+
 /**
  * ice_vc_add_eth_addr_msg - Handle VIRTCHNL_OP_ADD_ETH_ADDR msg from VF
  * @sc: device private structure
@@ -1195,38 +1235,50 @@ ice_vc_add_eth_addr_msg(struct ice_softc *sc, struct ice_vf *vf, u8 *msg_buf)
 	enum virtchnl_status_code v_status = VIRTCHNL_STATUS_SUCCESS;
 	struct virtchnl_ether_addr_list *addr_list;
 	struct ice_hw *hw = &sc->hw;
-	u16 added_addr_cnt = 0;
+	u16 new_filters;
 	int error = 0;
 
 	addr_list = (struct virtchnl_ether_addr_list *)msg_buf;
 
-	if (addr_list->num_elements >
-	    (vf->mac_filter_limit - vf->mac_filter_cnt)) {
+	/* Validate the entire batch and charge only unique, absent filters. */
+	new_filters = 0;
+	for (int i = 0; i < addr_list->num_elements; i++) {
+		u8 *addr = addr_list->list[i].addr;
+		int j;
+
+		error = ice_vf_validate_mac(vf, addr);
+		if (error != 0) {
+			device_printf(sc->dev,
+			    "%s: VF-%d: invalid or unauthorized MAC for VSI %d\n",
+			    __func__, vf->vf_num, vf->vsi->idx);
+			v_status = VIRTCHNL_STATUS_ERR_PARAM;
+			goto done;
+		}
+		for (j = 0; j < i; j++) {
+			if (memcmp(addr_list->list[j].addr, addr,
+			    ETHER_ADDR_LEN) == 0)
+				break;
+		}
+		if (j != i || memcmp(addr, vf->mac, ETHER_ADDR_LEN) == 0 ||
+		    ice_vf_mac_filter_index(vf, addr) >= 0)
+			continue;
+		new_filters++;
+	}
+	if ((u32)vf->mac_filter_cnt + new_filters > vf->mac_filter_limit) {
 		v_status = VIRTCHNL_STATUS_ERR_NO_MEMORY;
 		goto done;
 	}
 
 	for (int i = 0; i < addr_list->num_elements; i++) {
 		u8 *addr = addr_list->list[i].addr;
+		bool assigned;
 
 		/* The type flag is currently ignored; every MAC address is
 		 * treated as the LEGACY type
 		 */
-
-		error = ice_vf_validate_mac(vf, addr);
-		if (error == EPERM) {
-			device_printf(sc->dev,
-			    "%s: VF-%d: Not permitted to add MAC addr for VSI %d\n",
-			    __func__, vf->vf_num, vf->vsi->idx);
-			v_status = VIRTCHNL_STATUS_ERR_PARAM;
-			continue;
-		} else if (error) {
-			device_printf(sc->dev,
-			    "%s: VF-%d: Did not add invalid MAC addr for VSI %d\n",
-			    __func__, vf->vf_num, vf->vsi->idx);
-			v_status = VIRTCHNL_STATUS_ERR_PARAM;
+		assigned = memcmp(addr, vf->mac, ETHER_ADDR_LEN) == 0;
+		if (!assigned && ice_vf_mac_filter_index(vf, addr) >= 0)
 			continue;
-		}
 
 		error = ice_add_vsi_mac_filter(vf->vsi, addr);
 		if (error) {
@@ -1236,13 +1288,14 @@ ice_vc_add_eth_addr_msg(struct ice_softc *sc, struct ice_vf *vf, u8 *msg_buf)
 			v_status = VIRTCHNL_STATUS_ERR_PARAM;
 			continue;
 		}
-		/* Don't count VF's MAC against its MAC filter limit */
-		if (memcmp(addr, vf->mac, ETHER_ADDR_LEN))
-			added_addr_cnt++;
+		if (!assigned) {
+			MPASS(vf->mac_filter_cnt < vf->mac_filter_limit);
+			memcpy(vf->mac_filters[vf->mac_filter_cnt].addr, addr,
+			    ETHER_ADDR_LEN);
+			vf->mac_filter_cnt++;
+		}
 	}
 
-	vf->mac_filter_cnt += added_addr_cnt;
-
 done:
 	ice_aq_send_msg_to_vf(hw, vf->vf_num, VIRTCHNL_OP_ADD_ETH_ADDR,
 	    v_status, NULL, 0, NULL);
@@ -1263,13 +1316,29 @@ ice_vc_del_eth_addr_msg(struct ice_softc *sc, struct ice_vf *vf, u8 *msg_buf)
 	enum virtchnl_status_code v_status = VIRTCHNL_STATUS_SUCCESS;
 	struct virtchnl_ether_addr_list *addr_list;
 	struct ice_hw *hw = &sc->hw;
-	u16 deleted_addr_cnt = 0;
 	int error = 0;
 
 	addr_list = (struct virtchnl_ether_addr_list *)msg_buf;
 
 	for (int i = 0; i < addr_list->num_elements; i++) {
-		error = ice_remove_vsi_mac_filter(vf->vsi, addr_list->list[i].addr);
+		u8 *addr = addr_list->list[i].addr;
+		bool assigned;
+		int index;
+
+		error = ice_vf_validate_mac(vf, addr);
+		if (error != 0) {
+			v_status = VIRTCHNL_STATUS_ERR_PARAM;
+			continue;
+		}
+		assigned = memcmp(addr, vf->mac, ETHER_ADDR_LEN) == 0;
+		if (assigned &&
+		    (vf->vf_flags & VF_FLAG_SET_MAC_CAP) == 0)
+			continue;
+		index = assigned ? -1 : ice_vf_mac_filter_index(vf, addr);
+		if (!assigned && index < 0)
+			continue;
+
+		error = ice_remove_vsi_mac_filter(vf->vsi, addr);
 		if (error) {
 			device_printf(sc->dev,
 			    "%s: VF-%d: Error removing MAC addr for VSI %d\n",
@@ -1277,16 +1346,17 @@ ice_vc_del_eth_addr_msg(struct ice_softc *sc, struct ice_vf *vf, u8 *msg_buf)
 			v_status = VIRTCHNL_STATUS_ERR_PARAM;
 			continue;
 		}
-		/* Don't count VF's MAC against its MAC filter limit */
-		if (memcmp(addr_list->list[i].addr, vf->mac, ETHER_ADDR_LEN))
-			deleted_addr_cnt++;
+		if (!assigned) {
+			if (index + 1 < vf->mac_filter_cnt) {
+				memmove(&vf->mac_filters[index],
+				    &vf->mac_filters[index + 1],
+				    (vf->mac_filter_cnt - index - 1) *
+				    sizeof(*vf->mac_filters));
+			}
+			vf->mac_filter_cnt--;
+		}
 	}
 
-	if (deleted_addr_cnt >= vf->mac_filter_cnt)
-		vf->mac_filter_cnt = 0;
-	else
-		vf->mac_filter_cnt -= deleted_addr_cnt;
-
 	ice_aq_send_msg_to_vf(hw, vf->vf_num, VIRTCHNL_OP_DEL_ETH_ADDR,
 	    v_status, NULL, 0, NULL);
 }
diff --git a/sys/dev/ice/ice_iov.h b/sys/dev/ice/ice_iov.h
index 9091bbd8fbf6..fc78169172d2 100644
--- a/sys/dev/ice/ice_iov.h
+++ b/sys/dev/ice/ice_iov.h
@@ -75,6 +75,10 @@ enum ice_vf_flags {
 	VF_FLAG_RESET_FAILED		= BIT(7),
 };
 
+struct ice_vf_mac_filter {
+	u8 addr[ETHER_ADDR_LEN];
+};
+
 /**
  * @struct ice_vf
  * @brief PF's VF software context
@@ -91,6 +95,7 @@ struct ice_vf {
 
 	u16 mac_filter_limit;
 	u16 mac_filter_cnt;
+	struct ice_vf_mac_filter *mac_filters;
 	u16 vlan_limit;
 	u16 vlan_cnt;
 #define ICE_VF_VLAN_MAP_LEN	(EVL_VLID_MASK + 1)