netdev - Re: [PATCH v2 2/2] dpaa_eth: support all modes with rate adapting PHYs

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: <CA+h21hqzR72v9=dWGk1zBptNHNst+kajh6SHHSUMp02fAq5m5g@mail.gmail.com>
Date:   Mon, 27 Jan 2020 17:31:07 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     madalin.bucur@....nxp.com
Cc:     "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        netdev <netdev@...r.kernel.org>, ykaukab@...e.de
Subject: Re: [PATCH v2 2/2] dpaa_eth: support all modes with rate adapting PHYs

Hi Madalin,

On Mon, 27 Jan 2020 at 17:08, Madalin Bucur <madalin.bucur@....nxp.com> wrote:
>
> Stop removing modes that are not supported on the system interface
> when the connected PHY is capable of rate adaptation. This addresses
> an issue with the LS1046ARDB board 10G interface no longer working
> with an 1G link partner after autonegotiation support was added
> for the Aquantia PHY on board in
>
> commit 09c4c57f7bc4 ("net: phy: aquantia: add support for auto-negotiation configuration")
>
> Before this commit the values advertised by the PHY were not
> influenced by the dpaa_eth driver removal of system-side unsupported
> modes as the aqr_config_aneg() was basically a no-op. After this
> commit, the modes removed by the dpaa_eth driver were no longer
> advertised thus autonegotiation with 1G link partners failed.
>
> Reported-by: Mian Yousaf Kaukab <ykaukab@...e.de>
> Signed-off-by: Madalin Bucur <madalin.bucur@....nxp.com>
> ---
>  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index a301f0095223..d3eb235450e5 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2471,9 +2471,13 @@ static int dpaa_phy_init(struct net_device *net_dev)
>                 return -ENODEV;
>         }
>
> -       /* Remove any features not supported by the controller */
> -       ethtool_convert_legacy_u32_to_link_mode(mask, mac_dev->if_support);
> -       linkmode_and(phy_dev->supported, phy_dev->supported, mask);
> +       if (mac_dev->phy_if != PHY_INTERFACE_MODE_XGMII ||
> +           !phy_dev->rate_adaptation) {
> +               /* Remove any features not supported by the controller */
> +               ethtool_convert_legacy_u32_to_link_mode(mask,
> +                                                       mac_dev->if_support);
> +               linkmode_and(phy_dev->supported, phy_dev->supported, mask);
> +       }

Is this sufficient?
I suppose this works because you have flow control enabled by default?
What would happen if the user would disable flow control with ethtool?

>
>         phy_support_asym_pause(phy_dev);
>
> --
> 2.1.0
>

Regards,
-Vladimir