Re: [PATCH net v6 4/4] macsec: Support VLAN-filtering lower devices

[Sabrina Dubroca <sd@queasysnail.net>]

2026-04-08, 10:24:28 +0000, Cosmin Ratiu wrote:
> On Wed, 2026-04-08 at 10:25 +0200, Sabrina Dubroca wrote:
> > 2026-04-07, 15:07:47 +0000, Cosmin Ratiu wrote:
> > > On Thu, 2026-04-02 at 16:48 +0200, Sabrina Dubroca wrote:
> > > > If this happens on a real device, the VLAN filters will be
> > > > broken.
> > > > I'm
> > > > not sure what the right behavior would be:
> > > > 
> > > > 1. reject the request to enable offload
> > > > 2. switch to promiscuous mode
> > > 
> > > I implemented and tested option 1. In the unlikely scenario adding
> > > VLAN
> > > filters prevents offloading
> > 
> > How unlikely is it? Resource allocation, talking to HW, device
> > limits,
> > anything else that could fail?
> > 
> > > , it's better for the driver to be explicit
> > > and let the user turn on promisc mode themselves. Keeping track of
> > > whether VLAN filters failed and promisc was used as a fallback adds
> > > some extra complexity.
> > 
> > If it's indeed (very) unlikely, sure. There's still a bit of a
> > regression/change of behavior for users compared to before the
> > IFF_UNICAST_FLT patch, but I think we can wait until someone
> > complains, and then add the tracking if that happens.
> > 
> > > What would be the point of IFF_UNICAST_FLT then?
> > 
> > The point is that this would just be a fallback.
> > 
> > > Please let me know if you agree with this approach, so I can send
> > > v8
> > > with it.
> > 
> > If you can confirm it's on the "very unlikely" side, yes, this
> > approach sounds ok. Thanks.
> 
> There are many, many drivers which implement .ndo_vlan_rx_add_vid, I
> can't really browse them all and determine overall likelihood of

Sorry, I wasn't requesting that much work, more of a "feeling" based
on your understanding. Anyway for macsec offload we don't have to care
about the vast majority of drivers.

> failure. But presumably there are some memory allocations involved and
> some FW communication to add the new filter.

I guess if FW communication fails we're in trouble anyway [*], and memory
allocation failure is an acceptable reason to reject offloading
anyway.

[*] rolling back the mdo_add_secy could be a problem if we had
difficulties talking to the HW?

> An AI prompt of "Explore implementations of .ndo_vlan_rx_add_vid in
> drivers and classify potential failure sources and their likelihood of
> failure." resulted in (edited for compactness):
> """
> Analyzed 24+ implementations across major drivers. Here's the
> breakdown:  
> Category   Likelihood Drivers                         Typical Error
> HW/FW cmds Low        mlx4, nfp, hinic, qeth, vmxnet3 Device-specific
> Mem alloc  Very Low   bnx2x, qede, virtio_net         -ENOMEM
> Filter add Low-Medium mlx5, mlx4, ice, qede, ocelot   -ENOSPC, -EEXIST
> Locking    Very Low   be2net, mlx4, nfp, hinic        -EBUSY
> Res limit  Medium     qede (quota check)              Promisc fallback
> 
> Three Driver Archetypes  
> 
> 1. Simple bitfield drivers (never fail): e1000, e1000e, ixgbe, igb —
> just set a VFTA bit in hardware registers, always return 0.
> 
> 2. Complex offload drivers (can fail): mlx5 (flow steering rules), mlx4
> (FW VLAN registration), ice (switch rules + promisc management), qede
> (quota-aware with promisc fallback), nfp (mailbox commands), hinic (FW
> commands with rollback).
> 
> 3. Delegation/passthrough drivers (inherit failures): bonding, team,
> hsr, dsa, macvlan, ipvlan, macsec — propagate vlan_vid_add() to lower
> devices with unwind-on-failure patterns.
> """
> 
> So I guess we go with option 1 for now.

ACK

-- 
Sabrina