linux-kernel - Re: [PATCH RFC v1 net-next 11/12] bridge: br_vlan_fill_forward_path_mode no _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: <20241021134726.dzfz5uu2peyin3kk@skbuf>
Date: Mon, 21 Oct 2024 16:47:26 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Eric Woudstra <ericwouds@...il.com>
Cc: Nikolay Aleksandrov <razor@...ckwall.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Pablo Neira Ayuso <pablo@...filter.org>,
	Jozsef Kadlecsik <kadlec@...filter.org>,
	Roopa Prabhu <roopa@...dia.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	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, Andrew Lunn <andrew@...n.ch>,
	Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH RFC v1 net-next 11/12] bridge:
 br_vlan_fill_forward_path_mode no _UNTAG_HW for dsa

On Sun, Oct 20, 2024 at 11:23:18AM +0200, Eric Woudstra wrote:
> So after doing some more reading, at creation of the code using
> BR_VLFLAG_ADDED_BY_SWITCHDEV would have been without problems.
> 
> After the switchdev was altered so that objects from foreign devices can
> be added, it is problematic in br_vlan_fill_forward_path_mode(). I have
> tested and indeed any foreign device does have this problem.
> 
> So we need a way to distinguish in br_vlan_fill_forward_path_mode()
> whether or not we are dealing with a (dsa) foreign device on the switchdev.
> 
> I have come up with something, but this is most likely to crude to be
> accepted, but for the sake of 'rfc' discussing it may lead to a proper
> solution. So what does work is the following patch, so that
> netif_has_dsa_foreign_vlan() can be used inside
> br_vlan_fill_forward_path_mode().
> 
> Any suggestions on how this could be implemented properly would be
> greatly appreciated.

I don't know nearly enough about the netfilter flowtable to even
understand exactly the problem you're describing and are trying to solve.
I've started to read up on things, but plenty of concepts are new and
I'm mixing this with plenty of other activities. If you could share some
commands to build a test setup so I could form my own independent
opinion of what is going on, it would be great as it would speed up that
process.

With respect to the patch you've posted, it doesn't look exactly great.
One would need to make a thorough analysis of the bridge's use of
BR_VLFLAG_ADDED_BY_SWITCHDEV, of whether it still makes sense in today's
world where br_switchdev_vlan_replay() is a thing (a VLAN that used to
not be "added by switchdev" can become "added by switchdev" after a
replay, but this flag will remain incorrectly unset), of whether VLANs on
foreign DSA interfaces should even have this flag set, and on whether
your flowtable forwarding path patches are conceptually using it correctly.
There's a lot to think about, and if somebody doesn't have the big picture,
I'm worried that a wrong decision will be taken.