Date: Fri, 06 Feb 2026 17:59:36 +0000 From: Zhenlei Huang <zlei@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 70256d2b86d9 - main - qlnxe: Overhaul setting the multicast MAC filters Message-ID: <69862c08.27044.1d324723@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by zlei: URL: https://cgit.FreeBSD.org/src/commit/?id=70256d2b86d95a678a63c65b157b9c635f1f4c6a commit 70256d2b86d95a678a63c65b157b9c635f1f4c6a Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2026-02-06 17:52:55 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2026-02-06 17:58:16 +0000 qlnxe: Overhaul setting the multicast MAC filters When operating the multicast MAC filters, the current usage of ECORE_FILTER_ADD and ECORE_FILTER_REMOVE are rather misleading. ECORE_FILTER_ADD reads "adding new filter", but it actually removes any existing filters and then addes a new one. ECORE_FILTER_REMOVE reads "removing a filter", but it actually removes all filters. Let's use ECORE_FILTER_REPLACE and ECORE_FILTER_FLUSH instead to avoid confusion. In the current implementation, only one MAC address is passed to ecore_sp_eth_filter_mcast() and any previously installed filters are removed, hence it breaks the multicast function. That can be observed via either assigning new IPv6 addresses to the interface or putting the interface as a member of lagg(4) interface with LACP aggregation protocol. Fix that by calculating the multicast filter bins directly from multicast MAC addresses and replace the filters every time the bins changes. Due to the nature of the multicast filter, which is hash based, a full 1's multicast filter bin means all multicast packets are to be accepted. Thus there's no need to make the vport into allmulti mode when the number of multicast MAC addresses exceeds the limit (ECORE_MAX_MC_ADDRS, 64). Tested with a FastLinQ QL41212HLCU 25GbE adapter, both MFW_Version 8.35.23.0 and 8.59.16.0 are tested. Note: Currently the VF port is set to promiscuous mode unconditionally, and the setting of the multicast MAC filters for VF ports is short-circuited, so the VF port functions as it did. PR: 265857 PR: 290973 Reviewed by: kbowling MFC after: 5 days Differential Revision: https://reviews.freebsd.org/D54892 --- sys/dev/qlnx/qlnxe/ecore_l2.c | 41 ++++---- sys/dev/qlnx/qlnxe/ecore_l2_api.h | 9 +- sys/dev/qlnx/qlnxe/ecore_vf.c | 11 +-- sys/dev/qlnx/qlnxe/qlnx_def.h | 5 +- sys/dev/qlnx/qlnxe/qlnx_os.c | 190 ++++++++------------------------------ 5 files changed, 66 insertions(+), 190 deletions(-) diff --git a/sys/dev/qlnx/qlnxe/ecore_l2.c b/sys/dev/qlnx/qlnxe/ecore_l2.c index ee7d225540d0..5e281c330765 100644 --- a/sys/dev/qlnx/qlnxe/ecore_l2.c +++ b/sys/dev/qlnx/qlnxe/ecore_l2.c @@ -1617,14 +1617,13 @@ ecore_sp_eth_filter_mcast(struct ecore_hwfn *p_hwfn, struct ecore_spq_comp_cb *p_comp_data) { struct vport_update_ramrod_data *p_ramrod = OSAL_NULL; - u32 bins[ETH_MULTICAST_MAC_BINS_IN_REGS]; struct ecore_spq_entry *p_ent = OSAL_NULL; struct ecore_sp_init_data init_data; u8 abs_vport_id = 0; enum _ecore_status_t rc; int i; - if (p_filter_cmd->opcode == ECORE_FILTER_ADD) + if (p_filter_cmd->opcode == ECORE_FILTER_REPLACE) rc = ecore_fw_vport(p_hwfn, p_filter_cmd->vport_to_add_to, &abs_vport_id); else @@ -1654,30 +1653,33 @@ ecore_sp_eth_filter_mcast(struct ecore_hwfn *p_hwfn, /* explicitly clear out the entire vector */ OSAL_MEMSET(&p_ramrod->approx_mcast.bins, 0, sizeof(p_ramrod->approx_mcast.bins)); - OSAL_MEMSET(bins, 0, sizeof(u32) * ETH_MULTICAST_MAC_BINS_IN_REGS); - /* filter ADD op is explicit set op and it removes - * any existing filters for the vport. - */ - if (p_filter_cmd->opcode == ECORE_FILTER_ADD) { - for (i = 0; i < p_filter_cmd->num_mc_addrs; i++) { - u32 bit; - - bit = ecore_mcast_bin_from_mac(p_filter_cmd->mac[i]); - bins[bit / 32] |= 1 << (bit % 32); - } - + /* + * filter REPLACE op is explicit set op and it removes + * any existing filters for the vport. + */ + if (p_filter_cmd->opcode == ECORE_FILTER_REPLACE) { + _Static_assert(sizeof(p_filter_cmd->bins) == sizeof(p_ramrod->approx_mcast.bins), "Size mismatch"); + _Static_assert(nitems(p_filter_cmd->bins) == ETH_MULTICAST_MAC_BINS_IN_REGS, "Size mismatch"); /* Convert to correct endianity */ for (i = 0; i < ETH_MULTICAST_MAC_BINS_IN_REGS; i++) { struct vport_update_ramrod_mcast *p_ramrod_bins; p_ramrod_bins = &p_ramrod->approx_mcast; - p_ramrod_bins->bins[i] = OSAL_CPU_TO_LE32(bins[i]); + p_ramrod_bins->bins[i] = OSAL_CPU_TO_LE32(p_filter_cmd->bins[i]); } - } + } /* else FLUSH op clears existing filters */ p_ramrod->common.vport_id = abs_vport_id; rc = ecore_spq_post(p_hwfn, p_ent, OSAL_NULL); + + DP_INFO(p_hwfn, "Multicast filter cmd: [%s], bins: [%08x, %08x, %08x, %08x, %08x, %08x, %08x, %08x], ret = %d\n", + p_filter_cmd->opcode == ECORE_FILTER_REPLACE ? "replace" : "flush", + p_ramrod->approx_mcast.bins[0], p_ramrod->approx_mcast.bins[1], + p_ramrod->approx_mcast.bins[2], p_ramrod->approx_mcast.bins[3], + p_ramrod->approx_mcast.bins[4], p_ramrod->approx_mcast.bins[5], + p_ramrod->approx_mcast.bins[6], p_ramrod->approx_mcast.bins[7], rc); + if (rc != ECORE_SUCCESS) DP_ERR(p_hwfn, "Multicast filter command failed %d\n", rc); @@ -1692,10 +1694,9 @@ enum _ecore_status_t ecore_filter_mcast_cmd(struct ecore_dev *p_dev, enum _ecore_status_t rc = ECORE_SUCCESS; int i; - /* only ADD and REMOVE operations are supported for multi-cast */ - if ((p_filter_cmd->opcode != ECORE_FILTER_ADD && - (p_filter_cmd->opcode != ECORE_FILTER_REMOVE)) || - (p_filter_cmd->num_mc_addrs > ECORE_MAX_MC_ADDRS)) { + /* only REPLACE and FLUSH operations are supported for multi-cast */ + if ((p_filter_cmd->opcode != ECORE_FILTER_REPLACE && + (p_filter_cmd->opcode != ECORE_FILTER_FLUSH))) { return ECORE_INVAL; } diff --git a/sys/dev/qlnx/qlnxe/ecore_l2_api.h b/sys/dev/qlnx/qlnxe/ecore_l2_api.h index 610cc329f1fd..3b2f48e9fe11 100644 --- a/sys/dev/qlnx/qlnxe/ecore_l2_api.h +++ b/sys/dev/qlnx/qlnxe/ecore_l2_api.h @@ -162,14 +162,13 @@ struct ecore_filter_ucast { }; struct ecore_filter_mcast { - /* MOVE is not supported for multicast */ + /* Only REPLACE and FLUSH is supported for multicast */ enum ecore_filter_opcode opcode; u8 vport_to_add_to; u8 vport_to_remove_from; - u8 num_mc_addrs; -#define ECORE_MAX_MC_ADDRS 64 - unsigned char mac[ECORE_MAX_MC_ADDRS][ETH_ALEN]; + u32 bins[ETH_MULTICAST_MAC_BINS_IN_REGS]; }; +#define ECORE_MAX_MC_ADDRS 64 struct ecore_filter_accept_flags { u8 update_rx_mode_config; @@ -384,7 +383,7 @@ struct ecore_sp_vport_update_params { u8 anti_spoofing_en; u8 update_accept_any_vlan_flg; u8 accept_any_vlan; - u32 bins[8]; + u32 bins[ETH_MULTICAST_MAC_BINS_IN_REGS]; struct ecore_rss_params *rss_params; struct ecore_filter_accept_flags accept_flags; struct ecore_sge_tpa_params *sge_tpa_params; diff --git a/sys/dev/qlnx/qlnxe/ecore_vf.c b/sys/dev/qlnx/qlnxe/ecore_vf.c index 6cdc94d05c86..a7b006b982de 100644 --- a/sys/dev/qlnx/qlnxe/ecore_vf.c +++ b/sys/dev/qlnx/qlnxe/ecore_vf.c @@ -1515,18 +1515,13 @@ void ecore_vf_pf_filter_mcast(struct ecore_hwfn *p_hwfn, struct ecore_filter_mcast *p_filter_cmd) { struct ecore_sp_vport_update_params sp_params; - int i; OSAL_MEMSET(&sp_params, 0, sizeof(sp_params)); sp_params.update_approx_mcast_flg = 1; - if (p_filter_cmd->opcode == ECORE_FILTER_ADD) { - for (i = 0; i < p_filter_cmd->num_mc_addrs; i++) { - u32 bit; - - bit = ecore_mcast_bin_from_mac(p_filter_cmd->mac[i]); - sp_params.bins[bit / 32] |= 1 << (bit % 32); - } + if (p_filter_cmd->opcode == ECORE_FILTER_REPLACE) { + _Static_assert(sizeof(sp_params.bins) == sizeof(p_filter_cmd->bins), "Size mismatch"); + memcpy(sp_params.bins, p_filter_cmd->bins, sizeof(sp_params.bins)); } ecore_vf_pf_vport_update(p_hwfn, &sp_params); diff --git a/sys/dev/qlnx/qlnxe/qlnx_def.h b/sys/dev/qlnx/qlnxe/qlnx_def.h index 6184e78fe846..f895487c47e4 100644 --- a/sys/dev/qlnx/qlnxe/qlnx_def.h +++ b/sys/dev/qlnx/qlnxe/qlnx_def.h @@ -319,7 +319,6 @@ typedef struct qlnx_link_output qlnx_link_output_t; #define QLNX_TPA_MAX_AGG_BUFFERS (20) -#define QLNX_MAX_NUM_MULTICAST_ADDRS ECORE_MAX_MC_ADDRS typedef struct _qlnx_mcast { uint16_t rsrvd; uint8_t addr[6]; @@ -442,9 +441,7 @@ struct qlnx_host { qlnx_ivec_t irq_vec[QLNX_MAX_RSS]; uint8_t filter; - uint32_t nmcast; - qlnx_mcast_t mcast[QLNX_MAX_NUM_MULTICAST_ADDRS]; - struct ecore_filter_mcast ecore_mcast; + uint32_t ecore_mcast_bins[ETH_MULTICAST_MAC_BINS_IN_REGS]; uint8_t primary_mac[ETH_ALEN]; uint8_t prio_to_tc[MAX_NUM_PRI]; struct ecore_eth_stats hw_stats; diff --git a/sys/dev/qlnx/qlnxe/qlnx_os.c b/sys/dev/qlnx/qlnxe/qlnx_os.c index 5f6f336aab8d..85c16842283c 100644 --- a/sys/dev/qlnx/qlnxe/qlnx_os.c +++ b/sys/dev/qlnx/qlnxe/qlnx_os.c @@ -48,6 +48,7 @@ #include "ecore_sp_commands.h" #include "ecore_dev_api.h" #include "ecore_l2_api.h" +#include "ecore_l2.h" #include "ecore_mcp.h" #include "ecore_hw_defs.h" #include "mcp_public.h" @@ -89,7 +90,7 @@ static void qlnx_fp_isr(void *arg); static void qlnx_init_ifnet(device_t dev, qlnx_host_t *ha); static void qlnx_init(void *arg); static void qlnx_init_locked(qlnx_host_t *ha); -static int qlnx_set_multi(qlnx_host_t *ha, uint32_t add_multi); +static int qlnx_set_multi(qlnx_host_t *ha); static int qlnx_set_promisc_allmulti(qlnx_host_t *ha, int flags); static int _qlnx_set_promisc_allmulti(qlnx_host_t *ha, bool promisc, bool allmulti); static int qlnx_ioctl(if_t ifp, u_long cmd, caddr_t data); @@ -127,14 +128,13 @@ static void qlnx_set_id(struct ecore_dev *cdev, char name[NAME_SIZE], char ver_str[VER_SIZE]); static void qlnx_unload(qlnx_host_t *ha); static int qlnx_load(qlnx_host_t *ha); -static void qlnx_hw_set_multi(qlnx_host_t *ha, uint8_t *mta, uint32_t mcnt, - uint32_t add_mac); static void qlnx_dump_buf8(qlnx_host_t *ha, const char *msg, void *dbuf, uint32_t len); static int qlnx_alloc_rx_buffer(qlnx_host_t *ha, struct qlnx_rx_queue *rxq); static void qlnx_reuse_rx_data(struct qlnx_rx_queue *rxq); static void qlnx_update_rx_prod(struct ecore_hwfn *p_hwfn, struct qlnx_rx_queue *rxq); +static int qlnx_remove_all_mcast_mac(qlnx_host_t *ha); static int qlnx_set_rx_accept_filter(qlnx_host_t *ha, uint8_t filter); static int qlnx_grc_dumpsize(qlnx_host_t *ha, uint32_t *num_dwords, int hwfn_index); @@ -2438,138 +2438,45 @@ qlnx_init(void *arg) return; } -static int -qlnx_config_mcast_mac_addr(qlnx_host_t *ha, uint8_t *mac_addr, uint32_t add_mac) -{ - struct ecore_filter_mcast *mcast; - struct ecore_dev *cdev; - int rc; - - cdev = &ha->cdev; - - mcast = &ha->ecore_mcast; - bzero(mcast, sizeof(struct ecore_filter_mcast)); - - if (add_mac) - mcast->opcode = ECORE_FILTER_ADD; - else - mcast->opcode = ECORE_FILTER_REMOVE; - - mcast->num_mc_addrs = 1; - memcpy(mcast->mac, mac_addr, ETH_ALEN); - - rc = ecore_filter_mcast_cmd(cdev, mcast, ECORE_SPQ_MODE_CB, NULL); - - return (rc); -} - -static int -qlnx_hw_add_mcast(qlnx_host_t *ha, uint8_t *mta) -{ - int i; - - for (i = 0; i < QLNX_MAX_NUM_MULTICAST_ADDRS; i++) { - if (QL_MAC_CMP(ha->mcast[i].addr, mta) == 0) - return 0; /* its been already added */ - } - - for (i = 0; i < QLNX_MAX_NUM_MULTICAST_ADDRS; i++) { - if ((ha->mcast[i].addr[0] == 0) && - (ha->mcast[i].addr[1] == 0) && - (ha->mcast[i].addr[2] == 0) && - (ha->mcast[i].addr[3] == 0) && - (ha->mcast[i].addr[4] == 0) && - (ha->mcast[i].addr[5] == 0)) { - if (qlnx_config_mcast_mac_addr(ha, mta, 1)) - return (-1); - - bcopy(mta, ha->mcast[i].addr, ETH_ALEN); - ha->nmcast++; - - return 0; - } - } - return 0; -} - -static int -qlnx_hw_del_mcast(qlnx_host_t *ha, uint8_t *mta) -{ - int i; - - for (i = 0; i < QLNX_MAX_NUM_MULTICAST_ADDRS; i++) { - if (QL_MAC_CMP(ha->mcast[i].addr, mta) == 0) { - if (qlnx_config_mcast_mac_addr(ha, mta, 0)) - return (-1); - - ha->mcast[i].addr[0] = 0; - ha->mcast[i].addr[1] = 0; - ha->mcast[i].addr[2] = 0; - ha->mcast[i].addr[3] = 0; - ha->mcast[i].addr[4] = 0; - ha->mcast[i].addr[5] = 0; - - ha->nmcast--; - - return 0; - } - } - return 0; -} - -/* - * Name: qls_hw_set_multi - * Function: Sets the Multicast Addresses provided the host O.S into the - * hardware (for the given interface) - */ -static void -qlnx_hw_set_multi(qlnx_host_t *ha, uint8_t *mta, uint32_t mcnt, - uint32_t add_mac) -{ - int i; - - for (i = 0; i < mcnt; i++) { - if (add_mac) { - if (qlnx_hw_add_mcast(ha, mta)) - break; - } else { - if (qlnx_hw_del_mcast(ha, mta)) - break; - } - - mta += ETHER_ADDR_LEN; - } - return; -} - static u_int -qlnx_copy_maddr(void *arg, struct sockaddr_dl *sdl, u_int mcnt) +qlnx_mcast_bins_from_maddr(void *arg, struct sockaddr_dl *sdl, u_int mcnt) { - uint8_t *mta = arg; - - if (mcnt == QLNX_MAX_NUM_MULTICAST_ADDRS) - return (0); + uint8_t bit; + uint32_t *bins = arg; - bcopy(LLADDR(sdl), &mta[mcnt * ETHER_ADDR_LEN], ETHER_ADDR_LEN); + bit = ecore_mcast_bin_from_mac(LLADDR(sdl)); + bins[bit / 32] |= 1 << (bit % 32); return (1); } static int -qlnx_set_multi(qlnx_host_t *ha, uint32_t add_multi) +qlnx_set_multi(qlnx_host_t *ha) { - uint8_t mta[QLNX_MAX_NUM_MULTICAST_ADDRS * ETHER_ADDR_LEN]; - if_t ifp = ha->ifp; - u_int mcnt; + struct ecore_filter_mcast mcast; + struct ecore_dev *cdev; + if_t ifp = ha->ifp; + u_int mcnt __unused; + int rc; if (qlnx_vf_device(ha) == 0) return (0); - mcnt = if_foreach_llmaddr(ifp, qlnx_copy_maddr, mta); + bzero(&mcast, sizeof(struct ecore_filter_mcast)); + mcnt = if_foreach_llmaddr(ifp, qlnx_mcast_bins_from_maddr, mcast.bins); + QL_DPRINT1(ha, "total %d multicast MACs found\n", mcnt); - qlnx_hw_set_multi(ha, mta, mcnt, add_multi); + if (memcmp(ha->ecore_mcast_bins, mcast.bins, sizeof(mcast.bins)) == 0) + return (0); - return (0); + cdev = &ha->cdev; + mcast.opcode = ECORE_FILTER_REPLACE; + rc = ecore_filter_mcast_cmd(cdev, &mcast, ECORE_SPQ_MODE_CB, NULL); + if (rc == 0) + memcpy(ha->ecore_mcast_bins, mcast.bins, sizeof(mcast.bins)); + + QL_DPRINT1(ha, "ecore_filter_mcast_cmd: end(%d)\n", rc); + return (rc); } static int @@ -2673,22 +2580,12 @@ qlnx_ioctl(if_t ifp, u_long cmd, caddr_t data) break; case SIOCADDMULTI: - QL_DPRINT4(ha, "%s (0x%lx)\n", "SIOCADDMULTI", cmd); - - QLNX_LOCK(ha); - if ((if_getdrvflags(ifp) & IFF_DRV_RUNNING) != 0) { - if (qlnx_set_multi(ha, 1)) - ret = EINVAL; - } - QLNX_UNLOCK(ha); - break; - case SIOCDELMULTI: - QL_DPRINT4(ha, "%s (0x%lx)\n", "SIOCDELMULTI", cmd); + QL_DPRINT4(ha, "%s (0x%lx)\n", "SIOCADDMULTI/SIOCDELMULTI", cmd); QLNX_LOCK(ha); if ((if_getdrvflags(ifp) & IFF_DRV_RUNNING) != 0) { - if (qlnx_set_multi(ha, 0)) + if (qlnx_set_multi(ha) != 0) ret = EINVAL; } QLNX_UNLOCK(ha); @@ -6955,31 +6852,18 @@ qlnx_remove_all_ucast_mac(qlnx_host_t *ha) static int qlnx_remove_all_mcast_mac(qlnx_host_t *ha) { - struct ecore_filter_mcast *mcast; + struct ecore_filter_mcast mcast; struct ecore_dev *cdev; - int rc, i; + int rc; cdev = &ha->cdev; - mcast = &ha->ecore_mcast; - bzero(mcast, sizeof(struct ecore_filter_mcast)); - - mcast->opcode = ECORE_FILTER_REMOVE; - - for (i = 0; i < QLNX_MAX_NUM_MULTICAST_ADDRS; i++) { - if (ha->mcast[i].addr[0] || ha->mcast[i].addr[1] || - ha->mcast[i].addr[2] || ha->mcast[i].addr[3] || - ha->mcast[i].addr[4] || ha->mcast[i].addr[5]) { - memcpy(&mcast->mac[i][0], &ha->mcast[i].addr[0], ETH_ALEN); - mcast->num_mc_addrs++; - } - } - mcast = &ha->ecore_mcast; - - rc = ecore_filter_mcast_cmd(cdev, mcast, ECORE_SPQ_MODE_CB, NULL); + bzero(&mcast, sizeof(struct ecore_filter_mcast)); + mcast.opcode = ECORE_FILTER_FLUSH; - bzero(ha->mcast, (sizeof(qlnx_mcast_t) * QLNX_MAX_NUM_MULTICAST_ADDRS)); - ha->nmcast = 0; + rc = ecore_filter_mcast_cmd(cdev, &mcast, ECORE_SPQ_MODE_CB, NULL); + if (rc == 0) + bzero(ha->ecore_mcast_bins, sizeof(ha->ecore_mcast_bins)); return (rc); } @@ -7041,7 +6925,7 @@ qlnx_set_rx_mode(qlnx_host_t *ha) if (rc) return rc; - rc = qlnx_remove_all_mcast_mac(ha); + rc = qlnx_set_multi(ha); if (rc) return rc;home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?69862c08.27044.1d324723>