netdev - Re: [PATCH RFC net-next 5/5] net: dsa: always use phylink for CPU and DSA ports

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]

Date:   Fri, 8 Jul 2022 00:48:52 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Alvin __ipraga <alsi@...g-olufsen.dk>,
        Claudiu Manoil <claudiu.manoil@....com>,
        "David S. Miller" <davem@...emloft.net>,
        DENG Qingfang <dqfext@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        George McCollister <george.mccollister@...il.com>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Jakub Kicinski <kuba@...nel.org>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Landen Chao <Landen.Chao@...iatek.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org,
        Matthias Brugger <matthias.bgg@...il.com>,
        netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
        Sean Wang <sean.wang@...iatek.com>,
        UNGLinuxDriver@...rochip.com,
        Vivien Didelot <vivien.didelot@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Marek Behún <kabel@...nel.org>
Subject: Re: [PATCH RFC net-next 5/5] net: dsa: always use phylink for CPU
 and DSA ports

On Thu, Jul 07, 2022 at 09:23:46PM +0100, Russell King (Oracle) wrote:
> > > I'm not sure how practical that is when we have both DT and ACPI to deal
> > > with, and ACPI is certainly out of my knowledge area to be able to
> > > construct a software node to specify a fixed-link. Maybe it can be done
> > > at the fwnode layer? I don't know.
> > 
> > I don't want to be misunderstood. I brought up software nodes because
> > I'm sure you must have thought about this too, before proposing what you
> > did here. And unless there's a technical reason against software nodes
> > (which there doesn't appear to be, but I don't want to get ahead of
> > myself), I figured you must be OK with phylink absorbing the logic, case
> > in which I just don't understand why you are pushing back on a proposal
> > how to make phylink absorb the logic completely.
> 
> The reason I hadn't is because switching DSA to fwnode brings with it
> issues for ACPI, and Andrew wants to be very careful about ACPI in
> networking - and I think quite rightly. As soon as one switches from
> DT APIs to fwnode APIs, you basically permit people an easy path to
> re-use DT properties in ACPI-land without the ACPI issues being first
> considered.
> Yep - there's at least one other property we need to carry over from the
> DT node, which is the "ethernet" property.

I've cropped only these segments because there is something I apparently
need to highlight. What my patch does is _not_ at the device fwnode
level (in fact, DSA ports do not have a struct device, only the switch does),
but indeed at the most crude fwnode_handle level. And the fwnode_handles
I'm creating using the software_node API are not connected in any way
with the device. I'm not replacing the device's fwnodes with prosthetic
ones. The fact that I wrote "dn->fwnode.secondary = new_port_fwnode;" is
just a cheap hack to minimize the patch delta so I wouldn't have to
transport the software fwnode_handle from one place to another within
dsa_port_link_register_of(). This should definitely dissapear in the
final solution. In fact, as long as phylink doesn't keep a reference on
the fwnode after phylink_create() or phylink_fwnode_phy_connect(), I'm
pretty sure that the software node can even be deallocated during the
probing stage, no need to keep it for the entire lifetime of the device.

Therefore, no, we don't need the "ethernet" phandle in the software node
we create, because we just use that to pass it to phylink. We still keep
our original OF node for the rest of the activities. We don't even need
the "reg" u32 property, I just added that for no reason (I wasn't
completely sure what the API offers, then I didn't remove it).

So the concern that this software node can be abused for a transition to
ACPI is quite overestimated. Nothing in DSA is "switched to fwnode" per se,
and the creation of a fwnode is just part of "speaking the software node
language", which phylink already happily understands and so, needs no
change. Just my 2 cents.