netdev - Re: [PATCH v2 net] net: phy: ensure that genphy_c45_an_config_eee_aneg() sees new value of phydev->eee_cfg.eee

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: <CAFSsGVtCPyto3DHD5bL9H6AOiDKQXcxS34nGtyM7gEKBepQFkA@mail.gmail.com>
Date: Wed, 20 Nov 2024 10:56:43 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: Choong Yong Liang <yong.liang.choong@...ux.intel.com>
Cc: Eric Dumazet <edumazet@...gle.com>, David Miller <davem@...emloft.net>, 
	Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>, 
	Russell King <rmk+kernel@...linux.org.uk>, Andrew Lunn <andrew@...n.ch>, 
	Andrew Lunn <andrew+netdev@...n.ch>, Oleksij Rempel <o.rempel@...gutronix.de>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net] net: phy: ensure that genphy_c45_an_config_eee_aneg()
 sees new value of phydev->eee_cfg.eee_enabled

On Wed, Nov 20, 2024 at 4:11 AM Choong Yong Liang
<yong.liang.choong@...ux.intel.com> wrote:
>
>
>
> On 17/11/2024 4:52 am, Heiner Kallweit wrote:
> > This is a follow-up to 41ffcd95015f ("net: phy: fix phylib's dual
> > eee_enabled") and resolves an issue with genphy_c45_an_config_eee_aneg()
> > (called from genphy_c45_ethtool_set_eee) not seeing the new value of
> > phydev->eee_cfg.eee_enabled.
> >
> > Fixes: 49168d1980e2 ("net: phy: Add phy_support_eee() indicating MAC support EEE")
> > Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> > ---
> > v2:
> > - change second arg of phy_ethtool_set_eee_noneg to pass the old settings
> > - reflect argument change in kdoc
> > ---
> >   drivers/net/phy/phy.c | 24 ++++++++++++++----------
> >   1 file changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 8876f3673..2ae0e3a67 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -1671,7 +1671,7 @@ EXPORT_SYMBOL(phy_ethtool_get_eee);
> >    * phy_ethtool_set_eee_noneg - Adjusts MAC LPI configuration without PHY
> >    *                         renegotiation
> >    * @phydev: pointer to the target PHY device structure
> > - * @data: pointer to the ethtool_keee structure containing the new EEE settings
> > + * @old_cfg: pointer to the eee_config structure containing the old EEE settings
> >    *
> >    * This function updates the Energy Efficient Ethernet (EEE) configuration
> >    * for cases where only the MAC's Low Power Idle (LPI) configuration changes,
> > @@ -1682,11 +1682,10 @@ EXPORT_SYMBOL(phy_ethtool_get_eee);
> >    * configuration.
> >    */
> >   static void phy_ethtool_set_eee_noneg(struct phy_device *phydev,
> > -                                   struct ethtool_keee *data)
> > +                                   const struct eee_config *old_cfg)
> >   {
> > -     if (phydev->eee_cfg.tx_lpi_enabled != data->tx_lpi_enabled ||
> > -         phydev->eee_cfg.tx_lpi_timer != data->tx_lpi_timer) {
> > -             eee_to_eeecfg(&phydev->eee_cfg, data);
> > +     if (phydev->eee_cfg.tx_lpi_enabled != old_cfg->tx_lpi_enabled ||
> > +         phydev->eee_cfg.tx_lpi_timer != old_cfg->tx_lpi_timer) {
> >               phydev->enable_tx_lpi = eeecfg_mac_can_tx_lpi(&phydev->eee_cfg);
> >               if (phydev->link) {
> >                       phydev->link = false;
> > @@ -1706,18 +1705,23 @@ static void phy_ethtool_set_eee_noneg(struct phy_device *phydev,
> >    */
> >   int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_keee *data)
> >   {
> > +     struct eee_config old_cfg;
> >       int ret;
> >
> >       if (!phydev->drv)
> >               return -EIO;
> >
> >       mutex_lock(&phydev->lock);
> > +
> > +     old_cfg = phydev->eee_cfg;
> > +     eee_to_eeecfg(&phydev->eee_cfg, data);
> > +
> >       ret = genphy_c45_ethtool_set_eee(phydev, data);
> > -     if (ret >= 0) {
> > -             if (ret == 0)
> > -                     phy_ethtool_set_eee_noneg(phydev, data);
> > -             eee_to_eeecfg(&phydev->eee_cfg, data);
> > -     }
> > +     if (ret == 0)
> > +             phy_ethtool_set_eee_noneg(phydev, &old_cfg);
> > +     else if (ret < 0)
> > +             phydev->eee_cfg = old_cfg;
> > +
> >       mutex_unlock(&phydev->lock);
> >
> >       return ret < 0 ? ret : 0;
>
> Hi Heiner,
>
> I hope this message finds you well.
>
> I noticed that the recent patch you submitted appears to be based on the
> previous work I did in this patch series:
> https://patchwork.kernel.org/project/netdevbpf/cover/20241115111151.183108-1-yong.liang.choong@linux.intel.com/.
>
> Would you mind including my name as "Reported-by" in the commit message? I
> believe this would appropriately acknowledge my role in identifying and
> reporting the issue.
>

Reported-by: Choong Yong Liang <yong.liang.choong@...ux.intel.com>

Hope this is enough for patchwork and/or the maintainers to pick up this tag,
w/o resubmitting the patch.