* [PATCH net] macsec: Support VLAN-filtering lower devices
@ 2026-01-07 10:47 Cosmin Ratiu
2026-01-09 10:26 ` Sabrina Dubroca
0 siblings, 1 reply; 9+ messages in thread
From: Cosmin Ratiu @ 2026-01-07 10:47 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Cosmin Ratiu, Dragos Tatulea
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).
Fixes: 0349659fd72f ("macsec: set IFF_UNICAST_FLT priv flag")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
drivers/net/macsec.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 5200fd5a10e5..bdb9b33970a6 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3486,7 +3486,8 @@ static netdev_tx_t macsec_start_xmit(struct sk_buff *skb,
}
#define MACSEC_FEATURES \
- (NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST)
+ (NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
+ NETIF_F_HW_VLAN_STAG_FILTER | NETIF_F_HW_VLAN_CTAG_FILTER)
#define MACSEC_OFFLOAD_FEATURES \
(MACSEC_FEATURES | NETIF_F_GSO_SOFTWARE | NETIF_F_SOFT_FEATURES | \
@@ -3707,6 +3708,23 @@ static int macsec_set_mac_address(struct net_device *dev, void *p)
return err;
}
+static int macsec_vlan_rx_add_vid(struct net_device *dev,
+ __be16 proto, u16 vid)
+{
+ struct macsec_dev *macsec = netdev_priv(dev);
+
+ return vlan_vid_add(macsec->real_dev, proto, vid);
+}
+
+static int macsec_vlan_rx_kill_vid(struct net_device *dev,
+ __be16 proto, u16 vid)
+{
+ struct macsec_dev *macsec = netdev_priv(dev);
+
+ vlan_vid_del(macsec->real_dev, proto, vid);
+ return 0;
+}
+
static int macsec_change_mtu(struct net_device *dev, int new_mtu)
{
struct macsec_dev *macsec = macsec_priv(dev);
@@ -3748,6 +3766,8 @@ static const struct net_device_ops macsec_netdev_ops = {
.ndo_set_rx_mode = macsec_dev_set_rx_mode,
.ndo_change_rx_flags = macsec_dev_change_rx_flags,
.ndo_set_mac_address = macsec_set_mac_address,
+ .ndo_vlan_rx_add_vid = macsec_vlan_rx_add_vid,
+ .ndo_vlan_rx_kill_vid = macsec_vlan_rx_kill_vid,
.ndo_start_xmit = macsec_start_xmit,
.ndo_get_stats64 = macsec_get_stats64,
.ndo_get_iflink = macsec_get_iflink,
--
2.45.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net] macsec: Support VLAN-filtering lower devices
2026-01-07 10:47 [PATCH net] macsec: Support VLAN-filtering lower devices Cosmin Ratiu
@ 2026-01-09 10:26 ` Sabrina Dubroca
2026-01-09 11:38 ` Cosmin Ratiu
0 siblings, 1 reply; 9+ messages in thread
From: Sabrina Dubroca @ 2026-01-09 10:26 UTC (permalink / raw)
To: Cosmin Ratiu
Cc: netdev, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Dragos Tatulea
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?
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.
--
Sabrina
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] macsec: Support VLAN-filtering lower devices
2026-01-09 10:26 ` Sabrina Dubroca
@ 2026-01-09 11:38 ` Cosmin Ratiu
2026-01-09 12:06 ` Sabrina Dubroca
0 siblings, 1 reply; 9+ messages in thread
From: Cosmin Ratiu @ 2026-01-09 11:38 UTC (permalink / raw)
To: sd@queasysnail.net
Cc: Dragos Tatulea, kuba@kernel.org, edumazet@google.com,
netdev@vger.kernel.org, andrew+netdev@lunn.ch, pabeni@redhat.com,
davem@davemloft.net
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.
> 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.
Cosmin.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] macsec: Support VLAN-filtering lower devices
2026-01-09 11:38 ` Cosmin Ratiu
@ 2026-01-09 12:06 ` Sabrina Dubroca
2026-01-09 13:50 ` Cosmin Ratiu
0 siblings, 1 reply; 9+ messages in thread
From: Sabrina Dubroca @ 2026-01-09 12:06 UTC (permalink / raw)
To: Cosmin Ratiu
Cc: Dragos Tatulea, kuba@kernel.org, edumazet@google.com,
netdev@vger.kernel.org, andrew+netdev@lunn.ch, pabeni@redhat.com,
davem@davemloft.net
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 ]
> > 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.
--
Sabrina
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] macsec: Support VLAN-filtering lower devices
2026-01-09 12:06 ` Sabrina Dubroca
@ 2026-01-09 13:50 ` Cosmin Ratiu
2026-01-10 22:45 ` Sabrina Dubroca
0 siblings, 1 reply; 9+ messages in thread
From: Cosmin Ratiu @ 2026-01-09 13:50 UTC (permalink / raw)
To: sd@queasysnail.net
Cc: pabeni@redhat.com, davem@davemloft.net, Dragos Tatulea,
kuba@kernel.org, netdev@vger.kernel.org, edumazet@google.com,
andrew+netdev@lunn.ch
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net] macsec: Support VLAN-filtering lower devices
2026-01-09 13:50 ` Cosmin Ratiu
@ 2026-01-10 22:45 ` Sabrina Dubroca
2026-01-12 10:32 ` Cosmin Ratiu
0 siblings, 1 reply; 9+ messages in thread
From: Sabrina Dubroca @ 2026-01-10 22:45 UTC (permalink / raw)
To: Cosmin Ratiu
Cc: pabeni@redhat.com, davem@davemloft.net, Dragos Tatulea,
kuba@kernel.org, netdev@vger.kernel.org, edumazet@google.com,
andrew+netdev@lunn.ch
2026-01-09, 13:50:24 +0000, Cosmin Ratiu wrote:
> 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.
Ok, thanks.
> > > > 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
Ok, that's what I was expecting.
(so it's not "ETH + VLAN 5 + MACSEC", either there was a typo or the
"protocol diagrams" you mentioned above are incorrect)
> 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.
Right.
> Would you like to see any tweaks to the proposed patch?
Well, updating the lower device's VLAN filters when not using offload
is undesireable, so macsec_vlan_rx_{add,kill}_vid should check that
offload is used, but then we'd have to remove/re-add then when offload
is toggled after some vlan devices have been created on top of the
macsec device. Keeping track of all the ids we've pushed down via
macsec_vlan_rx_add_vid seems a bit unreasonable, but maybe we can
call vlan_{get,drop}_rx_*_filter_info when we toggle macsec offload?
(not sure if that will have the behavior we want)
--
Sabrina
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net] macsec: Support VLAN-filtering lower devices
2026-01-10 22:45 ` Sabrina Dubroca
@ 2026-01-12 10:32 ` Cosmin Ratiu
2026-01-13 14:47 ` Sabrina Dubroca
0 siblings, 1 reply; 9+ messages in thread
From: Cosmin Ratiu @ 2026-01-12 10:32 UTC (permalink / raw)
To: sd@queasysnail.net
Cc: edumazet@google.com, andrew+netdev@lunn.ch, Dragos Tatulea,
davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
netdev@vger.kernel.org
On Sat, 2026-01-10 at 23:45 +0100, Sabrina Dubroca wrote:
> 2026-01-09, 13:50:24 +0000, Cosmin Ratiu wrote:
>
> > Would you like to see any tweaks to the proposed patch?
>
> Well, updating the lower device's VLAN filters when not using offload
> is undesireable, so macsec_vlan_rx_{add,kill}_vid should check that
> offload is used, but then we'd have to remove/re-add then when
> offload
> is toggled after some vlan devices have been created on top of the
> macsec device. Keeping track of all the ids we've pushed down via
> macsec_vlan_rx_add_vid seems a bit unreasonable, but maybe we can
> call vlan_{get,drop}_rx_*_filter_info when we toggle macsec offload?
> (not sure if that will have the behavior we want)
>
Perhaps "undesirable" is too strong of a word. I would use "unneeded".
Having the encrypted VLANs in the lower dev HW filter can't do too much
harm, except maybe allowing some non-macsec packets with those vlans
when previously they wouldn't be allowed.
But remember what happened before the mentioned "Fixes" patch: the
lower device was put in promisc mode because it didn't advertise
IFF_UNICAST_FLT so it would have received all packets anyway.
So this fix is strictly better, simple enough that it can be understood
to be harmless.
The vlan_{get,drop}_rx_*_filter_info functions simply call device
notifiers when the VLAN filter flags change, they're not useful for
obtaining the list of VLANs. The upper devs keep track of those.
If I engineer the fix we're discussing here (which would make macsec
keep track of VLANs), it would be significantly more complicated, and
it belonging into net instead of net-next could be called into
question.
Cosmin.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net] macsec: Support VLAN-filtering lower devices
2026-01-12 10:32 ` Cosmin Ratiu
@ 2026-01-13 14:47 ` Sabrina Dubroca
2026-01-22 12:15 ` Cosmin Ratiu
0 siblings, 1 reply; 9+ messages in thread
From: Sabrina Dubroca @ 2026-01-13 14:47 UTC (permalink / raw)
To: Cosmin Ratiu
Cc: edumazet@google.com, andrew+netdev@lunn.ch, Dragos Tatulea,
davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
netdev@vger.kernel.org
2026-01-12, 10:32:17 +0000, Cosmin Ratiu wrote:
> On Sat, 2026-01-10 at 23:45 +0100, Sabrina Dubroca wrote:
> > 2026-01-09, 13:50:24 +0000, Cosmin Ratiu wrote:
> >
> > > Would you like to see any tweaks to the proposed patch?
> >
> > Well, updating the lower device's VLAN filters when not using offload
> > is undesireable, so macsec_vlan_rx_{add,kill}_vid should check that
> > offload is used, but then we'd have to remove/re-add then when
> > offload
> > is toggled after some vlan devices have been created on top of the
> > macsec device. Keeping track of all the ids we've pushed down via
> > macsec_vlan_rx_add_vid seems a bit unreasonable, but maybe we can
> > call vlan_{get,drop}_rx_*_filter_info when we toggle macsec offload?
> > (not sure if that will have the behavior we want)
> >
>
> Perhaps "undesirable" is too strong of a word. I would use "unneeded".
> Having the encrypted VLANs in the lower dev HW filter can't do too much
> harm, except maybe allowing some non-macsec packets with those vlans
> when previously they wouldn't be allowed.
Well, if an admin has the filters working and suddenly starts seeing
packets that should have been filtered out, they may get confused.
> But remember what happened before the mentioned "Fixes" patch: the
> lower device was put in promisc mode because it didn't advertise
> IFF_UNICAST_FLT so it would have received all packets anyway.
> So this fix is strictly better, simple enough that it can be understood
> to be harmless.
Ok.
> The vlan_{get,drop}_rx_*_filter_info functions simply call device
> notifiers when the VLAN filter flags change, they're not useful for
> obtaining the list of VLANs. The upper devs keep track of those.
But that notifier gets caught in vlan_device_event, which calls
vlan_filter_push_vids. That iterates all existing vlans and pushes
them into the real device. That's why I thought it might work here,
but I haven't tried.
> If I engineer the fix we're discussing here (which would make macsec
> keep track of VLANs), it would be significantly more complicated, and
> it belonging into net instead of net-next could be called into
> question.
Sure, if we have to implement all that in macsec, I would agree to
take the current patch, and do the tracking later in net-next. In that
case, please just add a note to the commit message about the offload
vs non-offload behavior we're discussing here.
Either way, it would also be good to add some selftests.
--
Sabrina
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net] macsec: Support VLAN-filtering lower devices
2026-01-13 14:47 ` Sabrina Dubroca
@ 2026-01-22 12:15 ` Cosmin Ratiu
0 siblings, 0 replies; 9+ messages in thread
From: Cosmin Ratiu @ 2026-01-22 12:15 UTC (permalink / raw)
To: sd@queasysnail.net
Cc: pabeni@redhat.com, netdev@vger.kernel.org, edumazet@google.com,
andrew+netdev@lunn.ch, davem@davemloft.net, Dragos Tatulea,
kuba@kernel.org
On Tue, 2026-01-13 at 15:47 +0100, Sabrina Dubroca wrote:
> 2026-01-12, 10:32:17 +0000, Cosmin Ratiu wrote:
> > Perhaps "undesirable" is too strong of a word. I would use
> > "unneeded".
> > Having the encrypted VLANs in the lower dev HW filter can't do too
> > much
> > harm, except maybe allowing some non-macsec packets with those
> > vlans
> > when previously they wouldn't be allowed.
>
> Well, if an admin has the filters working and suddenly starts seeing
> packets that should have been filtered out, they may get confused.
>
> > But remember what happened before the mentioned "Fixes" patch: the
> > lower device was put in promisc mode because it didn't advertise
> > IFF_UNICAST_FLT so it would have received all packets anyway.
> > So this fix is strictly better, simple enough that it can be
> > understood
> > to be harmless.
>
> Ok.
>
> > The vlan_{get,drop}_rx_*_filter_info functions simply call device
> > notifiers when the VLAN filter flags change, they're not useful for
> > obtaining the list of VLANs. The upper devs keep track of those.
>
> But that notifier gets caught in vlan_device_event, which calls
> vlan_filter_push_vids. That iterates all existing vlans and pushes
> them into the real device. That's why I thought it might work here,
> but I haven't tried.
Looked over this, and I think it's exactly what's needed.
I'll get back with an updated patch hopefully soon.
> > If I engineer the fix we're discussing here (which would make
> > macsec
> > keep track of VLANs), it would be significantly more complicated,
> > and
> > it belonging into net instead of net-next could be called into
> > question.
>
> Sure, if we have to implement all that in macsec, I would agree to
> take the current patch, and do the tracking later in net-next. In
> that
> case, please just add a note to the commit message about the offload
> vs non-offload behavior we're discussing here.
>
> Either way, it would also be good to add some selftests.
Will add.
Cosmin.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-01-22 12:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-07 10:47 [PATCH net] macsec: Support VLAN-filtering lower devices Cosmin Ratiu
2026-01-09 10:26 ` Sabrina Dubroca
2026-01-09 11:38 ` Cosmin Ratiu
2026-01-09 12:06 ` Sabrina Dubroca
2026-01-09 13:50 ` Cosmin Ratiu
2026-01-10 22:45 ` Sabrina Dubroca
2026-01-12 10:32 ` Cosmin Ratiu
2026-01-13 14:47 ` Sabrina Dubroca
2026-01-22 12:15 ` Cosmin Ratiu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox