On Wed, 2013-01-02 at 13:28 +0100, 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 upper present. In case the upper link is
> replacement for dev->master, it is marked with "master" flag.
>
> New upper device list resolves this limitation. Also, the information
> stored in lists is used for preventing looping setups like
> "bond->somethingelse->samebond"
Thanks for continuing with this, Jiri. I just see some
cosmetic/documentation issues:
[...]
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
[...]
> +static bool __netdev_has_upper_dev(struct net_device *dev,
> + struct net_device *upper_dev)
> +{
> + LIST_HEAD(search_list);
> + struct netdev_upper *upper;
> + struct netdev_upper *tmp;
> + bool ret = false;
> +
> + __append_search_uppers(&search_list, dev);
> + list_for_each_entry(upper, &search_list, search_list) {
> + if (upper->dev == upper_dev) {
> + ret = true;
> + break;
> + }
> + __append_search_uppers(&search_list, upper->dev);
> + }
> + list_for_each_entry_safe(upper, tmp, &search_list, search_list)
> + INIT_LIST_HEAD(&upper->search_list);
> + return ret;
> +}
> +
> +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
Please clarify that this (and other functions) only checks for an
immediate upper device and not through a complete stack of devices.
> + * @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.
It is no longer a semaphore, even if some kernel-doc in this file calls
it that. 'RTNL lock' would be better as it matches the function naming:
rtnl_lock() etc.
> + */
> +bool netdev_has_upper_dev(struct net_device *dev,
> + struct net_device *upper_dev)
The '__' prefix normally implies doing less work than the un-prefixed
function but __netdev_has_upper_dev() checks all devices stacked above
dev whereas this only checks a single level. Therefore I think one or
both should be renamed.
> +{
> + ASSERT_RTNL();
> +
> + return __netdev_find_upper(dev, upper_dev);
> +}
> +EXPORT_SYMBOL(netdev_has_upper_dev);
[...]
> +static int __netdev_upper_dev_link(struct net_device *dev,
> + struct net_device *upper_dev, bool master)
> +{
[...]
> +}
> +/**
> + * netdev_upper_dev_link - Add a link to the upper device
[...]
There should be a blank line between the closing brace and the comment
for the next function.
Ben.
--
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.
