All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Cosmin Ratiu <cratiu@nvidia.com>
Cc: Dragos Tatulea <dtatulea@nvidia.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"edumazet@google.com" <edumazet@google.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"andrew+netdev@lunn.ch" <andrew+netdev@lunn.ch>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH net] macsec: Support VLAN-filtering lower devices
Date: Fri, 9 Jan 2026 13:06:39 +0100	[thread overview]
Message-ID: <aWDvTx9JUHzUKEGm@krikkit> (raw)
In-Reply-To: <5bbb83c9964515526b3d14a43bea492f20f3a0fa.camel@nvidia.com>

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

  reply	other threads:[~2026-01-09 12:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

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=aWDvTx9JUHzUKEGm@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=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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.