[PATCH net-next 02/11] net: Introduce new api for walking upper and lower devices

2016-10-17 Thread David Ahern 
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 (>list == >adj_list.upper)
+   return NULL;
+
+   *iter = >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 = >adj_list.upper,
+udev = netdev_next_upper_dev_rcu(dev, );
+udev;
+udev = netdev_next_upper_dev_rcu(dev, )) {
+   /* 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

2016-10-17 Thread David Ahern 
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

2016-10-17 Thread David Miller 
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

2016-10-17 Thread Stephen Hemminger 
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

2016-10-14 Thread David Ahern 
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 (>list == >adj_list.upper)
+   return NULL;
+
+   *iter = >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 = >adj_list.upper,
+udev = netdev_next_upper_dev_rcu(dev, );
+udev;
+udev = netdev_next_upper_dev_rcu(dev, )) {
+   /* 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

2016-10-13 Thread Jiri Pirko 
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

2016-10-12 Thread David Ahern 
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 (>list == >adj_list.upper)
+   return NULL;
+
+   *iter = >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 */
+