netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net] i40e: When removing VF MAC filters, only check PF-set MAC
@ 2025-06-24 23:29 Jamie Bainbridge
  2025-06-25  9:07 ` Simon Horman
  0 siblings, 1 reply; 3+ messages in thread
From: Jamie Bainbridge @ 2025-06-24 23:29 UTC (permalink / raw)
  To: Simon Horman, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Michal Schmidt, Brett Creeley, Ivan Vecera
  Cc: Jamie Bainbridge, intel-wired-lan, netdev, linux-kernel

When the PF is processing an Admin Queue message to delete a VF's MACs
from the MAC filter, we currently check if the PF set the MAC and if
the VF is trusted.

This results in undesirable behaviour, where if a trusted VF with a
PF-set MAC sets itself down (which sends an AQ message to delete the
VF's MAC filters) then the VF MAC is erased from the interface.

This results in the VF losing its PF-set MAC which should not happen.

There is no need to check for trust at all, because an untrusted VF
cannot change its own MAC. The only check needed is whether the PF set
the MAC. If the PF set the MAC, then don't erase the MAC on link-down.

Resolve this by changing the deletion check only for PF-set MAC.

(the out-of-tree driver has also intentionally removed the check for VF
trust here with OOT driver version 2.26.8, this changes the Linux kernel
driver behaviour and comment to match the OOT driver behaviour)

Fixes: ea2a1cfc3b201 ("i40e: Fix VF MAC filter removal")
Signed-off-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>
---
v2: Reword commit message as suggested by Simon Horman.
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 88e6bef69342c2e65d8d5b2d7df5ac2cc32ee3d9..45ccbb1cdda0a33527852096ee9759fc19db3a5d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -3137,10 +3137,10 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
 		const u8 *addr = al->list[i].addr;
 
 		/* Allow to delete VF primary MAC only if it was not set
-		 * administratively by PF or if VF is trusted.
+		 * administratively by PF.
 		 */
 		if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
-			if (i40e_can_vf_change_mac(vf))
+			if (!vf->pf_set_mac)
 				was_unimac_deleted = true;
 			else
 				continue;
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 net] i40e: When removing VF MAC filters, only check PF-set MAC
  2025-06-24 23:29 [PATCH v2 net] i40e: When removing VF MAC filters, only check PF-set MAC Jamie Bainbridge
@ 2025-06-25  9:07 ` Simon Horman
  2025-07-21  9:42   ` [Intel-wired-lan] " Romanowski, Rafal
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2025-06-25  9:07 UTC (permalink / raw)
  To: Jamie Bainbridge
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Michal Schmidt,
	Brett Creeley, Ivan Vecera, intel-wired-lan, netdev, linux-kernel

On Wed, Jun 25, 2025 at 09:29:18AM +1000, Jamie Bainbridge wrote:
> When the PF is processing an Admin Queue message to delete a VF's MACs
> from the MAC filter, we currently check if the PF set the MAC and if
> the VF is trusted.
> 
> This results in undesirable behaviour, where if a trusted VF with a
> PF-set MAC sets itself down (which sends an AQ message to delete the
> VF's MAC filters) then the VF MAC is erased from the interface.
> 
> This results in the VF losing its PF-set MAC which should not happen.
> 
> There is no need to check for trust at all, because an untrusted VF
> cannot change its own MAC. The only check needed is whether the PF set
> the MAC. If the PF set the MAC, then don't erase the MAC on link-down.
> 
> Resolve this by changing the deletion check only for PF-set MAC.
> 
> (the out-of-tree driver has also intentionally removed the check for VF
> trust here with OOT driver version 2.26.8, this changes the Linux kernel
> driver behaviour and comment to match the OOT driver behaviour)
> 
> Fixes: ea2a1cfc3b201 ("i40e: Fix VF MAC filter removal")
> Signed-off-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>
> ---
> v2: Reword commit message as suggested by Simon Horman.

Thanks for the update.

Reviewed-by: Simon Horman <horms@kernel.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [Intel-wired-lan] [PATCH v2 net] i40e: When removing VF MAC filters, only check PF-set MAC
  2025-06-25  9:07 ` Simon Horman
@ 2025-07-21  9:42   ` Romanowski, Rafal
  0 siblings, 0 replies; 3+ messages in thread
From: Romanowski, Rafal @ 2025-07-21  9:42 UTC (permalink / raw)
  To: Simon Horman, Jamie Bainbridge
  Cc: Vecera, Ivan, Brett Creeley, Kitszel, Przemyslaw,
	linux-kernel@vger.kernel.org, Andrew Lunn, Eric Dumazet,
	netdev@vger.kernel.org, Nguyen, Anthony L,
	intel-wired-lan@lists.osuosl.org, Jakub Kicinski, Paolo Abeni,
	David S. Miller

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Simon
> Horman
> Sent: Wednesday, June 25, 2025 11:08 AM
> To: Jamie Bainbridge <jamie.bainbridge@gmail.com>
> Cc: Vecera, Ivan <ivecera@redhat.com>; Brett Creeley
> <brett.creeley@amd.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>;
> linux-kernel@vger.kernel.org; Andrew Lunn <andrew+netdev@lunn.ch>; Eric
> Dumazet <edumazet@google.com>; netdev@vger.kernel.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; intel-wired-lan@lists.osuosl.org; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David S. Miller
> <davem@davemloft.net>
> Subject: Re: [Intel-wired-lan] [PATCH v2 net] i40e: When removing VF MAC
> filters, only check PF-set MAC
> 
> On Wed, Jun 25, 2025 at 09:29:18AM +1000, Jamie Bainbridge wrote:
> > When the PF is processing an Admin Queue message to delete a VF's MACs
> > from the MAC filter, we currently check if the PF set the MAC and if
> > the VF is trusted.
> >
> > This results in undesirable behaviour, where if a trusted VF with a
> > PF-set MAC sets itself down (which sends an AQ message to delete the
> > VF's MAC filters) then the VF MAC is erased from the interface.
> >
> > This results in the VF losing its PF-set MAC which should not happen.
> >
> > There is no need to check for trust at all, because an untrusted VF
> > cannot change its own MAC. The only check needed is whether the PF set
> > the MAC. If the PF set the MAC, then don't erase the MAC on link-down.
> >
> > Resolve this by changing the deletion check only for PF-set MAC.
> >
> > (the out-of-tree driver has also intentionally removed the check for
> > VF trust here with OOT driver version 2.26.8, this changes the Linux
> > kernel driver behaviour and comment to match the OOT driver behaviour)
> >
> > Fixes: ea2a1cfc3b201 ("i40e: Fix VF MAC filter removal")
> > Signed-off-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>
> > ---
> > v2: Reword commit message as suggested by Simon Horman.
> 
> Thanks for the update.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>


Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-21  9:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-24 23:29 [PATCH v2 net] i40e: When removing VF MAC filters, only check PF-set MAC Jamie Bainbridge
2025-06-25  9:07 ` Simon Horman
2025-07-21  9:42   ` [Intel-wired-lan] " Romanowski, Rafal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).