| 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: <20120813145217.38748c4f@obelix.rh>
Date: Mon, 13 Aug 2012 14:52:17 -0300
From: Flavio Leitner <fbl@...hat.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,
bhutchings@...arflare.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
Subject: Re: [patch net-next 01/16] net: introduce upper device lists
On Mon, 13 Aug 2012 17:27:00 +0200
Jiri Pirko <jiri@...nulli.us> 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>
> ---
> 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 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