All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Cosmin Ratiu <cratiu@nvidia.com>, Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Simon Horman <horms@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>,
	Shuah Khan <shuah@kernel.org>,
	linux-kselftest@vger.kernel.org,
	Dragos Tatulea <dtatulea@nvidia.com>
Subject: Re: [PATCH net v6 4/4] macsec: Support VLAN-filtering lower devices
Date: Thu, 2 Apr 2026 16:48:16 +0200	[thread overview]
Message-ID: <ac6BsG4IhAajip1s@krikkit> (raw)
In-Reply-To: <20260330130130.989236-5-cratiu@nvidia.com>

2026-03-30, 16:01:30 +0300, Cosmin Ratiu wrote:
> @@ -2616,14 +2616,22 @@ static int macsec_update_offload(struct net_device *dev, enum macsec_offload off
>  	if (!ops)
>  		return -EOPNOTSUPP;
>  
> -	macsec->offload = offload;
> -
>  	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) {
> -		macsec->offload = prev_offload;
> +	if (ret)
>  		return ret;
> +
> +	/* 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);

Paolo pointed me to the sashiko review for this patch
https://sashiko.dev/#/patchset/20260330130130.989236-1-cratiu%40nvidia.com

A simple way to trigger this is to do s/VLAN_N_VID/500/ in
nsim_vlan_rx_*_vid.

For example:

echo 1 > /sys/bus/netdevsim/new_device
ip link add link eni1np1 macsec0 type macsec
ip link add link macsec0 macsec0.1 type vlan id 1
ip link add link macsec0 macsec0.1000 type vlan id 1000
ip link set macsec0 type macsec offload mac
cat /sys/kernel/debug/netdevsim/netdevsim1/ports/0/vlan      # empty


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


OTOH maybe we don't need to care, since __netdev_update_features also
(kind of) ignores those errors:

echo 1 > /sys/bus/netdevsim/new_device
ethtool -K eni1np1 rx-vlan-filter off
ip link add link eni1np1 eni1np1.1 type vlan id 1
ip link add link eni1np1 eni1np1.1000 type vlan id 1000
cat /sys/kernel/debug/netdevsim/netdevsim1/ports/0/vlan      # empty as expected
ethtool -K eni1np1 rx-vlan-filter on                         # succeeds
ethtool -k eni1np1 | grep rx-vlan-filter                     # "rx-vlan-filter: on"
cat /sys/kernel/debug/netdevsim/netdevsim1/ports/0/vlan      # still empty because id=1000 was rejected
                                                             # and everything got rolled back
ip link add link eni1np1 eni1np1.123 type vlan id 123        # succeeds
cat /sys/kernel/debug/netdevsim/netdevsim1/ports/0/vlan      # only "ctag 123"


[at this point running
    ip link del eni1np1.1
or
    ethtool -K eni1np1 rx-vlan-filter off
will splat because vlan_filter_push_vids did a rollback/never added
id=1, and now we call vlan_kill_rx_filter_info, but that's specific to
this vid limit]

-- 
Sabrina

  reply	other threads:[~2026-04-02 14:48 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-30 13:01 [PATCH net v6 0/4] macsec: Add support for VLAN filtering in offload mode Cosmin Ratiu
2026-03-30 13:01 ` [PATCH net v6 1/4] selftests: Migrate nsim-only MACsec tests to Python Cosmin Ratiu
2026-04-02 10:03   ` Sabrina Dubroca
2026-04-02 11:51     ` Cosmin Ratiu
2026-03-30 13:01 ` [PATCH net v6 2/4] nsim: Add support for VLAN filters Cosmin Ratiu
2026-04-02 10:09   ` Sabrina Dubroca
2026-03-30 13:01 ` [PATCH net v6 3/4] selftests: Add MACsec VLAN propagation traffic test Cosmin Ratiu
2026-04-02 11:37   ` Sabrina Dubroca
2026-04-02 14:18     ` Cosmin Ratiu
2026-03-30 13:01 ` [PATCH net v6 4/4] macsec: Support VLAN-filtering lower devices Cosmin Ratiu
2026-04-02 14:48   ` Sabrina Dubroca [this message]
2026-04-07 15:07     ` Cosmin Ratiu
2026-04-08  8:25       ` Sabrina Dubroca
2026-04-08 10:24         ` Cosmin Ratiu
2026-04-08 11:01           ` Sabrina Dubroca
2026-04-02  2:54 ` [PATCH net v6 0/4] macsec: Add support for VLAN filtering in offload mode Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ac6BsG4IhAajip1s@krikkit \
    --to=sd@queasysnail.net \
    --cc=andrew+netdev@lunn.ch \
    --cc=cratiu@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=dtatulea@nvidia.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.