All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: Jakub Kicinski <kuba@kernel.org>,
	Nikolay Aleksandrov <nikolay@nvidia.com>,
	Jiri Pirko <jiri@nvidia.com>, Ido Schimmel <idosch@nvidia.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>
Subject: Is it ok for switch TCAMs to depend on the bridge state?
Date: Tue, 2 Nov 2021 11:03:53 +0000	[thread overview]
Message-ID: <20211102110352.ac4kqrwqvk37wjg7@skbuf> (raw)

Hi,

I've been reviewing a patch set which offloads to hardware some
tc-flower filters with some TSN-specific actions (ingress policing).
The keys of those offloaded tc-flower filters are not arbitrary, they
are the destination MAC address and VLAN ID of the frames, which is
relevant because these TSN policers are actually coupled with the
bridging service in hardware. So the premise of that patch set was that
the user would first need to add static FDB entries to the bridge with
the same key as the tc-flower key, before the tc-flower filters would be
accepted for offloading.

Naturally, with the current bridge/switchdev design where drivers cannot
actually NACK the removal of a bridge FDB entry, that is quite fragile,
because if the user would then proceed to delete the FDB entry, the
tc-flower filter would stop working and the user would wonder why.
So that patch set has stalled, currently.

But I was thinking, the above case is not the only one where features
offloaded through tc-flower might depend on the state of the bridging
service (and therefore on stuff configured in the Linux bridge and
offloaded through switchdev). Another example I can find is where there
are some tc-flower filters that involve VLANs (either in the key or in
the action portion). Generally, switches have the notion of a classified
VLAN, aka the VID used for internal processing of a packet. This may or
may not be equal to the VID from the 802.1Q header. For example, if a
port is VLAN-unaware or standalone, the classified VLAN is pretty much
guaranteed to not be equal to the VID from the 802.1Q header, if that
exists at all.

Also, I don't know whether this is the case in general or not, but the
hardware I'm working with has TCAM actions that operate on the
classified VLAN, not on the VID from the 802.1Q header. Therefore, this
again is tightly coupled with the bridging service.

What is currently done to support things like VLAN rewriting using the
tc-vlan action is to require vlan_filtering to be set to 1 at the time
when the tc-flower rule is added, and then dynamic changes to the
vlan_filtering property are denied.

But the driver still cannot veto the removal of the port from the
bridge, or the deletion of the bridge itself. So this is still very
fragile, and there are cases where we could end up with broken
offloading for non-obvious reasons.

I don't have a clear picture in my mind about what is wrong. An airplane
viewer might argue that the TCAM should be completely separate from the
bridging service, but I'm not completely sure that this can be achieved
in the aforementioned case with VLAN rewriting on ingress and on egress,
it would seem more natural for these features to operate on the
classified VLAN (which again, depends on VLAN awareness being turned on).
Alternatively, one might argue that the deletion of a bridge interface
should be vetoed, and so should the removal of a port from a bridge.
But that is quite complicated, and doesn't answer questions such as
"what should you do when you reboot".
Alternatively, one might say that letting the user remove TCAM
dependencies from the bridging service is fine, but the driver should
have a way to also unoffload the tc-flower keys as long as the
requirements are not satisfied. I think this is also difficult to
implement.

I haven't copied any of the directly interested parties because I would
like to hear some neutral opinions first. Thanks for reading.

             reply	other threads:[~2021-11-02 11:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-02 11:03 Vladimir Oltean [this message]
2021-11-03 16:38 ` Is it ok for switch TCAMs to depend on the bridge state? Jakub Kicinski
2021-11-07 11:50 ` Ido Schimmel
2021-11-11 11:52   ` Vladimir Oltean
2021-11-11 13:46     ` Ido Schimmel
2021-11-11 14:12       ` Vladimir Oltean

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=20211102110352.ac4kqrwqvk37wjg7@skbuf \
    --to=vladimir.oltean@nxp.com \
    --cc=andrew@lunn.ch \
    --cc=f.fainelli@gmail.com \
    --cc=idosch@nvidia.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@nvidia.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.