[PATCH net-next v2 0/3] macsec: Add support for VLAN filtering in offload mode

[PATCH net-next v2 0/3] macsec: Add support for VLAN filtering in offload mode

[Cosmin Ratiu]

This short series adds support for VLANs in macsec devices when offload
mode is enabled. This allows VLAN netdevs on top of macsec netdevs to
function, which accidentally used to be the case in the past, but was
broken. This series adds back proper support.

netdevsim was extended to support VLANs and there's also a selftest for
the new functionality.

V1->V2:
- Changed VLAN filtering to only function in offload mode. 
- Added tests.

Cosmin Ratiu (3):
  nsim: Add support for VLAN filters
  selftests: Add macsec offload VLAN tests
  macsec: Support VLAN-filtering lower devices

 drivers/net/macsec.c                          | 38 +++++++++++-
 drivers/net/netdevsim/netdev.c                | 59 ++++++++++++++++++-
 drivers/net/netdevsim/netdevsim.h             |  8 +++
 .../drivers/net/netdevsim/ethtool-common.sh   |  5 +-
 .../drivers/net/netdevsim/macsec-offload.sh   | 59 +++++++++++++++++++
 5 files changed, 164 insertions(+), 5 deletions(-)

-- 
2.49.0

[PATCH net-next v2 1/3] nsim: Add support for VLAN filters

[Cosmin Ratiu]

Add support for storing the list of VLANs in nsim devices, together with
ops for adding/removing them and a debug file to show them.

This will be used in upcoming tests.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
---
 drivers/net/netdevsim/netdev.c    | 59 +++++++++++++++++++++++++++++--
 drivers/net/netdevsim/netdevsim.h |  8 +++++
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 6285fbefe38a..7177711cf9fc 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -602,6 +602,36 @@ static int nsim_stop(struct net_device *dev)
 	return 0;
 }
 
+static int nsim_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	if (vid >= VLAN_N_VID)
+		return -EINVAL;
+
+	if (proto == htons(ETH_P_8021Q))
+		set_bit(vid, ns->vlan.ctag);
+	else if (proto == htons(ETH_P_8021AD))
+		set_bit(vid, ns->vlan.stag);
+
+	return 0;
+}
+
+static int nsim_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+
+	if (vid >= VLAN_N_VID)
+		return -EINVAL;
+
+	if (proto == htons(ETH_P_8021Q))
+		clear_bit(vid, ns->vlan.ctag);
+	else if (proto == htons(ETH_P_8021AD))
+		clear_bit(vid, ns->vlan.stag);
+
+	return 0;
+}
+
 static int nsim_shaper_set(struct net_shaper_binding *binding,
 			   const struct net_shaper *shaper,
 			   struct netlink_ext_ack *extack)
@@ -659,6 +689,8 @@ static const struct net_device_ops nsim_netdev_ops = {
 	.ndo_bpf		= nsim_bpf,
 	.ndo_open		= nsim_open,
 	.ndo_stop		= nsim_stop,
+	.ndo_vlan_rx_add_vid	= nsim_vlan_rx_add_vid,
+	.ndo_vlan_rx_kill_vid	= nsim_vlan_rx_kill_vid,
 	.net_shaper_ops		= &nsim_shaper_ops,
 };
 
@@ -670,6 +702,8 @@ static const struct net_device_ops nsim_vf_netdev_ops = {
 	.ndo_change_mtu		= nsim_change_mtu,
 	.ndo_setup_tc		= nsim_setup_tc,
 	.ndo_set_features	= nsim_set_features,
+	.ndo_vlan_rx_add_vid	= nsim_vlan_rx_add_vid,
+	.ndo_vlan_rx_kill_vid	= nsim_vlan_rx_kill_vid,
 };
 
 /* We don't have true per-queue stats, yet, so do some random fakery here.
@@ -967,6 +1001,20 @@ static const struct file_operations nsim_pp_hold_fops = {
 	.owner = THIS_MODULE,
 };
 
+static int nsim_vlan_show(struct seq_file *s, void *data)
+{
+	struct netdevsim *ns = s->private;
+	int vid;
+
+	for_each_set_bit(vid, ns->vlan.ctag, VLAN_N_VID)
+		seq_printf(s, "ctag %d\n", vid);
+	for_each_set_bit(vid, ns->vlan.stag, VLAN_N_VID)
+		seq_printf(s, "stag %d\n", vid);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(nsim_vlan);
+
 static void nsim_setup(struct net_device *dev)
 {
 	ether_setup(dev);
@@ -979,14 +1027,18 @@ static void nsim_setup(struct net_device *dev)
 			 NETIF_F_FRAGLIST |
 			 NETIF_F_HW_CSUM |
 			 NETIF_F_LRO |
-			 NETIF_F_TSO;
+			 NETIF_F_TSO |
+			 NETIF_F_HW_VLAN_CTAG_FILTER |
+			 NETIF_F_HW_VLAN_STAG_FILTER;
 	dev->hw_features |= NETIF_F_HW_TC |
 			    NETIF_F_SG |
 			    NETIF_F_FRAGLIST |
 			    NETIF_F_HW_CSUM |
 			    NETIF_F_LRO |
 			    NETIF_F_TSO |
-			    NETIF_F_LOOPBACK;
+			    NETIF_F_LOOPBACK |
+			    NETIF_F_HW_VLAN_CTAG_FILTER |
+			    NETIF_F_HW_VLAN_STAG_FILTER;
 	dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
 	dev->max_mtu = ETH_MAX_MTU;
 	dev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_HW_OFFLOAD;
@@ -1154,6 +1206,8 @@ struct netdevsim *nsim_create(struct nsim_dev *nsim_dev,
 	ns->qr_dfs = debugfs_create_file("queue_reset", 0200,
 					 nsim_dev_port->ddir, ns,
 					 &nsim_qreset_fops);
+	ns->vlan_dfs = debugfs_create_file("vlan", 0400, nsim_dev_port->ddir,
+					   ns, &nsim_vlan_fops);
 	return ns;
 
 err_free_netdev:
@@ -1166,6 +1220,7 @@ void nsim_destroy(struct netdevsim *ns)
 	struct net_device *dev = ns->netdev;
 	struct netdevsim *peer;
 
+	debugfs_remove(ns->vlan_dfs);
 	debugfs_remove(ns->qr_dfs);
 	debugfs_remove(ns->pp_dfs);
 
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index f767fc8a7505..f844c27ca78b 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -18,6 +18,7 @@
 #include <linux/ethtool.h>
 #include <linux/ethtool_netlink.h>
 #include <linux/kernel.h>
+#include <linux/if_vlan.h>
 #include <linux/list.h>
 #include <linux/netdevice.h>
 #include <linux/ptp_mock.h>
@@ -75,6 +76,11 @@ struct nsim_macsec {
 	u8 nsim_secy_count;
 };
 
+struct nsim_vlan {
+	DECLARE_BITMAP(ctag, VLAN_N_VID);
+	DECLARE_BITMAP(stag, VLAN_N_VID);
+};
+
 struct nsim_ethtool_pauseparam {
 	bool rx;
 	bool tx;
@@ -135,6 +141,7 @@ struct netdevsim {
 	bool bpf_map_accept;
 	struct nsim_ipsec ipsec;
 	struct nsim_macsec macsec;
+	struct nsim_vlan vlan;
 	struct {
 		u32 inject_error;
 		u32 __ports[2][NSIM_UDP_TUNNEL_N_PORTS];
@@ -146,6 +153,7 @@ struct netdevsim {
 	struct page *page;
 	struct dentry *pp_dfs;
 	struct dentry *qr_dfs;
+	struct dentry *vlan_dfs;
 
 	struct nsim_ethtool ethtool;
 	struct netdevsim __rcu *peer;
-- 
2.49.0

[PATCH net-next v2 2/3] selftests: Add macsec offload VLAN tests

[Cosmin Ratiu]

Add macsec offload VLAN tests using the netsim VLAN support just added.
In case that VLAN support is missing (e.g. when running these tests on
older kernels), the VLAN tests are simply skipped.

Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
---
 .../drivers/net/netdevsim/ethtool-common.sh   |  5 +-
 .../drivers/net/netdevsim/macsec-offload.sh   | 59 +++++++++++++++++++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
index 80160579e0cc..b80b88240883 100644
--- a/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-common.sh
@@ -30,16 +30,17 @@ function check {
 
     if [ $code $cop 0 ]; then
 	((num_errors++))
-	return
+	return 1
     fi
 
     if [ "$str" != "$exp_str"  ]; then
 	echo -e "Expected: '$exp_str', got '$str'"
 	((num_errors++))
-	return
+	return 1
     fi
 
     ((num_passes++))
+    return 0
 }
 
 function make_netdev {
diff --git a/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh b/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh
index 98033e6667d2..c4af47eec9fa 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/macsec-offload.sh
@@ -108,6 +108,65 @@ TMP_FEATS_ON_3="$(ethtool -k $MACSEC_NETDEV)"
 check $?
 
 
+ip link del $MACSEC_NETDEV
+
+
+#
+# test VLAN filter propagation through macsec
+#
+
+VLAN_DFS="$NSIM_DEV_DFS/vlan"
+
+check_vid() {
+    local vid=$1
+    local expected=$2
+
+    if grep -q "ctag $vid" "$VLAN_DFS" 2>/dev/null; then
+	present=1
+    else
+	present=0
+    fi
+    [ "$present" -eq "$expected" ]
+}
+
+# Skip VLAN tests if nsim doesn't support VLANs
+if [ -f $VLAN_DFS ]; then
+    ip link add link $NSIM_NETDEV $MACSEC_NETDEV type macsec offload mac
+    check $?
+    ip link add link $MACSEC_NETDEV ${MACSEC_NETDEV}.10 type vlan id 10
+    check $?
+    check_vid 10 1
+    check $? || echo "VID 10 should be on $MACSEC_NETDEV with offload ON"
+
+    ip link add link $NSIM_NETDEV ${MACSEC_NETDEV}2 type macsec port 5
+    check $?
+    ip link add link ${MACSEC_NETDEV}2 ${MACSEC_NETDEV}2.20 type vlan id 20
+    check $?
+    check_vid 20 0
+    check $? || echo "VID 20 should NOT be on $MACSEC_NETDEV2 with offload OFF"
+
+    ip link set ${MACSEC_NETDEV}2 type macsec offload mac
+    check $?
+    check_vid 20 1
+    check $? || echo "VID 20 should appear after offload ON"
+
+    ip link set ${MACSEC_NETDEV}2 type macsec offload off
+    check $?
+    check_vid 20 0
+    check $? || echo "VID 20 should disappear after offload OFF"
+
+    ip link del ${MACSEC_NETDEV}.10
+    check $?
+    check_vid 10 0
+    check $? || echo "VID 10 should be gone after VLAN delete with offload ON"
+
+    ip link del ${MACSEC_NETDEV}2.20
+    ip link del ${MACSEC_NETDEV}2
+    ip link del $MACSEC_NETDEV
+else
+    echo "SKIP: macsec VLAN tests, no netdevsim support."
+fi
+
 if [ $num_errors -eq 0 ]; then
     echo "PASSED all $((num_passes)) checks"
     exit 0
-- 
2.49.0

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

[Cosmin Ratiu]

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
device (and thus the lower device) was put in promiscuous mode and the
VLAN filter was not used. But after commit [1] 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).

For macsec, VLAN filters are only relevant for offload, otherwise
the VLANs are encrypted and the lower devices don't care about them. So
VLAN filters are only passed on to lower devices in offload mode.
Flipping between offload modes now needs to offload/unoffload the
filters with vlan_{get,drop}_rx_*_filter_info().

[1] commit 0349659fd72f ("macsec: set IFF_UNICAST_FLT priv flag")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
---
 drivers/net/macsec.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index c2cb2d20976b..27b36288c5e5 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -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)
@@ -3486,7 +3496,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 +3718,29 @@ 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);
+
+	if (!macsec_is_offloaded(macsec))
+		return 0;
+
+	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);
+
+	if (!macsec_is_offloaded(macsec))
+		return 0;
+
+	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 +3782,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.49.0

Re: [PATCH net-next v2 2/3] selftests: Add macsec offload VLAN tests

[Jakub Kicinski]

On Fri, 27 Feb 2026 11:02:26 +0200 Cosmin Ratiu wrote:
> Add macsec offload VLAN tests using the netsim VLAN support just added.
> In case that VLAN support is missing (e.g. when running these tests on
> older kernels), the VLAN tests are simply skipped.

You have to enable macsec in the config file for netdevsim selftests
-- 
pw-bot: cr

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

[Jakub Kicinski]

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
> device (and thus the lower device) was put in promiscuous mode and the
> VLAN filter was not used. But after commit [1] 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).
> 
> For macsec, VLAN filters are only relevant for offload, otherwise
> the VLANs are encrypted and the lower devices don't care about them. So
> VLAN filters are only passed on to lower devices in offload mode.
> Flipping between offload modes now needs to offload/unoffload the
> filters with vlan_{get,drop}_rx_*_filter_info().

AI says:

This commit implements VLAN-filtering support for macsec devices by
adding ndo_vlan_rx_add_vid and ndo_vlan_rx_kill_vid callbacks and
advertising the appropriate netdev features when the lower device
supports VLAN filtering.

> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
> index f6cad0746a02..73e0b75d9b69 100644
> --- a/drivers/net/macsec.c
> +++ b/drivers/net/macsec.c
> @@ -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:

	ctx.secy = &macsec->secy;
	ret = offload == MACSEC_OFFLOAD_OFF ? macsec_offload(ops->mdo_del_secy, &ctx)
					    : macsec_offload(ops->mdo_add_secy, &ctx);
	if (ret)
		goto rollback;

	...

rollback:
	if (ret) {
		macsec->offload = prev_offload;
		return ret;
	}

The error path restores macsec->offload to prev_offload (MACSEC_OFFLOAD_OFF)
but doesn't call vlan_drop_rx_ctag_filter_info(dev) and
vlan_drop_rx_stag_filter_info(dev) to remove the VLAN filters that were
already pushed to the lower device. This leaves stale VLAN filter entries on
real_dev.

Since macsec->offload is now back to MACSEC_OFFLOAD_OFF, future calls to
macsec_vlan_rx_kill_vid will return early without removing these VIDs, so the
leaked state persists until the lower device is brought down.

[ ... ]

Re: [PATCH net-next v2 1/3] nsim: Add support for VLAN filters

[Sabrina Dubroca]

2026-02-27, 11:02:25 +0200, Cosmin Ratiu wrote:
> Add support for storing the list of VLANs in nsim devices, together with
> ops for adding/removing them and a debug file to show them.
> 
> This will be used in upcoming tests.
> 
> Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> ---
>  drivers/net/netdevsim/netdev.c    | 59 +++++++++++++++++++++++++++++--
>  drivers/net/netdevsim/netdevsim.h |  8 +++++
>  2 files changed, 65 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index 6285fbefe38a..7177711cf9fc 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -602,6 +602,36 @@ static int nsim_stop(struct net_device *dev)
>  	return 0;
>  }
>  
> +static int nsim_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
> +{
> +	struct netdevsim *ns = netdev_priv(dev);
> +
> +	if (vid >= VLAN_N_VID)
> +		return -EINVAL;
> +
> +	if (proto == htons(ETH_P_8021Q))
> +		set_bit(vid, ns->vlan.ctag);

Maybe

    DEBUG_NET_WARN_ON_ONCE(test_and_set_bit(vid, ns->vlan.ctag));

> +	else if (proto == htons(ETH_P_8021AD))
> +		set_bit(vid, ns->vlan.stag);
> +
> +	return 0;
> +}
> +
> +static int nsim_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
> +{
> +	struct netdevsim *ns = netdev_priv(dev);
> +
> +	if (vid >= VLAN_N_VID)
> +		return -EINVAL;
> +
> +	if (proto == htons(ETH_P_8021Q))
> +		clear_bit(vid, ns->vlan.ctag);

and

    DEBUG_NET_WARN_ON_ONCE(!test_and_clear_bit(vid, ns->vlan.ctag));


> +	else if (proto == htons(ETH_P_8021AD))
> +		clear_bit(vid, ns->vlan.stag);
> +
> +	return 0;
> +}


and in nsim_destroy, something like:

    for_each_set_bit(vid, ns->vlan.ctag, VLAN_N_VID)
        DEBUG_NET_WARN_ON_ONCE(1);
    for_each_set_bit(vid, ns->vlan.stag, VLAN_N_VID)
        DEBUG_NET_WARN_ON_ONCE(1);

just to make sure that we did all the accounting correctly?


This last one would catch problems like the error handling spotted by
AI (just add 3 offloaded devices + one non-offloaded, then try to
offload the last one, netdevsim accepts only 3 secys so mdo_add_secy
will fail):


# echo 1 > /sys/bus/netdevsim/new_device
# ip link add link eni1np1 type macsec port 5 offload mac
# ip link add link eni1np1 type macsec port 6 offload mac
# ip link add link eni1np1 type macsec port 7 offload mac
# ip link add macsec_H link eni1np1 type macsec offload off
# ip link add link macsec_H type  vlan id 20
# cat /sys/kernel/debug/netdevsim/netdevsim1/ports/0/vlan
# ip link set macsec_H type  macsec offload mac
RTNETLINK answers: No space left on device
# cat /sys/kernel/debug/netdevsim/netdevsim1/ports/0/vlan
ctag 20
# echo 1 > /sys/bus/netdevsim/del_device
<splat>

-- 
Sabrina

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

[Sabrina Dubroca]

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

Re: [PATCH net-next v2 2/3] selftests: Add macsec offload VLAN tests

[Cosmin Ratiu]

On Fri, 2026-02-27 at 06:58 -0800, Jakub Kicinski wrote:
> On Fri, 27 Feb 2026 11:02:26 +0200 Cosmin Ratiu wrote:
> > Add macsec offload VLAN tests using the netsim VLAN support just
> > added.
> > In case that VLAN support is missing (e.g. when running these tests
> > on
> > older kernels), the VLAN tests are simply skipped.
> 
> You have to enable macsec in the config file for netdevsim selftests

macsec is already enabled, I assumed you meant VLAN. Will do.

Cosmin.

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

[Cosmin Ratiu]

On Sat, 2026-02-28 at 12:27 +0100, Sabrina Dubroca wrote:
> 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?

You're right. I ended up thinking the complexity is higher than average
for a net fix, but it's mostly in test infra. Apologies for the back-
and-forth.

> [...]
> > > @@ -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?

Will do.

Cosmin.

Re: [PATCH net-next v2 2/3] selftests: Add macsec offload VLAN tests

[Jakub Kicinski]

On Fri, 6 Mar 2026 15:01:02 +0000 Cosmin Ratiu wrote:
> On Fri, 2026-02-27 at 06:58 -0800, Jakub Kicinski wrote:
> > On Fri, 27 Feb 2026 11:02:26 +0200 Cosmin Ratiu wrote:  
> > > Add macsec offload VLAN tests using the netsim VLAN support just
> > > added.
> > > In case that VLAN support is missing (e.g. when running these tests
> > > on
> > > older kernels), the VLAN tests are simply skipped.  
> > 
> > You have to enable macsec in the config file for netdevsim selftests  
> 
> macsec is already enabled, I assumed you meant VLAN. Will do.

Responding after a week and then immediately posting a v-next is really
bad. If someone responds to you here the conversation will be
splintered between v2 and v3.