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

[Cosmin Ratiu <cratiu@nvidia.com>]

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
failure. But presumably there are some memory allocations involved and
some FW communication to add the new filter.
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.

> 
> > > OTOH maybe we don't need to care, since __netdev_update_features
> > > also
> > > (kind of) ignores those errors:
> > > 
> [...]
> > 
> > Well, in this case we have the chance to do something nicer (even
> > proper error message back to the user via extack) for a small
> 
> (That reminds me I have a branch of "macsec: add extack to X" patches
> I still need to polish and submit. I don't think I'll get to it
> before
> the merge window opens, I'll rebase on top of your changes.)
> 
> > complexity cost. Perhaps the VLAN filter handling could be improved
> > separately.
> 
> I'm not sure if this would be an "improvement to VLAN filter
> handling"
> or a "(breaking) change of user-visible behavior". Probably
> improvement. Or maybe it's "unlikely enough" that nobody has ever
> cared.
>