RFC: dsa slave open handling

RFC: dsa slave open handling

[Peter Korsgaard]

Hi,

Currently the state of the dsa master device and slave devices are not
synchronized which leads to various issues. With dsa, the master device
needs to be running before you can use the slave devices, but it
shouldn't be used directly for I/O.

As an example, I have a device with a single network interface connected
to a mv88e6060 switch. This is nicely handled through dsa, but with this
setup, IP autoconfiguration doesn't work:

If you boot with ip=on (E.G. use any device) eth0 is first brought up
and then the dsa slave devices. DHCP/bootp requests are sent out on both
the dsa slaves and the master eth0 interface. This is not optimal as the
mv88e6060 uses the trailer tag format, so it ends up stripping the
trailing 4 bytes of the requests sent directly on eth0 (and then sends
out on port 0), which leads to invalid UDP checksum, but that's worse -
Once a reply has been received on one of the dsa ports, autoconfig
closes all other devices including eth0, which also brings down the dsa
ports so nfs mount fails.

If on the other hand you boot with ip=:::::<dsa port name>
(E.G. explicitly force autoconfig to use that port), then it fails when
it tries to open the device because eth0 isn't up:

static int dsa_slave_open(struct net_device *dev)
{
	struct dsa_slave_priv *p = netdev_priv(dev);
	struct net_device *master = p->parent->dst->master_netdev;
	int err;

	if (!(master->flags & IFF_UP))
		return -ENETDOWN;


How should this preferably be handled? We could either automatically
bring up the master device whenever a dsa slave is brought up:

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 64ca2a6..dfc7558 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -63,8 +63,9 @@ static int dsa_slave_open(struct net_device *dev)
        struct net_device *master = p->parent->dst->master_netdev;
        int err;
 
-       if (!(master->flags & IFF_UP))
-               return -ENETDOWN;
+       err = dev_change_flags(master, master->flags | IFF_UP);
+       if (err < 0)
+               return err;
 
        if (compare_ether_addr(dev->dev_addr, master->dev_addr)) {
                err = dev_uc_add(master, dev->dev_addr);

      dev_change_flags(master, master->flags | IFF_UP);

Or we could simply handle it in ipconfig by not closing dsa master devices:

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..4d4474b 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -277,6 +277,11 @@ static void __init ic_close_devs(void)
                next = d->next;
                dev = d->dev;
                if (dev != ic_dev) {
+#ifdef CONFIG_NET_DSA
+                       /* master device might be needed for dsa access */
+                       if (dev->dsa_ptr)
+                               continue;
+#endif
                        DBG(("IP-Config: Downing %s\n", dev->name));
                        dev_change_flags(dev, d->flags);
                }

Which is preferred?

-- 
Bye, Peter Korsgaard

Re: RFC: dsa slave open handling

[Lennert Buytenhek]

On Mon, Mar 07, 2011 at 05:46:08PM +0100, Peter Korsgaard wrote:

> Hi,

Hello,


> Currently the state of the dsa master device and slave devices are not
> synchronized which leads to various issues. With dsa, the master device
> needs to be running before you can use the slave devices, but it
> shouldn't be used directly for I/O.

I think this should not be handled any differently than VLAN interfaces,
where eth0 needs to be up before you can exchange packets over eth0.123.

IOW, if you want to do DHCP over wan (a DSA-tunneled interface over
eth0), then just use an initramfs that sets eth0 up and then runs a
DHCP client on wan.

The fact that in-kernel IP autoconfiguration can't deal with DSA just
means that you shouldn't use the in-kernel IP autoconfiguration -- you
could "fix" this with one of your proposed patches, but then the next
thing will be someone wanting to do in-kernel IP autoconfig over a VLAN
interface stacked onto a DSA interface, etc, which then still wouldn't
work.


cheers,
Lennert

Re: RFC: dsa slave open handling

[Peter Korsgaard]

>>>>> "Lennert" == Lennert Buytenhek <buytenh@wantstofly.org> writes:

Hi,

 >> Currently the state of the dsa master device and slave devices are not
 >> synchronized which leads to various issues. With dsa, the master device
 >> needs to be running before you can use the slave devices, but it
 >> shouldn't be used directly for I/O.

 Lennert> I think this should not be handled any differently than VLAN
 Lennert> interfaces, where eth0 needs to be up before you can exchange
 Lennert> packets over eth0.123.

 Lennert> IOW, if you want to do DHCP over wan (a DSA-tunneled interface
 Lennert> over eth0), then just use an initramfs that sets eth0 up and
 Lennert> then runs a DHCP client on wan.

True. There are a few differences between vlans and dsa devices though,
which might make us want to handle them differently:

- VLANs are configured at runtime from userspace, DSA devices are
  configured directly in kernel code (E.G platform devices) - So by
  definition you cannot use the autconfig / nfsroot stuff on VLANs as
  they are not created before userspace runs, but DSA devices are
  basically indistinguishable from "real" devices from the POV of
  userspace.

- VLANs relation to their upstream ethernet device is clearly visible
  (E.G through brctl), whereas the details about what interface a DSA
  switch is connected to isn't.

-- 
Bye, Peter Korsgaard

Re: RFC: dsa slave open handling

[Lennert Buytenhek]

On Mon, Mar 07, 2011 at 08:35:47PM +0100, Peter Korsgaard wrote:

>  >> Currently the state of the dsa master device and slave devices are not
>  >> synchronized which leads to various issues. With dsa, the master device
>  >> needs to be running before you can use the slave devices, but it
>  >> shouldn't be used directly for I/O.
> 
>  Lennert> I think this should not be handled any differently than VLAN
>  Lennert> interfaces, where eth0 needs to be up before you can exchange
>  Lennert> packets over eth0.123.
> 
>  Lennert> IOW, if you want to do DHCP over wan (a DSA-tunneled interface
>  Lennert> over eth0), then just use an initramfs that sets eth0 up and
>  Lennert> then runs a DHCP client on wan.
> 
> True. There are a few differences between vlans and dsa devices though,
> which might make us want to handle them differently:
> 
> - VLANs are configured at runtime from userspace, DSA devices are
>   configured directly in kernel code (E.G platform devices) - So by
>   definition you cannot use the autconfig / nfsroot stuff on VLANs as
>   they are not created before userspace runs, but DSA devices are
>   basically indistinguishable from "real" devices from the POV of
>   userspace.

Sure.  That doesn't change the argument, though.  In fact, in most
situations, you'll e.g. want to bridge the lan[1-4] interfaces
together, and if you then want to DHCP over that bridge, you cannot
explain that to the kernel autoconfigurator either.


> - VLANs relation to their upstream ethernet device is clearly visible
>   (E.G through brctl), whereas the details about what interface a DSA
>   switch is connected to isn't.

commit c084080151e1de92159f8437fde34b6e5bebe35e
Author: Lennert Buytenhek <buytenh@wantstofly.org>
Date:   Fri Mar 20 09:49:49 2009 +0000

    dsa: set ->iflink on slave interfaces to the ifindex of the parent
    
    ..so that we can parse the DSA topology from 'ip link' output:
    
    1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue
    2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast qlen 1000
    3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast qlen 1000
    4: lan1@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue
    5: lan2@eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue
    6: lan3@eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue
    7: lan4@eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue
    
    Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>


cheers,
Lennert