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

[Cosmin Ratiu <cratiu@nvidia.com>]

On Fri, 2026-01-09 at 13:06 +0100, Sabrina Dubroca wrote:
> 2026-01-09, 11:38:59 +0000, Cosmin Ratiu wrote:
> > On Fri, 2026-01-09 at 11:26 +0100, Sabrina Dubroca wrote:
> > > 2026-01-07, 12:47:23 +0200, Cosmin Ratiu wrote:
> > > > VLAN-filtering is done through two netdev features
> > > > (NETIF_F_HW_VLAN_CTAG_FILTER and NETIF_F_HW_VLAN_STAG_FILTER)
> > > > and
> > > > two
> > > > netdev ops (ndo_vlan_rx_add_vid and ndo_vlan_rx_kill_vid).
> > > > 
> > > > Implement these and advertise the features if the lower device
> > > > supports
> > > > them. This allows proper VLAN filtering to work on top of
> > > > macsec
> > > > devices, when the lower device is capable of VLAN filtering.
> > > > As a concrete example, having this chain of interfaces now
> > > > works:
> > > > vlan_filtering_capable_dev(1) -> macsec_dev(2) ->
> > > > macsec_vlan_dev(3)
> > > > 
> > > > Before the "Fixes" commit this used to accidentally work
> > > > because
> > > > the
> > > > macsec device (and thus the lower device) was put in
> > > > promiscuous
> > > > mode
> > > > and the VLAN filter was not used. But after that commit
> > > > correctly
> > > > made
> > > > the macsec driver expose the IFF_UNICAST_FLT flag, promiscuous
> > > > mode
> > > > was
> > > > no longer used and VLAN filters on dev 1 kicked in. Without
> > > > support
> > > > in
> > > > dev 2 for propagating VLAN filters down, the register_vlan_dev
> > > > ->
> > > > vlan_vid_add -> __vlan_vid_add -> vlan_add_rx_filter_info call
> > > > from
> > > > dev
> > > > 3 is silently eaten (because vlan_hw_filter_capable returns
> > > > false
> > > > and
> > > > vlan_add_rx_filter_info silently succeeds).
> > > 
> > > We only want to propagate VLAN filters when macsec offload is
> > > used,
> > > no? If offload isn't used, the lower device should be unaware of
> > > whatever is happening on top of macsec, so I don't think non-
> > > offloaded
> > > setups are affected by this?
> > 
> > VLAN filters are not related to macsec offload, right? It's about
> > informing the lower netdevice which VLANs should be allowed.
> > Without
> > this patch, the VLAN-tagged packets intended for the macsec vlan
> > device
> > are discarded by the lower device VLAN filter.
> 
> Why does the lower device need to know in the non-offload case? It
> has
> no idea whether it's VLAN traffic or anything else once it's stuffed
> into macsec.
> 
> The packet will look like
> 
> ETH | MACSEC | [some opaque data that may or may not start with a
> VLAN header ]

You're right, I checked the failure and it happens only when offloads
are enabled.

> > > Even when offload is used, the lower device should probably
> > > handle
> > > "ETH + VLAN 5" differently from "ETH + MACSEC + VLAN 5", but that
> > > may
> > > not be possible with just the existing device ops.
> > 
> > I don't see how macsec plays a role into how the lower device
> > handles
> > VLANs. From the protocol diagrams, I see that it's ETH + VLAN 5 +
> > MACSEC, the VLAN isn't encrypted if present.
> 
> Wait, if we're talking about ETH + VLAN 5 + MACSEC, macsec shouldn't
> even be involved in VLAN id 5.
> 
> ip link add link eth0 type vlan id 5
> 
> should never go through any macsec code at all.
> 

These are the interfaces:
ip link add link $LOWER_DEV macsec0 type macsec sci ...
ip macsec offload macsec0 mac
ip link add link macsec0 name macsec_vlan type vlan id 5

What happens is that without the VLAN filter configured correctly, the
hw on the rx side decrypts and decapsulates macsec packets but drops
them shorty after.

Would you like to see any tweaks to the proposed patch?

Cosmin.