netdev - Re: Testing of r8169 workaround removal

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: <CAFbqK8kk8UqLXC=FPHjjYawHRozCmsKuV3WcD8x1y5HvYw_2rA@mail.gmail.com>
Date:   Sun, 28 Apr 2019 18:30:00 +0100
From:   Neil MacLeod <neil@...cleod.com>
To:     Heiner Kallweit <hkallweit1@...il.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Testing of r8169 workaround removal

Hi Heiner

Do you know if this is already fixed in 5.1-rc6 (Linus Torvalds tree),
as in order to test your request I thought I would reproduce the issue
with plain 5.1-rc6 with the workaround removed, however without the
workaround 5.1-rc6 is resuming correctly at 1000Mbps.

I went back to 4.19-rc4 (which we know is brroken) and I can reproduce
the issue with the PC (Revo 3700) resuming at 10Mbps, but with 5.1-rc6
I can no longer reproduce the issue when the workaround is removed.

I also tested 5.0.10 without the workaround, and again 5.0.10 is
resuming correctly at 1000Mbps.

I finally tested 4.19.23 without the workaround (the last iteration of
this kernel I published) and this does NOT resume correctly at
1000Mbps (it resumes at 10Mbps).

I'll test a few more iterations of 5.0.y to see if I can identify when
it was "fixed" but if you have any suggestions when it might have been
fixed I can try to confirm this that - currently it's somewhere
between 4.19.24 and 5.0.10!

Regards
Neil

On Sun, 28 Apr 2019 at 14:33, Heiner Kallweit <hkallweit1@...il.com> wrote:
>
> Hi Neil,
>
> you once reported the original issue resulting in this workaround.
> This workaround shouldn't be needed any longer, but I have no affected HW
> to test on. Do you have the option to apply the patch below to latest
> net-next and test link speed after resume from suspend?
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
> That would be much appreciated.
>
> Heiner
>
> ----------------------------------------------------------------
>
> After 8c90b795e90f ("net: phy: improve genphy_soft_reset") this
> workaround shouldn't be needed any longer. However I don't have
> affected hardware so I can't test it.
>
> This was the bug report leading to the workaround:
> https://bugzilla.kernel.org/show_bug.cgi?id=201081
>
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 383242df0..d4ec08e37 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4083,14 +4083,6 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
>         phy_speed_up(tp->phydev);
>
>         genphy_soft_reset(tp->phydev);
> -
> -       /* It was reported that several chips end up with 10MBit/Half on a
> -        * 1GBit link after resuming from S3. For whatever reason the PHY on
> -        * these chips doesn't properly start a renegotiation when soft-reset.
> -        * Explicitly requesting a renegotiation fixes this.
> -        */
> -       if (tp->phydev->autoneg == AUTONEG_ENABLE)
> -               phy_restart_aneg(tp->phydev);
>  }
>
>  static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr)
> --
> 2.21.0