netdev - Re: [PATCH v5 net-next 12/14] bridge: No DEV_PATH_BR_VLAN_UNTAG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250207150340.sxhsva7qz7bb7qjd@skbuf>
Date: Fri, 7 Feb 2025 17:03:40 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Eric Woudstra <ericwouds@...il.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Pablo Neira Ayuso <pablo@...filter.org>,
	Jozsef Kadlecsik <kadlec@...filter.org>,
	Jiri Pirko <jiri@...nulli.us>, Ivan Vecera <ivecera@...hat.com>,
	Roopa Prabhu <roopa@...dia.com>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Kuniyuki Iwashima <kuniyu@...zon.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Joe Damato <jdamato@...tly.com>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	Frank Wunderlich <frank-w@...lic-files.de>,
	Daniel Golle <daniel@...rotopia.org>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, netfilter-devel@...r.kernel.org,
	coreteam@...filter.org, bridge@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v5 net-next 12/14] bridge: No DEV_PATH_BR_VLAN_UNTAG_HW
 for dsa foreign

On Tue, Feb 04, 2025 at 08:49:19PM +0100, Eric Woudstra wrote:
> In network setup as below:
> 
>              fastpath bypass
>  .----------------------------------------.
> /                                          \
> |                        IP - forwarding    |
> |                       /                \  v
> |                      /                  wan ...
> |                     /
> |                     |
> |                     |
> |                   brlan.1
> |                     |
> |    +-------------------------------+
> |    |           vlan 1              |
> |    |                               |
> |    |     brlan (vlan-filtering)    |
> |    |               +---------------+
> |    |               |  DSA-SWITCH   |
> |    |    vlan 1     |               |
> |    |      to       |               |
> |    |   untagged    1     vlan 1    |
> |    +---------------+---------------+
> .         /                   \
>  ----->wlan1                 lan0
>        .                       .
>        .                       ^
>        ^                     vlan 1 tagged packets
>      untagged packets
> 
> br_vlan_fill_forward_path_mode() sets DEV_PATH_BR_VLAN_UNTAG_HW when
> filling in from brlan.1 towards wlan1. But it should be set to
> DEV_PATH_BR_VLAN_UNTAG in this case. Using BR_VLFLAG_ADDED_BY_SWITCHDEV
> is not correct. The dsa switchdev adds it as a foreign port.
> 
> The same problem for all foreignly added dsa vlans on the bridge.
> 
> First add the vlan, trying only native devices.
> If this fails, we know this may be a vlan from a foreign device.
> 
> Use BR_VLFLAG_TAGGING_BY_SWITCHDEV to make sure DEV_PATH_BR_VLAN_UNTAG_HW
> is set only when there if no foreign device involved.
> 
> Signed-off-by: Eric Woudstra <ericwouds@...il.com>
> ---

Shouldn't mlxsw_sp_switchdev_vxlan_vlans_add() also respect the
SWITCHDEV_F_NO_FOREIGN flag? My (maybe incorrect) understanding of
bridging topologies with vxlan and mlxsw is that they are neighbor
bridge ports, and mlxsw doesn't (seem to) call
switchdev_bridge_port_offload() for the vxlan bridge port. This
technically makes vxlan a foreign bridge port to mlxsw, so it should
skip reacting on VLAN switchdev objects when that flag is set, just
for uniform behavior across the board.

(your patch repeats the notifier without the SWITCHDEV_F_NO_FOREIGN
flag anyway, so it only matters for flowtable offload).