netdev - Re: Fix promiscous and multicast mode on iavf after reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8236bef5-d1e3-42ab-ba1f-b1d89f305d0a@intel.com>
Date: Thu, 24 Apr 2025 14:59:38 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Lennart Sorensen <lsorense@...lub.uwaterloo.ca>,
	<intel-wired-lan@...ts.osuosl.org>
CC: Przemek Kitszel <przemyslaw.kitszel@...el.com>, Tony Nguyen
	<anthony.l.nguyen@...el.com>, LKML <linux-kernel@...r.kernel.org>, Netdev
	<netdev@...r.kernel.org>
Subject: Re: Fix promiscous and multicast mode on iavf after reset



On 4/23/2025 10:12 AM, Lennart Sorensen wrote:
> I discovered that anything that causes a reset in iavf makes breaks
> promiscous mode and multicast.  This is because the host side ice
> driver clears the VF from filters when it is reset.  iavf then correctly
> calls iavf_configure, but since the current_netdev_promisc_flags already
> match the netdev promisc settings, nothing is done, so the promisc and
> multicast settings are not sent to the ice host driver after the reset.
> As a result the iavf side shows promisc enabled but it isn't working.
> Disabling and re-enabling promisc on the iavf side fixes it of course.
> Simple test case to show this is to enable promisc, check that packets
> are being seen, then change the mtu size (which does a reset) and check
> packets received again, and promisc is no longer active.  Disabling
> promisc and enabling it again restores receiving the packets.
> 
> The following seems to work for me, but I am not sure it is the correct
> place to clear the saved flags.
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 6d7ba4d67a19..4018a08d63c1 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -3233,6 +3233,14 @@ static void iavf_reset_task(struct work_struct *work)
>  		iavf_shutdown_adminq(hw);
>  		iavf_init_adminq(hw);
>  		iavf_request_reset(adapter);
> +
> +		/* Clear remembered promisc and multicast flags since
> +		 * reset clears them on the host so they will get force
> +		 * applied again through iavf_configure() down below.
> +		 */
> +		spin_lock_bh(&adapter->current_netdev_promisc_flags_lock);
> +		adapter->current_netdev_promisc_flags &= ~(IFF_PROMISC | IFF_ALLMULTI);
> +		spin_unlock_bh(&adapter->current_netdev_promisc_flags_lock);
>  	}
>  	adapter->flags |= IAVF_FLAG_RESET_PENDING;
>  
> 

We probably need to do something similar in the flow where we get an
unexpected reset (such as if PF resets us by changing trusted flag or
other state).

I don't think there's a better solution. Arguably the PF shouldn't be
losing data, but I think its a bit late to go that route at this point..
Its pretty baked into the virtchnl API :(