Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752208Ab2HMRxC (ORCPT ); Mon, 13 Aug 2012 13:53:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56697 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750861Ab2HMRw7 (ORCPT ); Mon, 13 Aug 2012 13:52:59 -0400 Date: Mon, 13 Aug 2012 14:52:17 -0300 From: Flavio Leitner To: Jiri Pirko Cc: netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com, faisal.latif@intel.com, roland@kernel.org, sean.hefty@intel.com, hal.rosenstock@gmail.com, fubar@us.ibm.com, andy@greyhouse.net, divy@chelsio.com, jitendra.kalsaria@qlogic.com, sony.chacko@qlogic.com, linux-driver@qlogic.com, kaber@trash.net, ursula.braun@de.ibm.com, blaschka@linux.vnet.ibm.com, linux390@de.ibm.com, shemminger@vyatta.com, bhutchings@solarflare.com, therbert@google.com, xiyou.wangcong@gmail.com, joe@perches.com, gregory.v.rose@intel.com, john.r.fastabend@intel.com, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org, bridge@lists.linux-foundation.org Subject: Re: [patch net-next 01/16] net: introduce upper device lists Message-ID: <20120813145217.38748c4f@obelix.rh> In-Reply-To: <1344871635-1052-2-git-send-email-jiri@resnulli.us> References: <1344871635-1052-1-git-send-email-jiri@resnulli.us> <1344871635-1052-2-git-send-email-jiri@resnulli.us> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10597 Lines: 344 On Mon, 13 Aug 2012 17:27:00 +0200 Jiri Pirko wrote: > This lists are supposed to serve for storing pointers to all upper devices. > Eventually it will replace dev->master pointer which is used for > bonding, bridge, team but it cannot be used for vlan, macvlan where > there might be multiple "masters" present. > > New upper device list resolves this limitation. Also, the information > stored in lists is used for preventing looping setups like > "bond->somethingelse->samebond" > > Signed-off-by: Jiri Pirko > --- > include/linux/netdevice.h | 14 +++ > net/core/dev.c | 232 ++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 244 insertions(+), 2 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index a9db4f3..e7a07f8 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -1173,6 +1173,8 @@ struct net_device { > * which this device is member of. > */ > > + struct list_head upper_dev_list; /* List of upper devices */ > + > /* Interface address info used in eth_type_trans() */ > unsigned char *dev_addr; /* hw address, (before bcast > because most packets are > @@ -2611,6 +2613,18 @@ extern int netdev_max_backlog; > extern int netdev_tstamp_prequeue; > extern int weight_p; > extern int bpf_jit_enable; > + > +extern bool netdev_has_upper_dev(struct net_device *dev, > + struct net_device *upper_dev); > +extern bool netdev_has_any_upper_dev(struct net_device *dev); > +extern struct net_device *netdev_unique_upper_dev_get(struct net_device *dev); > +extern struct net_device *netdev_unique_upper_dev_get_rcu(struct net_device *dev); > +extern int netdev_upper_dev_link(struct net_device *dev, > + struct net_device *upper_dev); > +extern int netdev_unique_upper_dev_link(struct net_device *dev, > + struct net_device *upper_dev); > +extern void netdev_upper_dev_unlink(struct net_device *dev, > + struct net_device *upper_dev); > extern int netdev_set_master(struct net_device *dev, struct net_device *master); > extern int netdev_set_bond_master(struct net_device *dev, > struct net_device *master); > diff --git a/net/core/dev.c b/net/core/dev.c > index 1f06df8..68db1ac 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -4425,6 +4425,229 @@ static int __init dev_proc_init(void) > #endif /* CONFIG_PROC_FS */ > > > +struct netdev_upper { > + struct net_device *dev; > + bool unique; unique is quite confusing here. I see that it is possible to have one unique and many non-unique linked in the list, so maybe rename to 'main_dev', 'master' or 'principal'... > + struct list_head list; > + struct rcu_head rcu; > +}; > + > +static bool __netdev_has_upper_dev(struct net_device *dev, > + struct net_device *upper_dev, > + bool deep) > +{ > + struct netdev_upper *upper; > + > + list_for_each_entry(upper, &dev->upper_dev_list, list) { > + if (upper->dev == upper_dev) > + return true; > + if (deep && __netdev_has_upper_dev(upper->dev, upper_dev, deep)) > + return true; > + } > + return false; > +} > + > +static struct netdev_upper *__netdev_find_upper(struct net_device *dev, > + struct net_device *upper_dev) > +{ > + struct netdev_upper *upper; > + > + list_for_each_entry(upper, &dev->upper_dev_list, list) { > + if (upper->dev == upper_dev) > + return upper; > + } > + return NULL; > +} > + > +/** > + * netdev_has_upper_dev - Check if device is linked to an upper device > + * @dev: device > + * @upper_dev: upper device to check > + * > + * Find out if a device is linked to specified upper device and return true > + * in case it is. The caller must hold the RTNL semaphore. > + */ > +bool netdev_has_upper_dev(struct net_device *dev, > + struct net_device *upper_dev) > +{ > + ASSERT_RTNL(); > + > + return __netdev_has_upper_dev(dev, upper_dev, false); > +} > +EXPORT_SYMBOL(netdev_has_upper_dev); > + > +/** > + * netdev_has_any_upper_dev - Check if device is linked to some device > + * @dev: device > + * > + * Find out if a device is linked to an upper device and return true in case > + * it is. The caller must hold the RTNL semaphore. > + */ > +bool netdev_has_any_upper_dev(struct net_device *dev) > +{ > + ASSERT_RTNL(); > + > + return !list_empty(&dev->upper_dev_list); > +} > +EXPORT_SYMBOL(netdev_has_any_upper_dev); > + > +/** > + * netdev_unique_upper_dev_get - Get unique upper device > + * @dev: device > + * > + * Find a unique upper device and return pointer to it or NULL in case > + * it's not there. The caller must hold the RTNL semaphore. > + */ > +struct net_device *netdev_unique_upper_dev_get(struct net_device *dev) > +{ > + struct netdev_upper *upper; > + > + ASSERT_RTNL(); > + > + if (list_empty(&dev->upper_dev_list)) > + return NULL; > + > + upper = list_first_entry(&dev->upper_dev_list, > + struct netdev_upper, list); > + if (likely(upper->unique)) > + return upper->dev; > + return NULL; > +} > +EXPORT_SYMBOL(netdev_unique_upper_dev_get); > + > +/** > + * netdev_unique_upper_dev_get_rcu - Get unique upper device > + * @dev: device > + * > + * Find a unique upper device and return pointer to it or NULL in case > + * it's not there. The caller must hold the RCU read lock. > + */ > +struct net_device *netdev_unique_upper_dev_get_rcu(struct net_device *dev) > +{ > + struct netdev_upper *upper; > + > + upper = list_first_or_null_rcu(&dev->upper_dev_list, > + struct netdev_upper, list); > + if (likely(upper->unique)) It will oopses here if 'upper' is NULL (i.e. no upper devices). > + return upper->dev; > + return NULL; > +} > +EXPORT_SYMBOL(netdev_unique_upper_dev_get_rcu); > + > +static int __netdev_upper_dev_link(struct net_device *dev, > + struct net_device *upper_dev, bool unique) > +{ > + struct netdev_upper *upper; > + > + ASSERT_RTNL(); > + > + if (dev == upper_dev) > + return -EBUSY; > + /* > + * To prevent loops, check if dev is not upper device to upper_dev. > + */ > + if (__netdev_has_upper_dev(upper_dev, dev, true)) > + return -EBUSY; > + > + if (__netdev_find_upper(dev, upper_dev)) > + return -EEXIST; __netdev_has_upper_dev() can go all the way up finding the device and the __netdev_find_upper() just check the first level. I think it would be better to use: __netdev_find_upper_dev(,,deep=true/false) __netdev_has_upper(,) thanks, fbl > + if (unique && netdev_unique_upper_dev_get(dev)) > + return -EBUSY; > + > + upper = kmalloc(sizeof(*upper), GFP_KERNEL); > + if (!upper) > + return -ENOMEM; > + > + upper->dev = upper_dev; > + upper->unique = unique; > + > + /* > + * Ensure that unique upper link is always the first item in the list. > + */ > + if (unique) > + list_add_rcu(&upper->list, &dev->upper_dev_list); > + else > + list_add_tail_rcu(&upper->list, &dev->upper_dev_list); > + dev_hold(upper_dev); > + > + return 0; > +} > +/** > + * netdev_upper_dev_link - Add a link to the upper device > + * @dev: device > + * @upper_dev: new upper device > + * > + * Adds a link to device which is upper to this one. The caller must hold > + * the RTNL semaphore. On a failure a negative errno code is returned. > + * On success the reference counts are adjusted and the function > + * returns zero. > + */ > +int netdev_upper_dev_link(struct net_device *dev, > + struct net_device *upper_dev) > +{ > + return __netdev_upper_dev_link(dev, upper_dev, false); > +} > +EXPORT_SYMBOL(netdev_upper_dev_link); > + > +/** > + * netdev_unique_upper_dev_link - Add a unique link to the upper device > + * @dev: device > + * @upper_dev: new upper device > + * > + * Adds a link to device which is upper to this one. In this case, only > + * one unique upper device can be linked, although other non-unique devices > + * might be linked as well. The caller must hold the RTNL semaphore. > + * On a failure a negative errno code is returned. On success the reference > + * counts are adjusted and the function returns zero. > + */ > +int netdev_unique_upper_dev_link(struct net_device *dev, > + struct net_device *upper_dev) > +{ > + return __netdev_upper_dev_link(dev, upper_dev, true); > +} > +EXPORT_SYMBOL(netdev_unique_upper_dev_link); > + > +/** > + * netdev_upper_free_rcu - Frees a upper device list item via the RCU pointer > + * @entry: the entry's RCU field > + * > + * This function is designed to be used as a callback to the call_rcu() > + * function so that the memory allocated to the netdev upper device list item > + * can be released safely. > + */ > +static void netdev_upper_free_rcu(struct rcu_head *entry) > +{ > + struct netdev_upper *upper; > + > + upper = container_of(entry, struct netdev_upper, rcu); > + kfree(upper); > +} > + > +/** > + * netdev_upper_dev_unlink - Removes a link to upper device > + * @dev: device > + * @upper_dev: new upper device > + * > + * Removes a link to device which is upper to this one. The caller must hold > + * the RTNL semaphore. > + */ > +void netdev_upper_dev_unlink(struct net_device *dev, > + struct net_device *upper_dev) > +{ > + struct netdev_upper *upper; > + > + ASSERT_RTNL(); > + > + upper = __netdev_find_upper(dev, upper_dev); > + if (!upper) > + return; > + list_del_rcu(&upper->list); > + dev_put(upper_dev); > + call_rcu(&upper->rcu, netdev_upper_free_rcu); > +} > +EXPORT_SYMBOL(netdev_upper_dev_unlink); > + > /** > * netdev_set_master - set up master pointer > * @slave: slave device > @@ -4438,19 +4661,23 @@ static int __init dev_proc_init(void) > int netdev_set_master(struct net_device *slave, struct net_device *master) > { > struct net_device *old = slave->master; > + int err; > > ASSERT_RTNL(); > > if (master) { > if (old) > return -EBUSY; > - dev_hold(master); > + err = netdev_unique_upper_dev_link(slave, master); > + if (err) > + return err; > } > > slave->master = master; > > if (old) > - dev_put(old); > + netdev_upper_dev_unlink(slave, master); > + > return 0; > } > EXPORT_SYMBOL(netdev_set_master); > @@ -5999,6 +6226,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, > INIT_LIST_HEAD(&dev->napi_list); > INIT_LIST_HEAD(&dev->unreg_list); > INIT_LIST_HEAD(&dev->link_watch_list); > + INIT_LIST_HEAD(&dev->upper_dev_list); > dev->priv_flags = IFF_XMIT_DST_RELEASE; > setup(dev); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/