From patchwork Mon Mar 7 16:46:08 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Korsgaard X-Patchwork-Id: 85765 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id D14F9B70EC for ; Tue, 8 Mar 2011 03:46:26 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751203Ab1CGQqW (ORCPT ); Mon, 7 Mar 2011 11:46:22 -0500 Received: from mail-wy0-f174.google.com ([74.125.82.174]:61709 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750901Ab1CGQqV (ORCPT ); Mon, 7 Mar 2011 11:46:21 -0500 Received: by wyg36 with SMTP id 36so4328145wyg.19 for ; Mon, 07 Mar 2011 08:46:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:sender:from:to:subject:date:message-id :user-agent:mime-version:content-type; bh=oLQckYzUW5owY4i4Zd/33ZFKyIC/+4BI2NcxFVeNM0s=; b=PQKH9hpqR0YRMIJan7YuNZwthXgbjbYwxMLGKqp0I4OI4ClXETw/Q38UYzfHW4rSHq 8aJ7FApPCQIyAGBABYMx+dDnt10oopMfFmGT64Xu1c6nD7UqtIcZBRu0wOVpGTVFSvF0 ygfSJgLwFLkpkSD/tU8J6cj/7dKXb+GD8XUn8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:from:to:subject:date:message-id:user-agent:mime-version :content-type; b=Zq+wtWr0wxKLwo9fYkp5KRBdDg6dntDp++fHWYNCfAB8tcu/HOzca5UMt8NYYbfYJK jS7u9GU5T/GlVGhdsOT8dDXpzUEI6Ix+KUm/sqtbxMr8i1AES9x+cy+7smmKD58igRBl TmLJkqMDP08MnKO20mTNtswNf8fFjv6FpWkZ0= Received: by 10.216.179.196 with SMTP id h46mr3507028wem.78.1299516380336; Mon, 07 Mar 2011 08:46:20 -0800 (PST) Received: from macbook.be.48ers.dk (191.207-78-194.adsl-fix.skynet.be [194.78.207.191]) by mx.google.com with ESMTPS id t11sm1358971wes.41.2011.03.07.08.46.16 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 07 Mar 2011 08:46:19 -0800 (PST) Received: by macbook.be.48ers.dk (Postfix, from userid 1000) id 68BF1C3CCE; Mon, 7 Mar 2011 17:46:08 +0100 (CET) From: Peter Korsgaard To: davem@davemloft.net, buytenh@wantstofly.org, netdev@vger.kernel.org Subject: RFC: dsa slave open handling Date: Mon, 07 Mar 2011 17:46:08 +0100 Message-ID: <871v2inajz.fsf@macbook.be.48ers.dk> User-Agent: Gnus/5.110009 (No Gnus v0.9) Emacs/22.3 (gnu/linux) MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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=::::: (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: Which is preferred? 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); }