Re: [PATCH net-next v2 3/3] macsec: Support VLAN-filtering lower devices

[Sabrina Dubroca <sd@queasysnail.net>]

2026-02-27, 06:59:08 -0800, Jakub Kicinski wrote:
> On Fri, 27 Feb 2026 11:02:27 +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 commit [1] this used to accidentally work because the macsec

The first submission was for net-next, then Jakub asked to make this a
fix for net:
https://lore.kernel.org/netdev/20260106171027.57a7757f@kernel.org/
which makes sense.

So "v1"
https://lore.kernel.org/netdev/20260107104723.2750725-1-cratiu@nvidia.com
was for net, and we discussed making it a net-next patch because the
changes were looking invasive. But in the end by using
vlan_{get,drop}_rx_*_filter_info it's a pretty simple patch, so I
think this should still be for net?


[...]
> > @@ -2616,7 +2616,17 @@ static int macsec_update_offload(struct net_device *dev, enum macsec_offload off
> >  	if (!ops)
> >  		return -EOPNOTSUPP;
> >
> > +	/* Remove VLAN filters when disabling offload. */
> > +	if (offload == MACSEC_OFFLOAD_OFF) {
> > +		vlan_drop_rx_ctag_filter_info(dev);
> > +		vlan_drop_rx_stag_filter_info(dev);
> > +	}
> >  	macsec->offload = offload;
> > +	/* Add VLAN filters when enabling offload. */
> > +	if (prev_offload == MACSEC_OFFLOAD_OFF) {
> > +		vlan_get_rx_ctag_filter_info(dev);
> > +		vlan_get_rx_stag_filter_info(dev);
> > +	}
> >
> >  	ctx.secy = &macsec->secy;
> >  	ret = offload == MACSEC_OFFLOAD_OFF ? macsec_offload(ops->mdo_del_secy, &ctx)
> > @@ -2633,6 +2643,11 @@ static int macsec_update_offload(struct net_device *dev, enum macsec_offload off
> >
> >  	if (ret) {
> >  		macsec->offload = prev_offload;
> > +		if (offload == MACSEC_OFFLOAD_OFF && prev_offload == MACSEC_OFFLOAD_MAC) {
> > +			vlan_get_rx_ctag_filter_info(dev);
> > +			vlan_get_rx_stag_filter_info(dev);
> > +		}
> > +
> >  		return ret;
> >  	}
> 
> Does the error path properly restore VLAN filter state when enabling offload
> fails? When prev_offload is MACSEC_OFFLOAD_OFF and the code calls
> vlan_get_rx_ctag_filter_info(dev) and vlan_get_rx_stag_filter_info(dev) to
> push VLAN filters to the lower device, but then macsec_offload() fails and
> returns an error:
[...]

Looks like it. Since the vlan_*_filter_info ops can't fail, just move
them until after we've called mdo_*_secy, and macsec_update_offload
can't fail anymore?

-- 
Sabrina