Re: 38663adb6144 - main - Revert "ixl: fix multicast filters handling"

On Thu, Dec 19, 2024 at 09:08:26AM -0800, Ravi Pokala wrote:
> > Revert "ixl: fix multicast filters handling"
> 
> Yes, but *why*?

Per the linked pull request and bug report, it breaks igmp-proxy for an
unknown reason.

> -Ravi (rpokala@)
> 
> -----Original Message-----
> From: <owner-src-committers@freebsd.org <mailto:owner-src-committers@freebsd.org>> on behalf of Mark Johnston <markj@FreeBSD.org <mailto:markj@FreeBSD.org>>
> Date: Thursday, December 19, 2024 at 05:49
> To: <src-committers@FreeBSD.org <mailto:src-committers@FreeBSD.org>>, <dev-commits-src-all@FreeBSD.org <mailto:dev-commits-src-all@FreeBSD.org>>, <dev-commits-src-main@FreeBSD.org <mailto:dev-commits-src-main@FreeBSD.org>>
> Subject: git: 38663adb6144 - main - Revert "ixl: fix multicast filters handling"
> 
> 
> The branch main has been updated by markj:
> 
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=38663adb61440bd659fb457909782b71ba8806fa <https://cgit.FreeBSD.org/src/commit/?id=38663adb61440bd659fb457909782b71ba8806fa>
> 
> 
> commit 38663adb61440bd659fb457909782b71ba8806fa
> Author: Franco Fichtner <franco@opnsense.org <mailto:franco@opnsense.org>>
> AuthorDate: 2024-12-11 14:08:40 +0000
> Commit: Mark Johnston <markj@FreeBSD.org <mailto:markj@FreeBSD.org>>
> CommitDate: 2024-12-19 13:49:30 +0000
> 
> 
> Revert "ixl: fix multicast filters handling"
> 
> 
> This reverts commit 89e73359424a338a7900a4854ad7439f5848ebb8.
> 
> 
> PR: 281125
> Reviewed by: Krzysztof Galazka <krzysztof.galazka@intel.com <mailto:krzysztof.galazka@intel.com>>
> MFC after: 3 days
> Pull Request: https://github.com/freebsd/freebsd-src/pull/1545 <https://github.com/freebsd/freebsd-src/pull/1545>
> ---
> sys/dev/ixl/ixl_pf_main.c | 97 +++++------------------------------------------
> 1 file changed, 10 insertions(+), 87 deletions(-)
> 
> 
> diff --git a/sys/dev/ixl/ixl_pf_main.c b/sys/dev/ixl/ixl_pf_main.c
> index 9755136df848..1752efc02fff 100644
> --- a/sys/dev/ixl/ixl_pf_main.c
> +++ b/sys/dev/ixl/ixl_pf_main.c
> @@ -593,15 +593,6 @@ ixl_add_maddr(void *arg, struct sockaddr_dl *sdl, u_int cnt)
> * Routines for multicast and vlan filter management.
> *
> *********************************************************************/
> -
> -/**
> - * ixl_add_multi - Add multicast filters to the hardware
> - * @vsi: The VSI structure
> - *
> - * In case number of multicast filters in the IFP exceeds 127 entries,
> - * multicast promiscuous mode will be enabled and the filters will be removed
> - * from the hardware
> - */
> void
> ixl_add_multi(struct ixl_vsi *vsi)
> {
> @@ -609,20 +600,14 @@ ixl_add_multi(struct ixl_vsi *vsi)
> struct i40e_hw *hw = vsi->hw;
> int mcnt = 0;
> struct ixl_add_maddr_arg cb_arg;
> - enum i40e_status_code status;
> 
> 
> IOCTL_DEBUGOUT("ixl_add_multi: begin");
> 
> 
> mcnt = if_llmaddr_count(ifp);
> if (__predict_false(mcnt >= MAX_MULTICAST_ADDR)) {
> - status = i40e_aq_set_vsi_multicast_promiscuous(hw, vsi->seid,
> - TRUE, NULL);
> - if (status != I40E_SUCCESS)
> - if_printf(ifp, "Failed to enable multicast promiscuous "
> - "mode, status: %s\n", i40e_stat_str(hw, status));
> - else
> - if_printf(ifp, "Enabled multicast promiscuous mode\n");
> - /* Delete all existing MC filters */
> + i40e_aq_set_vsi_multicast_promiscuous(hw,
> + vsi->seid, TRUE, NULL);
> + /* delete all existing MC filters */
> ixl_del_multi(vsi, true);
> return;
> }
> @@ -648,92 +633,30 @@ ixl_match_maddr(void *arg, struct sockaddr_dl *sdl, u_int cnt)
> return (0);
> }
> 
> 
> -/**
> - * ixl_dis_multi_promisc - Disable multicast promiscuous mode
> - * @vsi: The VSI structure
> - * @vsi_mcnt: Number of multicast filters in the VSI
> - *
> - * Disable multicast promiscuous mode based on number of entries in the IFP
> - * and the VSI, then re-add multicast filters.
> - *
> - */
> -static void
> -ixl_dis_multi_promisc(struct ixl_vsi *vsi, int vsi_mcnt)
> -{
> - struct ifnet *ifp = vsi->ifp;
> - struct i40e_hw *hw = vsi->hw;
> - int ifp_mcnt = 0;
> - enum i40e_status_code status;
> -
> - ifp_mcnt = if_llmaddr_count(ifp);
> - /*
> - * Equal lists or empty ifp list mean the list has not been changed
> - * and in such case avoid disabling multicast promiscuous mode as it
> - * was not previously enabled. Case where multicast promiscuous mode has
> - * been enabled is when vsi_mcnt == 0 && ifp_mcnt > 0.
> - */
> - if (ifp_mcnt == vsi_mcnt || ifp_mcnt == 0 ||
> - ifp_mcnt >= MAX_MULTICAST_ADDR)
> - return;
> -
> - status = i40e_aq_set_vsi_multicast_promiscuous(hw, vsi->seid,
> - FALSE, NULL);
> - if (status != I40E_SUCCESS) {
> - if_printf(ifp, "Failed to disable multicast promiscuous "
> - "mode, status: %s\n", i40e_stat_str(hw, status));
> -
> - return;
> - }
> -
> - if_printf(ifp, "Disabled multicast promiscuous mode\n");
> -
> - ixl_add_multi(vsi);
> -}
> -
> -/**
> - * ixl_del_multi - Delete multicast filters from the hardware
> - * @vsi: The VSI structure
> - * @all: Bool to determine if all the multicast filters should be removed
> - *
> - * In case number of multicast filters in the IFP drops to 127 entries,
> - * multicast promiscuous mode will be disabled and the filters will be reapplied
> - * to the hardware.
> - */
> void
> ixl_del_multi(struct ixl_vsi *vsi, bool all)
> {
> - int to_del_cnt = 0, vsi_mcnt = 0;
> + struct ixl_ftl_head to_del;
> if_t ifp = vsi->ifp;
> struct ixl_mac_filter *f, *fn;
> - struct ixl_ftl_head to_del;
> + int mcnt = 0;
> 
> 
> IOCTL_DEBUGOUT("ixl_del_multi: begin");
> 
> 
> LIST_INIT(&to_del);
> /* Search for removed multicast addresses */
> LIST_FOREACH_SAFE(f, &vsi->ftl, ftle, fn) {
> - if ((f->flags & IXL_FILTER_MC) == 0)
> - continue;
> -
> - /* Count all the multicast filters in the VSI for comparison */
> - vsi_mcnt++;
> -
> - if (!all && if_foreach_llmaddr(ifp, ixl_match_maddr, f) != 0)
> + if ((f->flags & IXL_FILTER_MC) == 0 ||
> + (!all && (if_foreach_llmaddr(ifp, ixl_match_maddr, f) == 0)))
> continue;
> 
> 
> LIST_REMOVE(f, ftle);
> LIST_INSERT_HEAD(&to_del, f, ftle);
> - to_del_cnt++;
> - }
> -
> - if (to_del_cnt > 0) {
> - ixl_del_hw_filters(vsi, &to_del, to_del_cnt);
> - return;
> + mcnt++;
> }
> 
> 
> - ixl_dis_multi_promisc(vsi, vsi_mcnt);
> -
> - IOCTL_DEBUGOUT("ixl_del_multi: end");
> + if (mcnt > 0)
> + ixl_del_hw_filters(vsi, &to_del, mcnt);
> }
> 
> 
> void
> 
> 
> 
>