[PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
This patch introduces netdev_walk_all_upper_dev_rcu, netdev_walk_all_lower_dev and netdev_walk_all_lower_dev_rcu. These functions recursively walk the adj_list of devices to determine all upper and lower devices. The functions take a callback function that is invoked for each device in the list. If the callback returns non-0, the walk is terminated and the functions return that code back to callers. v3 - simplified netdev_has_upper_dev_all_rcu and __netdev_has_upper_dev and removed typecast as suggested by Stephen v2 - fixed definition of netdev_next_lower_dev_rcu to mirror the upper_dev version. Signed-off-by: David Ahern --- include/linux/netdevice.h | 17 + net/core/dev.c| 155 ++ 2 files changed, 172 insertions(+) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index bf341b65ca5e..a5902d995907 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3778,6 +3778,14 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev, updev; \ updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter))) +int netdev_walk_all_upper_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *upper_dev, + void *data), + void *data); + +bool netdev_has_upper_dev_all_rcu(struct net_device *dev, + struct net_device *upper_dev); + void *netdev_lower_get_next_private(struct net_device *dev, struct list_head **iter); void *netdev_lower_get_next_private_rcu(struct net_device *dev, @@ -3821,6 +3829,15 @@ struct net_device *netdev_all_lower_get_next_rcu(struct net_device *dev, ldev; \ ldev = netdev_all_lower_get_next_rcu(dev, &(iter))) +int netdev_walk_all_lower_dev(struct net_device *dev, + int (*fn)(struct net_device *lower_dev, + void *data), + void *data); +int netdev_walk_all_lower_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *lower_dev, + void *data), + void *data); + void *netdev_adjacent_get_private(struct list_head *adj_list); void *netdev_lower_get_first_private_rcu(struct net_device *dev); struct net_device *netdev_master_upper_dev_get(struct net_device *dev); diff --git a/net/core/dev.c b/net/core/dev.c index f67fd16615bb..fc48337cfab8 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5156,6 +5156,31 @@ bool netdev_has_upper_dev(struct net_device *dev, EXPORT_SYMBOL(netdev_has_upper_dev); /** + * netdev_has_upper_dev_all - 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. Note that this checks the entire upper device chain. + * The caller must hold rcu lock. + */ + +static int __netdev_has_upper_dev(struct net_device *upper_dev, void *data) +{ + struct net_device *dev = data; + + return upper_dev == dev; +} + +bool netdev_has_upper_dev_all_rcu(struct net_device *dev, + struct net_device *upper_dev) +{ + return !!netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev, + upper_dev); +} +EXPORT_SYMBOL(netdev_has_upper_dev_all_rcu); + +/** * netdev_has_any_upper_dev - Check if device is linked to some device * @dev: device * @@ -5255,6 +5280,51 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev, } EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu); +static struct net_device *netdev_next_upper_dev_rcu(struct net_device *dev, + struct list_head **iter) +{ + struct netdev_adjacent *upper; + + WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_rtnl_is_held()); + + upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list); + + if (&upper->list == &dev->adj_list.upper) + return NULL; + + *iter = &upper->list; + + return upper->dev; +} + +int netdev_walk_all_upper_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *dev, + void *data), + void *data) +{ + struct net_device *udev; + struct list_head *iter; + int ret; + + for (iter = &dev->adj_list.upper, +udev = netdev_next_upper_dev_rcu(dev, &iter); +udev; +udev = netdev_next_upper_dev_rcu(dev, &iter)) { + /* first is the upper device itself */ + ret = fn(udev, data); + if (ret) +
Re: [PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
On 10/17/16 6:21 AM, Stephen Hemminger wrote: > > No if/else needed. No cast of void * ptr need. Use const if possible? > so much of the stack does not use const and trying to add it for this API does not work -- the upper or lower device is passed to the callbacks and those callbacks invoke other apis. e.g., the bond patch calls vlan_get_encap_level, bond_verify_device_path and bond_confirm_addr and none of those accept a const dev. v3 coming up with the more succinct versions, but const is not possible.
Re: [PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
From: Stephen Hemminger Date: Mon, 17 Oct 2016 05:21:21 -0700 > You should write this more succinctly as: > > static bool __netdev_has_upper_dev(struct net_device *upper_dev, void *data) > { > struct net_device *dev = data; > > return upper_dev == dev; > } > > bool netdev_has_upper_dev_all_rcu(struct net_device *dev, > struct net_device *upper_dev) > { > return netdev_walk_all_upper_dev_rcu(dev, > __netdev_has_upper_dev, upper_dev); > } > > No if/else needed. No cast of void * ptr need. Use const if possible? Agreed.
Re: [PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
On Fri, 14 Oct 2016 18:28:42 -0700 David Ahern wrote: > > /** > + * netdev_has_upper_dev_all - 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. Note that this checks the entire upper device chain. > + * The caller must hold rcu lock. > + */ > + > +static int __netdev_has_upper_dev(struct net_device *upper_dev, void *data) > +{ > + struct net_device *dev = (struct net_device *)data; > + > + if (upper_dev == dev) > + return 1; > + > + return 0; > +} > + > +bool netdev_has_upper_dev_all_rcu(struct net_device *dev, > + struct net_device *upper_dev) > +{ > + if (netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev, > + upper_dev)) > + return true; > + > + return false; > +} > +EXPORT_SYMBOL(netdev_has_upper_dev_all_rcu); You should write this more succinctly as: static bool __netdev_has_upper_dev(struct net_device *upper_dev, void *data) { struct net_device *dev = data; return upper_dev == dev; } bool netdev_has_upper_dev_all_rcu(struct net_device *dev, struct net_device *upper_dev) { return netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev, upper_dev); } No if/else needed. No cast of void * ptr need. Use const if possible?
[PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
This patch introduces netdev_walk_all_upper_dev_rcu, netdev_walk_all_lower_dev and netdev_walk_all_lower_dev_rcu. These functions recursively walk the adj_list of devices to determine all upper and lower devices. The functions take a callback function that is invoked for each device in the list. If the callback returns non-0, the walk is terminated and the functions return that code back to callers. v2 - fixed definition of netdev_next_lower_dev_rcu to mirror the upper_dev version. Signed-off-by: David Ahern --- include/linux/netdevice.h | 17 + net/core/dev.c| 161 ++ 2 files changed, 178 insertions(+) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index bf341b65ca5e..a5902d995907 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3778,6 +3778,14 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev, updev; \ updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter))) +int netdev_walk_all_upper_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *upper_dev, + void *data), + void *data); + +bool netdev_has_upper_dev_all_rcu(struct net_device *dev, + struct net_device *upper_dev); + void *netdev_lower_get_next_private(struct net_device *dev, struct list_head **iter); void *netdev_lower_get_next_private_rcu(struct net_device *dev, @@ -3821,6 +3829,15 @@ struct net_device *netdev_all_lower_get_next_rcu(struct net_device *dev, ldev; \ ldev = netdev_all_lower_get_next_rcu(dev, &(iter))) +int netdev_walk_all_lower_dev(struct net_device *dev, + int (*fn)(struct net_device *lower_dev, + void *data), + void *data); +int netdev_walk_all_lower_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *lower_dev, + void *data), + void *data); + void *netdev_adjacent_get_private(struct list_head *adj_list); void *netdev_lower_get_first_private_rcu(struct net_device *dev); struct net_device *netdev_master_upper_dev_get(struct net_device *dev); diff --git a/net/core/dev.c b/net/core/dev.c index 5399af8fdac0..1780f94ed25f 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5156,6 +5156,37 @@ bool netdev_has_upper_dev(struct net_device *dev, EXPORT_SYMBOL(netdev_has_upper_dev); /** + * netdev_has_upper_dev_all - 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. Note that this checks the entire upper device chain. + * The caller must hold rcu lock. + */ + +static int __netdev_has_upper_dev(struct net_device *upper_dev, void *data) +{ + struct net_device *dev = (struct net_device *)data; + + if (upper_dev == dev) + return 1; + + return 0; +} + +bool netdev_has_upper_dev_all_rcu(struct net_device *dev, + struct net_device *upper_dev) +{ + if (netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev, + upper_dev)) + return true; + + return false; +} +EXPORT_SYMBOL(netdev_has_upper_dev_all_rcu); + +/** * netdev_has_any_upper_dev - Check if device is linked to some device * @dev: device * @@ -5255,6 +5286,51 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev, } EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu); +static struct net_device *netdev_next_upper_dev_rcu(struct net_device *dev, + struct list_head **iter) +{ + struct netdev_adjacent *upper; + + WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_rtnl_is_held()); + + upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list); + + if (&upper->list == &dev->adj_list.upper) + return NULL; + + *iter = &upper->list; + + return upper->dev; +} + +int netdev_walk_all_upper_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *dev, + void *data), + void *data) +{ + struct net_device *udev; + struct list_head *iter; + int ret; + + for (iter = &dev->adj_list.upper, +udev = netdev_next_upper_dev_rcu(dev, &iter); +udev; +udev = netdev_next_upper_dev_rcu(dev, &iter)) { + /* first is the upper device itself */ + ret = fn(udev, data); + if (ret) + return ret;
Re: [PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
Wed, Oct 12, 2016 at 10:51:50PM CEST, d...@cumulusnetworks.com wrote: >This patch introduces netdev_walk_all_upper_dev_rcu, >netdev_walk_all_lower_dev and netdev_walk_all_lower_dev_rcu. These >functions recursively walk the adj_list of devices to determine all upper >and lower devices. > >The functions take a callback function that is invoked for each device >in the list. If the callback returns non-0, the walk is terminated and >the functions return that code back to callers. > >Signed-off-by: David Ahern [...] >+int netdev_walk_all_lower_dev(struct net_device *dev, >+int (*fn)(struct net_device *dev, >+ void *data), >+void *data) >+{ >+ struct list_head *iter; >+ struct net_device *ldev; >+ int ret; >+ >+ for (iter = &(dev)->adj_list.lower, >+ ldev = netdev_next_lower_dev(dev, &(iter)); >+ ldev; >+ ldev = netdev_next_lower_dev(dev, &(iter))) { >+ /* first is the lower device itself */ >+ ret = fn(ldev, data); >+ if (ret) >+ return ret; >+ >+ /* then look at all of its lower devices */ >+ ret = netdev_walk_all_lower_dev(ldev, fn, data); I believe that Veaceslav's reason to collapse the upper/lower trees was to avoid this recursivity. I also believe that the recursivity was a big issue for DaveM and BenH. Something changed?
[PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices
This patch introduces netdev_walk_all_upper_dev_rcu, netdev_walk_all_lower_dev and netdev_walk_all_lower_dev_rcu. These functions recursively walk the adj_list of devices to determine all upper and lower devices. The functions take a callback function that is invoked for each device in the list. If the callback returns non-0, the walk is terminated and the functions return that code back to callers. Signed-off-by: David Ahern --- include/linux/netdevice.h | 17 + net/core/dev.c| 158 ++ 2 files changed, 175 insertions(+) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 136ae6bbe81e..053f6f75f26a 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3839,6 +3839,14 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev, updev; \ updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter))) +int netdev_walk_all_upper_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *lower_dev, + void *data), + void *data); + +bool netdev_has_upper_dev_all_rcu(struct net_device *dev, + struct net_device *upper_dev); + void *netdev_lower_get_next_private(struct net_device *dev, struct list_head **iter); void *netdev_lower_get_next_private_rcu(struct net_device *dev, @@ -3882,6 +3890,15 @@ struct net_device *netdev_all_lower_get_next_rcu(struct net_device *dev, ldev; \ ldev = netdev_all_lower_get_next_rcu(dev, &(iter))) +int netdev_walk_all_lower_dev(struct net_device *dev, + int (*fn)(struct net_device *lower_dev, + void *data), + void *data); +int netdev_walk_all_lower_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *lower_dev, + void *data), + void *data); + void *netdev_adjacent_get_private(struct list_head *adj_list); void *netdev_lower_get_first_private_rcu(struct net_device *dev); struct net_device *netdev_master_upper_dev_get(struct net_device *dev); diff --git a/net/core/dev.c b/net/core/dev.c index 5aaa12a9e369..fcf3641db783 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5292,6 +5292,37 @@ bool netdev_has_upper_dev(struct net_device *dev, EXPORT_SYMBOL(netdev_has_upper_dev); /** + * netdev_has_upper_dev_all - 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. Note that this checks the entire upper device chain. + * The caller must hold rcu lock. + */ + +static int __netdev_has_upper_dev(struct net_device *upper_dev, void *data) +{ + struct net_device *dev = (struct net_device *)data; + + if (upper_dev == dev) + return 1; + + return 0; +} + +bool netdev_has_upper_dev_all_rcu(struct net_device *dev, + struct net_device *upper_dev) +{ + if (netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev, + upper_dev)) + return true; + + return false; +} +EXPORT_SYMBOL(netdev_has_upper_dev_all_rcu); + +/** * netdev_has_any_upper_dev - Check if device is linked to some device * @dev: device * @@ -5391,6 +5422,51 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev, } EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu); +static struct net_device *netdev_next_upper_dev_rcu(struct net_device *dev, + struct list_head **iter) +{ + struct netdev_adjacent *upper; + + WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_rtnl_is_held()); + + upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list); + + if (&upper->list == &dev->adj_list.upper) + return NULL; + + *iter = &upper->list; + + return upper->dev; +} + +int netdev_walk_all_upper_dev_rcu(struct net_device *dev, + int (*fn)(struct net_device *dev, + void *data), + void *data) +{ + struct list_head *iter; + struct net_device *udev; + int ret; + + for (iter = &(dev)->adj_list.upper, +udev = netdev_next_upper_dev_rcu(dev, &(iter)); +udev; +udev = netdev_next_upper_dev_rcu(dev, &(iter))) { + /* first is the upper device itself */ + ret = fn(udev, data); + if (ret) + return ret; + + /* then look at all of its upper devices */ + re