Message-ID: <Y87ky0q3HWJhxfca@lunn.ch>
Date:   Mon, 23 Jan 2023 20:49:31 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>,
        "olteanv@...il.com" <olteanv@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Freihofer, Adrian" <adrian.freihofer@...mens.com>
Subject: Re: dsa: phy started on dsa_register_switch()

On Mon, Jan 23, 2023 at 10:57:10AM -0800, Florian Fainelli wrote:
> On 1/23/23 10:35, Sverdlin, Alexander wrote:
> > Dear DSA maintainers,
> > 
> > I've been puzzled by the fact ports of the DSA switches are enabled on
> > bootup by default (PHYs configured, LEDs ON, etc) in contrast to the
> > normal Ethernet ports.
> > 
> > Some people tend to think this is a security issue that port is "open"
> > even though no configuration has been performed on the port, so I
> > looked into the differences between Ethernet drivers and DSA
> > infrastructure.
> 
> If you are concerned about security with a switch, then clearly the switch
> should have an EEPROM which configures it to isolate all of the ports from
> one another, and possibly do additional configuration to prevent any packets
> from leaking.

It depends on the switch vendor, but Marvell Switches have a NO_CPU
strapping. If strapped this way, they power up in dumb switch mode,
all ports active, one big bridge. If NO_CPU is not strapped, depending
on the generation of device, the PHYs may come up, but the switch core
keeps the ports isolated, so no traffic will flow. So if security is a
design consideration, check if the switch you are using has such a
strapping. And do consider an EEPROM to explicitly shut everything
down. Also, watch out for the bootloader. Some switches have basic
support in u-boot, and barebox, enough to get the switch into a state
your can tftpboot over one of its ports.

> > Traditionally phylink_of_phy_connect() and phylink_connect_phy() are
> > being called from _open() callbacks of the Ethernet drivers so
> > as long as the Ethernet ports are !IFF_UP PHYs are not running,
> > LEDs are OFF, etc.
> 
> This is what is advised for Ethernet controller drivers to do, but is not
> strictly enforced or true throughout the entire tree, that is, it depends
> largely upon whether people writing/maintaining those drivers are sensitive
> to that behavior.

There are a number of MACs which do the connect in the probe. Both are
considered correct, and doing it in probe has advantages, such as
being able to return -EPROBE_DEFER because the PHY has not appeared
yet.

It should also be noted that phylib will not explicitly down a PHY
during the PHYs probe. So if the PHY is strapped to come up on power
up, or something else like the bootloader to brought it up, it will
remain up.

> I am sensitive to the power management aspect that getting the PHY and
> Ethernet link negotiated and then (re)negotiated several times through a
> products' boot cycle is a waste of energy and too many times do we break and
> make the link. The security aspect, I am less sensitive since the PHY is not
> how it should be enforced.

There are also people who as sensitive to the time it takes to
establish link. Auto-neg takes around 1.5 seconds. In some embedded
uses cases, that can be a long time, they want to avoid waiting for
that by strapping the PHY to start autoneg on power on, so it might be
ready to go by the time userspace ifup's the interface.

In general, PHY LEDs are a wild west in Linux, no well defined
behaviour. So i expect there are traditional NICs whos LEDs are on
when the link is admin down.

     Andrew

Powered by blists - more mailing lists