netdev - Re: [PATCH net-next 2/2] net: phy: add keep_data

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 26 Jul 2023 09:10:59 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: "mengyuanlou@...-swift.com" <mengyuanlou@...-swift.com>
Cc: Simon Horman <simon.horman@...igine.com>, netdev@...r.kernel.org,
	Jakub Kicinski <kuba@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH net-next 2/2] net: phy: add keep_data_connection to
 struct phydev

On Wed, Jul 26, 2023 at 10:35:32AM +0800, mengyuanlou@...-swift.com wrote:
> 
> 
> > 2023年7月25日 21:12，Russell King (Oracle) <linux@...linux.org.uk> 写道：
> > 
> > Hi Simon,
> > 
> > Thanks for spotting that this wasn't sent to those who should have
> > been.
> > 
> > Mengyuan Lou, please ensure that you address your patches to
> > appropriate recipients.
> > 
> > On Tue, Jul 25, 2023 at 02:05:36PM +0200, Simon Horman wrote:
> >>> + * @keep_data_connection: Set to true if the PHY or the attached MAC need
> >>> + *                        physical connection to receive packets.
> > 
> > Having had a brief read through, this comment seems to me to convey
> > absolutely no useful information what so ever.
> > 
> > In order to receive packets, a physical connection between the MAC and
> > PHY is required. So, based on that comment, keep_data_connection must
> > always be true!
> > 
> > So, the logic in phylib at the moment is:
> > 
> >        phydev->wol_enabled = wol.wolopts || (netdev && netdev->wol_enabled);
> >        /* If the device has WOL enabled, we cannot suspend the PHY */
> >        if (phydev->wol_enabled && !(phydrv->flags & PHY_ALWAYS_CALL_SUSPEND))
> >                return -EBUSY;
> > 
> > wol_enabled will be true if the PHY driver reports that WoL is
> > enabled at the PHY, or the network device marks that WoL is
> > enabled at the network device. netdev->wol_enabled should be set
> > when the network device is looking for the wakeup packets.
> > 
> > Then, the PHY_ALWAYS_CALL_SUSPEND flag basically says that "even
> > in these cases, we want to suspend the PHY".
> > 
> > This patch appears to drop netdev->wol_enabled, replacing it with
> > netdev->ncsi_enabled, whatever that is - and this change alone is
> > probably going to break drivers, since they will already be
> > expecting that netdev->wol_enabled causes the PHY _not_ to be
> > suspended.
> > 
> > Therefore, I'm not sure this patch makes much sense.
> > 
> > Since the phylib maintainers were not copied with the original
> > patch, that's also a reason to NAK it.
> > 
> > Thanks.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> > 
> 
> 
> Now Mac and phy in kernel is separated into two parts.
> There are some features need to keep data connection.
> 
> Phy ——— Wake-on-Lan —— magic packets
> 
> When NIC as a ethernet in host os and it also supports ncsi as a bmc network port at same time.
> Mac/mng —— LLDP/NCSI —— ncsi packtes
> I think it need a way to notice phy modules.

Right, so this is _in addtion_ to WoL. Therefore, when adding support
for it, you need to _keep_ the existing WoL support, not remove it in
preference for NCSI.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!