| 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
| ||
|
Message-ID: <DB8PR04MB6795FC58C1D0E2481E2BC35EE6D29@DB8PR04MB6795.eurprd04.prod.outlook.com>
Date: Mon, 6 Sep 2021 02:29:30 +0000
From: Joakim Zhang <qiangqing.zhang@....com>
To: Russell King <linux@...linux.org.uk>
CC: Andrew Lunn <andrew@...n.ch>, Vladimir Oltean <olteanv@...il.com>,
"peppe.cavallaro@...com" <peppe.cavallaro@...com>,
"alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
"joabreu@...opsys.com" <joabreu@...opsys.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
"mcoquelin.stm32@...il.com" <mcoquelin.stm32@...il.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"f.fainelli@...il.com" <f.fainelli@...il.com>,
"hkallweit1@...il.com" <hkallweit1@...il.com>,
dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH] net: stmmac: fix MAC not working when system resume back
with WoL enabled
Hi Russell,
> -----Original Message-----
> From: Russell King <linux@...linux.org.uk>
> Sent: 2021年9月4日 4:12
> To: Joakim Zhang <qiangqing.zhang@....com>
> Cc: Andrew Lunn <andrew@...n.ch>; Vladimir Oltean <olteanv@...il.com>;
> peppe.cavallaro@...com; alexandre.torgue@...s.st.com;
> joabreu@...opsys.com; davem@...emloft.net; kuba@...nel.org;
> mcoquelin.stm32@...il.com; netdev@...r.kernel.org; f.fainelli@...il.com;
> hkallweit1@...il.com; dl-linux-imx <linux-imx@....com>
> Subject: Re: [PATCH] net: stmmac: fix MAC not working when system resume
> back with WoL enabled
>
> Hi,
>
> Here's a patch to try - you'll need to integrate the new calls into stmmac's
> suspend and resume hooks. Obviously, given my previous comments, this isn't
> tested!
>
> I didn't need to repeat the mac_wol boolean to phylink_resume as we can
> record the state internally - mac_wol should not change between a call to
> phylink_suspend() and subsequent phylink_resume() anyway.
>
> mac_wol should only be true if the MAC is involved in processing packets for
> WoL, false otherwise.
>
> Please let me know if this resolves your stmmac WoL issue.
Thanks a lot for your work. 😊
There is a build issue in this patch, could you please have a check? I work on the latest net-next repo.
> Thanks.
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index
> f0c769027145..c4d0de04416a 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -33,6 +33,7 @@
> enum {
> PHYLINK_DISABLE_STOPPED,
> PHYLINK_DISABLE_LINK,
> + PHYLINK_DISABLE_MAC_WOL,
> };
>
> /**
> @@ -1313,6 +1314,9 @@ EXPORT_SYMBOL_GPL(phylink_start);
> * network device driver's &struct net_device_ops ndo_stop() method. The
> * network device's carrier state should not be changed prior to calling this
> * function.
> + *
> + * This will synchronously bring down the link if the link is not
> + already
> + * down (in other words, it will trigger a mac_link_down() method
> + call.)
> */
> void phylink_stop(struct phylink *pl)
> {
> @@ -1338,6 +1342,81 @@ void phylink_stop(struct phylink *pl) }
> EXPORT_SYMBOL_GPL(phylink_stop);
>
> +
> +/**
> + * phylink_suspend() - handle a network device suspend event
> + * @pl: a pointer to a &struct phylink returned from phylink_create()
> + * @mac_wol: true if the MAC needs to receive packets for Wake-on-Lan
> + *
> + * Handle a network device suspend event. There are several cases:
> + * - If Wake-on-Lan is not active, we can bring down the link between
> + * the MAC and PHY by calling phylink_stop().
> + * - If Wake-on-Lan is active, and being handled only by the PHY, we
> + * can also bring down the link between the MAC and PHY.
> + * - If Wake-on-Lan is active, but being handled by the MAC, the MAC
> + * still needs to receive packets, so we can not bring the link down.
> + */
> +void phylink_suspend(struct phylink *pl, bool mac_wol) {
> + ASSERT_RTNL();
> +
> + if (mac_wol && (!pl->netdev || pl->netdev->wol_enabled)) {
> + /* Wake-on-Lan enabled, MAC handling */
> + mutex_lock(&pl->state_mutex);
> +
> + /* Stop the resolver bringing the link up */
> + __set_bit(PHYLINK_DISABLE_MAC_WOL,
> &pl->phylink_disable_state);
> +
> + /* Disable the carrier, to prevent transmit timeouts,
> + * but one would hope all packets have been sent.
> + */
> + netif_carrier_off(pl->netdev);
> +
> + /* We do not call mac_link_down() here as we want the
> + * link to remain up to receive the WoL packets.
> + */
> + mutex_unlock(&pl->state_mutex);
> + } else {
> + phylink_stop(pl);
> + }
> +}
> +EXPORT_SYMBOL_GPL(phylink_suspend);
> +
> +/**
> + * phylink_resume() - handle a network device resume event
> + * @pl: a pointer to a &struct phylink returned from phylink_create()
> + *
> + * Undo the effects of phylink_suspend(), returning the link to an
> + * operational state.
> + */
> +void phylink_resume(struct phylink *pl) {
> + ASSERT_RTNL();
> +
> + if (test_bit(PHYLINK_DISABLE_MAC_WOL, &pl->phylink_disable_state)) {
> + /* Wake-on-Lan enabled, MAC handling */
> +
> + /* Call mac_link_down() so we keep the overall state balanced.
> + * Do this under the state_mutex lock for consistency. This
> + * will cause a "Link Down" message to be printed during
> + * resume, which is harmless - the true link state will be
> + * printed when we run a resolve.
> + */
> + mutex_lock(&pl->state_mutex);
> + phylink_link_down(pl);
> + mutex_unlock(&pl->state_mutex);
> +
> + /* Re-apply the link parameters so that all the settings get
> + * restored to the MAC.
> + */
> + phylink_mac_initial_config(pl, true);
> + phylink_enable_and_run_resolve(pl, PHYLINK_DISABLE_MAC_WOL);
There is no "phylink_enable_and_run_resolve " sysbol, I guess you want do below operations in this function:
clear_bit(PHYLINK_DISABLE_MAC_WOL, &pl->phylink_disable_state);
phylink_run_resolve(pl);
> + } else {
> + phylink_start(pl);
> + }
> +}
> +EXPORT_SYMBOL_GPL(phylink_resume);
> +
> /**
> * phylink_ethtool_get_wol() - get the wake on lan parameters for the PHY
> * @pl: a pointer to a &struct phylink returned from phylink_create() diff --git
> a/include/linux/phylink.h b/include/linux/phylink.h index
> bdeec800da5c..ba0ab7126b96 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -462,6 +462,9 @@ void phylink_mac_change(struct phylink *, bool up);
> void phylink_start(struct phylink *); void phylink_stop(struct phylink *);
>
> +void phylink_suspend(struct phylink *pl, bool mac_wol); void
> +phylink_resume(struct phylink *pl);
> +
> void phylink_ethtool_get_wol(struct phylink *, struct ethtool_wolinfo *); int
> phylink_ethtool_set_wol(struct phylink *, struct ethtool_wolinfo *);
Best Regards,
Joakim Zhang
Powered by blists - more mailing lists