netdev - Re: Component API not right for DSA?

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] [day] [month] [year] [list]

Message-ID: <20240918142551.nelssrithsyqgg6p@skbuf>
Date: Wed, 18 Sep 2024 17:25:51 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
	Saravana Kannan <saravanak@...gle.com>, netdev@...r.kernel.org
Subject: Re: Component API not right for DSA?

On Wed, Sep 18, 2024 at 02:47:30PM +0100, Russell King (Oracle) wrote:
> On Wed, Sep 18, 2024 at 02:10:08PM +0300, Vladimir Oltean wrote:
> > This is all great, but then I realized that, for addressing issue #2,
> > it is no better than what we currently have. Namely, by default the tree
> > looks like this:
> > 
> ...
> > 
> > but after this operation:
> > 
> > $ echo d0032004.mdio-mii:11 > /sys/bus/mdio_bus/devices/d0032004.mdio-mii\:11/driver/unbind
> > $ cat /sys/kernel/debug/device_component/dsa_tree.0.auto
> > aggregate_device name                                  status
> > -------------------------------------------------------------
> > dsa_tree.0.auto                                     not bound
> > 
> > device name                                            status
> > -------------------------------------------------------------
> > (unknown)                                      not registered
> > d0032004.mdio-mii:10                                not bound
> > d0032004.mdio-mii:12                                not bound
> > 
> > the tree (component master) is unbound, its unbind() method calls
> > component_unbind_all(), and this also unbinds the other switches.
> 
> Correct. As author of the component helper... The component helper was
> designed for an overall device that is made up of multiple component
> devices that are themselves drivers, and _all_ need to be present in
> order for the overall device to be functional. It is not intended to
> address cases where an overall device has optional components.
> 
> The helper was originally written to address that problem for the
> Freescale i.MX IPU, which had been sitting in staging for considerable
> time, and was blocked from being moved out because of issues with this
> that weren't solvable at the time (we didn't have device links back
> then, which probably could've been used instead.)

Thanks for confirming. Although I could use the component helper in DSA
as it is now, it would ossify a limitation which I would like to remove.
The logic I would need is "aggregate device is bound as long as one
component is added, and gets notified of the addition/removal of all
other components". I guess it would be better suited to open-code this
logic in DSA.

As for the actual device_component debugfs folder that the component API
creates by default, I like the introspection into the switch tree state
that it offers when applied to DSA (it is useful especially if some
switches will go missing). Maybe I will keep the idea of having a
platform_device per dsa_switch_tree, and also create a debugfs with
similar information for it. If I also open-code this, I could also look
into adding a column with the last error code returned by each switch's
setup() function (I've lost count of how many times I had to debug
cross-chip probe issues). I will have to see.