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: <20241007123751.3df87430@device-21.home>
Date: Mon, 7 Oct 2024 12:37:51 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, davem@...emloft.net,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 thomas.petazzoni@...tlin.com, Jakub Kicinski <kuba@...nel.org>, Eric
 Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 linux-arm-kernel@...ts.infradead.org, Christophe Leroy
 <christophe.leroy@...roup.eu>, Herve Codina <herve.codina@...tlin.com>,
 Florian Fainelli <f.fainelli@...il.com>, Heiner Kallweit
 <hkallweit1@...il.com>, Vladimir Oltean <vladimir.oltean@....com>, Marek
 Behún <kabel@...nel.org>, Köry Maincent
 <kory.maincent@...tlin.com>, Oleksij Rempel <o.rempel@...gutronix.de>
Subject: Re: [PATCH net-next v2 7/9] net: phy: introduce ethtool_phy_ops to
 get and set phy configuration

Hello Andrew, Russell,

On Fri, 4 Oct 2024 20:02:05 +0100
"Russell King (Oracle)" <linux@...linux.org.uk> wrote:

[...]

> > This seems overly simplistic to me. Don't you need to iterate over all
> > the other PHYs attached to this MAC and ensure they are isolated? Only
> > one can be unisolated at once.
> > 
> > It is also not clear to me how this is going to work from a MAC
> > perspective. Does the MAC call phy_connect() multiple times? How does
> > ndev->phydev work? Who is responsible for the initial configuration,
> > such that all but one PHY is isolated?
> > 
> > I assume you have a real board that needs this. So i think we need to
> > see a bit more of the complete solution, including the MAC changes and
> > the device tree for the board, so we can see the big picture.  
> 
> Also what the ethernet driver looks like too!
> 
> One way around the ndev->phydev problem, assuming that we decide that
> isolate is a good idea, would be to isolate the current PHY, disconnect
> it from the net_device, connect the new PHY, and then clear the isolate
> on the new PHY. Essentially, ndev->phydev becomes the currently-active
> PHY.

It seems I am missing details in my cover and the overall work I'm
trying to achieve.

This series focuses on isolating the PHY in the case where only one
PHY is attached to the MAC. I have followup work to support multi-PHY
interfaces. I will do my best to send the RFC this week so that you can
take a look. I'm definitely not saying the current code supports that.

To tell you some details, it indeed works as Russell says, I
detach/re-attach the PHYs, ndev->phydev is the "currently active" PHY.

I'm using a new dedicated "struct phy_mux" for that, which has :

 - Parent ops (that would be filled either by the MAC, or by phylink,
in the same spirit as phylink can be an sfp_upstream), which manages
PHY attach / detach to the netdev, but also the state-machine or the
currently inactive PHY.

 - multiplexer ops, that implement the switching logic, if any (drive a
GPIO, write a register, this is in the case of real multiplexers like
we have on some of the Turris Omnia boards, which the phy_mux framework
would support)

 - child ops, that would be hooks to activate/deactivate a PHY itself
(isoalte/unisolate, power-up/power-down).

I'll send the RFC ASAP, I still have a few rough edges that I will
mention in the cover.

> However, I still want to hear whether multiple PHYs can be on the same
> MII bus from a functional electrical perspective.

Yup, I have that hardware.

Thanks,

Maxime


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ