netdev - Re: [PATCH v2 net-next] net: dsa: Allow only up to two HSR HW offloaded ports for KSZ9477

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: <20240619154814.dvjcry7ahvtznfxb@skbuf>
Date: Wed, 19 Jun 2024 18:48:14 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Lukasz Majewski <lukma@...x.de>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
	"David S. Miller" <davem@...emloft.net>,
	Oleksij Rempel <o.rempel@...gutronix.de>, Tristram.Ha@...rochip.com,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Simon Horman <horms@...nel.org>,
	Dan Carpenter <dan.carpenter@...aro.org>,
	"Ricardo B. Marliere" <ricardo@...liere.net>,
	Casper Andersson <casper.casan@...il.com>,
	linux-kernel@...r.kernel.org,
	Woojung Huh <woojung.huh@...rochip.com>,
	UNGLinuxDriver@...rochip.com, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH v2 net-next] net: dsa: Allow only up to two HSR HW
 offloaded ports for KSZ9477

On Wed, Jun 19, 2024 at 05:10:57PM +0200, Lukasz Majewski wrote:
> > How do you know you are rejecting the offloading of the interlink
> > port, and not of one of the ring ports?
> 
> It seems like iproute2 is providing the correct ordering (and assures
> that lan3/port2 is called as a third one - please see below).

This is not iproute2 providing the correct ordering, but rather
hsr_dev_finalize() in the kernel calling hsr_add_port() in a certain
order that matches what is expected in ksz9477.

Granted, this isn't an actual functional problem, but given that you
are fixing a newly developed feature for net-next, and that this is API
that gets progressively harder to change as more devices implement
offloads, I would expect a more obvious signaling mechanism to exist
for this, and now seems a good time to do it, rather than opting for the
most minimal fix.

One way to make the restriction more elegantly obvious (both to the user
and to the kernel developer) that it is about interlink ports rather
than the number of ports in general is to carry the port type in a
structure similar to struct netdev_lag_upper_info.

You would have to make hsr_portdev_setup() call something else rather
than netdev_upper_dev_link(), because that eats the "void *upper_info"
argument when calling __netdev_upper_dev_link(). Possibly create a new
netdev_upper_dev_link_info() ("link upper dev with extra info")
function, which only HSR calls with a new info structure. Then, the DSA
core has access to that and implicitly to the port type, and from there
on, you can apply the proper restriction.