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: Windows password security audit tool. GUI, reports in PDF.

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

Date: Wed, 26 Jul 2023 10:35:32 +0800
From: "mengyuanlou@...-swift.com" <mengyuanlou@...-swift.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
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



> 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.