netdev - RE: [PATCH net-next 1/3] net: phylink: Set host

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:   Thu, 5 Jan 2023 08:00:34 +0000
From:   Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To:     Russell King <linux@...linux.org.uk>
CC:     "andrew@...n.ch" <andrew@...n.ch>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH net-next 1/3] net: phylink: Set host_interfaces for a
 non-sfp PHY

Hello Russell,

> From: Russell King, Sent: Tuesday, January 3, 2023 6:54 PM
> 
> On Mon, Dec 26, 2022 at 04:14:23PM +0900, Yoshihiro Shimoda wrote:
> > Set phy_dev->host_interfaces by pl->link_interface in
> > phylink_fwnode_phy_connect() for a non-sfp PHY.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> > ---
> >  drivers/net/phy/phylink.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index 09cc65c0da93..1958d6cc9ef9 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -1809,6 +1809,7 @@ int phylink_fwnode_phy_connect(struct phylink *pl,
> >  		pl->link_interface = phy_dev->interface;
> >  		pl->link_config.interface = pl->link_interface;
> >  	}
> > +	__set_bit(pl->link_interface, phy_dev->host_interfaces);
> 
> This is probably going to break Macchiatobin platforms, since we
> declare that the link mode there is 10GBASE-R, we'll end up with
> host_interfaces containing just this mode. This will cause the
> 88x3310 driver to select a rate matching interface mode, which the
> mvpp2 MAC can't support.
> 
> If we want to fill host_interfaces in, then it needs to be filled in
> properly - and by that I mean with all the host interface modes that
> can be electrically supported - otherwise platforms will break.
> 
> So, sorry, but NAK on this change.

Thank you for your review! I understood why NAK on this change:
---
static int mv3310_select_mactype(unsigned long *interfaces)
{
...
        else if (test_bit(PHY_INTERFACE_MODE_SGMII, interfaces) &&
                 test_bit(PHY_INTERFACE_MODE_10GBASER, interfaces))
                return MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER;
...
        else if (test_bit(PHY_INTERFACE_MODE_10GBASER, interfaces))
                return MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH;
...
---
- On this change, since the interfaces is set to PHY_INTERFACE_MODE_10GBASER only,
  this function will return MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH.
- Without this change, since the host_interfaces value is zero, the mv3310_select_mactype()
  will not called.

I'll investigate phylink and marvell10 codes again.

Best regards,
Yoshihiro Shimoda