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
| ||
|
Message-ID: <1344877451.2733.26.camel@bwh-desktop.uk.solarflarecom.com> Date: Mon, 13 Aug 2012 18:04:11 +0100 From: Ben Hutchings <bhutchings@...arflare.com> To: Jiri Pirko <jiri@...nulli.us> CC: <netdev@...r.kernel.org>, <davem@...emloft.net>, <edumazet@...gle.com>, <faisal.latif@...el.com>, <roland@...nel.org>, <sean.hefty@...el.com>, <hal.rosenstock@...il.com>, <fubar@...ibm.com>, <andy@...yhouse.net>, <divy@...lsio.com>, <jitendra.kalsaria@...gic.com>, <sony.chacko@...gic.com>, <linux-driver@...gic.com>, <kaber@...sh.net>, <ursula.braun@...ibm.com>, <blaschka@...ux.vnet.ibm.com>, <linux390@...ibm.com>, <shemminger@...tta.com>, <therbert@...gle.com>, <xiyou.wangcong@...il.com>, <joe@...ches.com>, <gregory.v.rose@...el.com>, <john.r.fastabend@...el.com>, <linux-rdma@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <linux-s390@...r.kernel.org>, <bridge@...ts.linux-foundation.org>, <fbl@...hat.com> Subject: Re: [patch net-next 01/16] net: introduce upper device lists On Mon, 2012-08-13 at 17:27 +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 <jiri@...nulli.us> [...] > --- 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; This needs a better name. It doesn't really have anything to do with uniqueness and doesn't ensure exclusivity. I think that it would be fine to keep the 'master' term. > + struct list_head list; > + struct rcu_head 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; > + > + 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); This behaviour (calling dev_hold()) matches netdev_set_master(). But it's oddly asymmetric: generally the administrator can remove either the upper device or the lower device (rtnl_link_ops or unbinding a physical device) and the upper device driver must then unlink itself from the lower device (using a notifier to catch lower device removal). If the upper device driver fails to unlink when the upper device is unregistered, then this extra reference causes netdev_wait_allrefs() to hang... is that the intent? Or should there be a more explicit counter and check on unregistration, e.g. WARN_ON(dev->num_lower_devs != 0)? If it fails to unlink when the lower device is removed, this warning in rollback_registered_many() may be triggered: /* Notifier chain MUST detach us from master device. */ WARN_ON(dev->master); I think that needs to become WARN_ON(netdev_has_upper_dev(dev)). > + return 0; > +} [...] -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists