[PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[Sven Eckelmann]

The initialization of an net_device object should be done in the
init/constructor function and not from the outside after the register_netdevice
was done to avoid race conditions.

Signed-off-by: Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
---
 net/batman-adv/hard-interface.c |    5 ++
 net/batman-adv/soft-interface.c |  147 ++++++++++++++++++---------------------
 2 files changed, 73 insertions(+), 79 deletions(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 365ed74..50f4a5c 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -542,6 +542,11 @@ static int batadv_hard_if_event(struct notifier_block *this,
 	struct batadv_hard_iface *primary_if = NULL;
 	struct batadv_priv *bat_priv;
 
+	if (batadv_softif_is_valid(net_dev) && event == NETDEV_REGISTER) {
+		batadv_sysfs_add_meshif(net_dev);
+		return NOTIFY_DONE;
+	}
+
 	hard_iface = batadv_hardif_get_by_netdev(net_dev);
 	if (!hard_iface && event == NETDEV_REGISTER)
 		hard_iface = batadv_hardif_add_interface(net_dev);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 54800c7..b545595 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -407,9 +407,73 @@ static void batadv_set_lockdep_class(struct net_device *dev)
  */
 static int batadv_softif_init(struct net_device *dev)
 {
+	struct batadv_priv *bat_priv;
+	int ret;
+	size_t cnt_len = sizeof(uint64_t) * BATADV_CNT_NUM;
+
 	batadv_set_lockdep_class(dev);
 
+	bat_priv = netdev_priv(dev);
+
+	/* batadv_interface_stats() needs to be available as soon as
+	 * register_netdevice() has been called
+	 */
+	bat_priv->bat_counters = __alloc_percpu(cnt_len, __alignof__(uint64_t));
+	if (!bat_priv->bat_counters)
+		return -ENOMEM;
+
+	atomic_set(&bat_priv->aggregated_ogms, 1);
+	atomic_set(&bat_priv->bonding, 0);
+	atomic_set(&bat_priv->bridge_loop_avoidance, 0);
+#ifdef CONFIG_BATMAN_ADV_DAT
+	atomic_set(&bat_priv->distributed_arp_table, 1);
+#endif
+	atomic_set(&bat_priv->ap_isolation, 0);
+	atomic_set(&bat_priv->vis_mode, BATADV_VIS_TYPE_CLIENT_UPDATE);
+	atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF);
+	atomic_set(&bat_priv->gw_sel_class, 20);
+	atomic_set(&bat_priv->gw_bandwidth, 41);
+	atomic_set(&bat_priv->orig_interval, 1000);
+	atomic_set(&bat_priv->hop_penalty, 30);
+	atomic_set(&bat_priv->log_level, 0);
+	atomic_set(&bat_priv->fragmentation, 1);
+	atomic_set(&bat_priv->bcast_queue_left, BATADV_BCAST_QUEUE_LEN);
+	atomic_set(&bat_priv->batman_queue_left, BATADV_BATMAN_QUEUE_LEN);
+
+	atomic_set(&bat_priv->mesh_state, BATADV_MESH_INACTIVE);
+	atomic_set(&bat_priv->bcast_seqno, 1);
+	atomic_set(&bat_priv->tt.vn, 0);
+	atomic_set(&bat_priv->tt.local_changes, 0);
+	atomic_set(&bat_priv->tt.ogm_append_cnt, 0);
+#ifdef CONFIG_BATMAN_ADV_BLA
+	atomic_set(&bat_priv->bla.num_requests, 0);
+#endif
+	bat_priv->tt.last_changeset = NULL;
+	bat_priv->tt.last_changeset_len = 0;
+
+	bat_priv->primary_if = NULL;
+	bat_priv->num_ifaces = 0;
+
+	ret = batadv_algo_select(bat_priv, batadv_routing_algo);
+	if (ret < 0)
+		goto free_bat_counters;
+
+	ret = batadv_debugfs_add_meshif(dev);
+	if (ret < 0)
+		goto free_bat_counters;
+
+	ret = batadv_mesh_init(dev);
+	if (ret < 0)
+		goto unreg_debugfs;
+
 	return 0;
+
+unreg_debugfs:
+	batadv_debugfs_del_meshif(dev);
+free_bat_counters:
+	free_percpu(bat_priv->bat_counters);
+
+	return ret;
 }
 
 static const struct net_device_ops batadv_netdev_ops = {
@@ -451,97 +515,22 @@ static void batadv_interface_setup(struct net_device *dev)
 struct net_device *batadv_softif_create(const char *name)
 {
 	struct net_device *soft_iface;
-	struct batadv_priv *bat_priv;
 	int ret;
-	size_t cnt_len = sizeof(uint64_t) * BATADV_CNT_NUM;
 
-	soft_iface = alloc_netdev(sizeof(*bat_priv), name,
+	soft_iface = alloc_netdev(sizeof(struct batadv_priv), name,
 				  batadv_interface_setup);
-
 	if (!soft_iface)
-		goto out;
-
-	bat_priv = netdev_priv(soft_iface);
-
-	/* batadv_interface_stats() needs to be available as soon as
-	 * register_netdevice() has been called
-	 */
-	bat_priv->bat_counters = __alloc_percpu(cnt_len, __alignof__(uint64_t));
-	if (!bat_priv->bat_counters)
-		goto free_soft_iface;
+		return NULL;
 
 	ret = register_netdevice(soft_iface);
 	if (ret < 0) {
 		pr_err("Unable to register the batman interface '%s': %i\n",
 		       name, ret);
-		goto free_bat_counters;
+		free_netdev(soft_iface);
+		return NULL;
 	}
 
-	atomic_set(&bat_priv->aggregated_ogms, 1);
-	atomic_set(&bat_priv->bonding, 0);
-	atomic_set(&bat_priv->bridge_loop_avoidance, 0);
-#ifdef CONFIG_BATMAN_ADV_DAT
-	atomic_set(&bat_priv->distributed_arp_table, 1);
-#endif
-	atomic_set(&bat_priv->ap_isolation, 0);
-	atomic_set(&bat_priv->vis_mode, BATADV_VIS_TYPE_CLIENT_UPDATE);
-	atomic_set(&bat_priv->gw_mode, BATADV_GW_MODE_OFF);
-	atomic_set(&bat_priv->gw_sel_class, 20);
-	atomic_set(&bat_priv->gw_bandwidth, 41);
-	atomic_set(&bat_priv->orig_interval, 1000);
-	atomic_set(&bat_priv->hop_penalty, 30);
-	atomic_set(&bat_priv->log_level, 0);
-	atomic_set(&bat_priv->fragmentation, 1);
-	atomic_set(&bat_priv->bcast_queue_left, BATADV_BCAST_QUEUE_LEN);
-	atomic_set(&bat_priv->batman_queue_left, BATADV_BATMAN_QUEUE_LEN);
-
-	atomic_set(&bat_priv->mesh_state, BATADV_MESH_INACTIVE);
-	atomic_set(&bat_priv->bcast_seqno, 1);
-	atomic_set(&bat_priv->tt.vn, 0);
-	atomic_set(&bat_priv->tt.local_changes, 0);
-	atomic_set(&bat_priv->tt.ogm_append_cnt, 0);
-#ifdef CONFIG_BATMAN_ADV_BLA
-	atomic_set(&bat_priv->bla.num_requests, 0);
-#endif
-	bat_priv->tt.last_changeset = NULL;
-	bat_priv->tt.last_changeset_len = 0;
-
-	bat_priv->primary_if = NULL;
-	bat_priv->num_ifaces = 0;
-
-	ret = batadv_algo_select(bat_priv, batadv_routing_algo);
-	if (ret < 0)
-		goto unreg_soft_iface;
-
-	ret = batadv_sysfs_add_meshif(soft_iface);
-	if (ret < 0)
-		goto unreg_soft_iface;
-
-	ret = batadv_debugfs_add_meshif(soft_iface);
-	if (ret < 0)
-		goto unreg_sysfs;
-
-	ret = batadv_mesh_init(soft_iface);
-	if (ret < 0)
-		goto unreg_debugfs;
-
 	return soft_iface;
-
-unreg_debugfs:
-	batadv_debugfs_del_meshif(soft_iface);
-unreg_sysfs:
-	batadv_sysfs_del_meshif(soft_iface);
-unreg_soft_iface:
-	free_percpu(bat_priv->bat_counters);
-	unregister_netdevice(soft_iface);
-	return NULL;
-
-free_bat_counters:
-	free_percpu(bat_priv->bat_counters);
-free_soft_iface:
-	free_netdev(soft_iface);
-out:
-	return NULL;
 }
 
 void batadv_softif_destroy(struct net_device *soft_iface)
-- 
1.7.10.4

[PATCH 2/7] batman-adv: Move deinitialization of soft-interface to destructor

[Sven Eckelmann]

The deinitialization of the soft-interface created in ndo_init/constructor
should be done in the destructor and not directly before calling
unregister_netdevice

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/soft-interface.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index b545595..8467b9f 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -487,6 +487,17 @@ static const struct net_device_ops batadv_netdev_ops = {
 	.ndo_validate_addr = eth_validate_addr
 };
 
+/**
+ * batadv_interface_free - Deconstructor of batadv_soft_interface
+ * @dev: Device to cleanup and remove
+ */
+static void batadv_interface_free(struct net_device *dev)
+{
+	batadv_debugfs_del_meshif(dev);
+	batadv_mesh_free(dev);
+	free_netdev(dev);
+}
+
 static void batadv_interface_setup(struct net_device *dev)
 {
 	struct batadv_priv *priv = netdev_priv(dev);
@@ -494,7 +505,7 @@ static void batadv_interface_setup(struct net_device *dev)
 	ether_setup(dev);
 
 	dev->netdev_ops = &batadv_netdev_ops;
-	dev->destructor = free_netdev;
+	dev->destructor = batadv_interface_free;
 	dev->tx_queue_len = 0;
 
 	/* can't call min_mtu, because the needed variables
@@ -535,9 +546,7 @@ struct net_device *batadv_softif_create(const char *name)
 
 void batadv_softif_destroy(struct net_device *soft_iface)
 {
-	batadv_debugfs_del_meshif(soft_iface);
 	batadv_sysfs_del_meshif(soft_iface);
-	batadv_mesh_free(soft_iface);
 	unregister_netdevice(soft_iface);
 }
 
-- 
1.7.10.4

[PATCH 3/7] batman-adv: Don't always delete softif when last slave was removed

[Sven Eckelmann]

batman-adv has an unusual way to manage softinterfaces. These will be created
automatically when a user writes to the batman-adv/mesh_iface file in sysfs and
removed when no slave device exists anymore.

This behaviour cannot be changed without breaking compatibility with existing
code. Instead other interfaces should be able to slightly reduce this behaviour
and provide a more common reaction to a removal of a slave interface.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/hard-interface.c |    8 +++++---
 net/batman-adv/hard-interface.h |   13 ++++++++++++-
 net/batman-adv/sysfs.c          |    6 ++++--
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 50f4a5c..daeb632 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -407,7 +407,8 @@ err:
 	return ret;
 }
 
-void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface)
+void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface,
+				     enum batadv_hard_if_cleanup autodel)
 {
 	struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
 	struct batadv_hard_iface *primary_if = NULL;
@@ -445,7 +446,7 @@ void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface)
 	dev_put(hard_iface->soft_iface);
 
 	/* nobody uses this interface anymore */
-	if (!bat_priv->num_ifaces)
+	if (!bat_priv->num_ifaces && autodel == BATADV_IF_CLEANUP_AUTO)
 		batadv_softif_destroy(hard_iface->soft_iface);
 
 	hard_iface->soft_iface = NULL;
@@ -511,7 +512,8 @@ static void batadv_hardif_remove_interface(struct batadv_hard_iface *hard_iface)
 
 	/* first deactivate interface */
 	if (hard_iface->if_status != BATADV_IF_NOT_IN_USE)
-		batadv_hardif_disable_interface(hard_iface);
+		batadv_hardif_disable_interface(hard_iface,
+						BATADV_IF_CLEANUP_AUTO);
 
 	if (hard_iface->if_status != BATADV_IF_NOT_IN_USE)
 		return;
diff --git a/net/batman-adv/hard-interface.h b/net/batman-adv/hard-interface.h
index 3732366..34cf367 100644
--- a/net/batman-adv/hard-interface.h
+++ b/net/batman-adv/hard-interface.h
@@ -29,13 +29,24 @@ enum batadv_hard_if_state {
 	BATADV_IF_I_WANT_YOU,
 };
 
+/**
+ * enum batadv_hard_if_cleanup - Cleanup modi for soft_iface after slave removal
+ * @BATADV_IF_CLEANUP_KEEP: Don't automatically delete soft-interface
+ * @BATADV_IF_CLEANUP_AUTO: Delete soft-interface after last slave was removed
+ */
+enum batadv_hard_if_cleanup {
+	BATADV_IF_CLEANUP_KEEP,
+	BATADV_IF_CLEANUP_AUTO,
+};
+
 extern struct notifier_block batadv_hard_if_notifier;
 
 struct batadv_hard_iface*
 batadv_hardif_get_by_netdev(const struct net_device *net_dev);
 int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
 				   const char *iface_name);
-void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface);
+void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface,
+				     enum batadv_hard_if_cleanup autodel);
 void batadv_hardif_remove_interfaces(void);
 int batadv_hardif_min_mtu(struct net_device *soft_iface);
 void batadv_update_min_mtu(struct net_device *soft_iface);
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 84a55cb..5d868d3 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -582,13 +582,15 @@ static ssize_t batadv_store_mesh_iface(struct kobject *kobj,
 	}
 
 	if (status_tmp == BATADV_IF_NOT_IN_USE) {
-		batadv_hardif_disable_interface(hard_iface);
+		batadv_hardif_disable_interface(hard_iface,
+						BATADV_IF_CLEANUP_AUTO);
 		goto unlock;
 	}
 
 	/* if the interface already is in use */
 	if (hard_iface->if_status != BATADV_IF_NOT_IN_USE)
-		batadv_hardif_disable_interface(hard_iface);
+		batadv_hardif_disable_interface(hard_iface,
+						BATADV_IF_CLEANUP_AUTO);
 
 	ret = batadv_hardif_enable_interface(hard_iface, buff);
 
-- 
1.7.10.4

[PATCH 4/7] batman-adv: Remove slave interfaces before removing master interface

[Sven Eckelmann]

The master (soft-interface) should remove the references to its soft-interfaces
before it gets destroyed. Otherwise the usage count of the interface will not be
zero and therefore the net_device cannot be removed. This leads to an endless
hang of the unregister call.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/soft-interface.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 8467b9f..5fd0560 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -546,6 +546,14 @@ struct net_device *batadv_softif_create(const char *name)
 
 void batadv_softif_destroy(struct net_device *soft_iface)
 {
+	struct batadv_hard_iface *hard_iface;
+
+	list_for_each_entry(hard_iface, &batadv_hardif_list, list) {
+		if (hard_iface->soft_iface == soft_iface)
+			batadv_hardif_disable_interface(hard_iface,
+							BATADV_IF_CLEANUP_KEEP);
+	}
+
 	batadv_sysfs_del_meshif(soft_iface);
 	unregister_netdevice(soft_iface);
 }
-- 
1.7.10.4

[PATCH 5/7] batman-adv: Allow to postpone softif unregistration

[Sven Eckelmann]

The caller of batadv_softif_destroy may want to postpone the unregistration of
the soft-interface. This is necessary when the rtnl_link configuration hooks
are used.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/hard-interface.c |    2 +-
 net/batman-adv/soft-interface.c |    5 +++--
 net/batman-adv/soft-interface.h |    3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index daeb632..5e2655d 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -447,7 +447,7 @@ void batadv_hardif_disable_interface(struct batadv_hard_iface *hard_iface,
 
 	/* nobody uses this interface anymore */
 	if (!bat_priv->num_ifaces && autodel == BATADV_IF_CLEANUP_AUTO)
-		batadv_softif_destroy(hard_iface->soft_iface);
+		batadv_softif_destroy(hard_iface->soft_iface, NULL);
 
 	hard_iface->soft_iface = NULL;
 	batadv_hardif_free_ref(hard_iface);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 5fd0560..68832f5 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -544,7 +544,8 @@ struct net_device *batadv_softif_create(const char *name)
 	return soft_iface;
 }
 
-void batadv_softif_destroy(struct net_device *soft_iface)
+void batadv_softif_destroy(struct net_device *soft_iface,
+			   struct list_head *head)
 {
 	struct batadv_hard_iface *hard_iface;
 
@@ -555,7 +556,7 @@ void batadv_softif_destroy(struct net_device *soft_iface)
 	}
 
 	batadv_sysfs_del_meshif(soft_iface);
-	unregister_netdevice(soft_iface);
+	unregister_netdevice_queue(soft_iface, head);
 }
 
 int batadv_softif_is_valid(const struct net_device *net_dev)
diff --git a/net/batman-adv/soft-interface.h b/net/batman-adv/soft-interface.h
index 07a08fe..fe9b0f7 100644
--- a/net/batman-adv/soft-interface.h
+++ b/net/batman-adv/soft-interface.h
@@ -25,7 +25,8 @@ void batadv_interface_rx(struct net_device *soft_iface,
 			 struct sk_buff *skb, struct batadv_hard_iface *recv_if,
 			 int hdr_size, struct batadv_orig_node *orig_node);
 struct net_device *batadv_softif_create(const char *name);
-void batadv_softif_destroy(struct net_device *soft_iface);
+void batadv_softif_destroy(struct net_device *soft_iface,
+			   struct list_head *head);
 int batadv_softif_is_valid(const struct net_device *net_dev);
 
 #endif /* _NET_BATMAN_ADV_SOFT_INTERFACE_H_ */
-- 
1.7.10.4

[PATCH 6/7] batman-adv: Allow to use rntl_link for device creation/deletion

[Sven Eckelmann]

The sysfs configuration interface of batman-adv to add/remove soft-interfaces
is not deadlock free and doesn't follow the currently common way to create new
virtual interfaces.

An additional interface though rtnl_link is introduced which provides easy device
creation/deletion with tools like "ip":

$ ip link add dev bat0 type batadv
$ ip link del dev bat0

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/main.c           |    2 ++
 net/batman-adv/main.h           |    1 +
 net/batman-adv/soft-interface.c |    9 +++++++++
 net/batman-adv/soft-interface.h |    1 +
 4 files changed, 13 insertions(+)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index f65a222..9b180a1 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -70,6 +70,7 @@ static int __init batadv_init(void)
 	batadv_debugfs_init();
 
 	register_netdevice_notifier(&batadv_hard_if_notifier);
+	rtnl_link_register(&batadv_link_ops);
 
 	pr_info("B.A.T.M.A.N. advanced %s (compatibility version %i) loaded\n",
 		BATADV_SOURCE_VERSION, BATADV_COMPAT_VERSION);
@@ -80,6 +81,7 @@ static int __init batadv_init(void)
 static void __exit batadv_exit(void)
 {
 	batadv_debugfs_destroy();
+	rtnl_link_unregister(&batadv_link_ops);
 	unregister_netdevice_notifier(&batadv_hard_if_notifier);
 	batadv_hardif_remove_interfaces();
 
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 2f85577..605b125 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -148,6 +148,7 @@ enum batadv_uev_type {
 #include <linux/percpu.h>
 #include <linux/slab.h>
 #include <net/sock.h>		/* struct sock */
+#include <net/rtnetlink.h>
 #include <linux/jiffies.h>
 #include <linux/seq_file.h>
 #include "types.h"
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 68832f5..fcb510e 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -533,6 +533,8 @@ struct net_device *batadv_softif_create(const char *name)
 	if (!soft_iface)
 		return NULL;
 
+	soft_iface->rtnl_link_ops = &batadv_link_ops;
+
 	ret = register_netdevice(soft_iface);
 	if (ret < 0) {
 		pr_err("Unable to register the batman interface '%s': %i\n",
@@ -567,6 +569,13 @@ int batadv_softif_is_valid(const struct net_device *net_dev)
 	return 0;
 }
 
+struct rtnl_link_ops batadv_link_ops __read_mostly = {
+	.kind		= "batadv",
+	.priv_size	= sizeof(struct batadv_priv),
+	.setup		= batadv_interface_setup,
+	.dellink	= batadv_softif_destroy,
+};
+
 /* ethtool */
 static int batadv_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
diff --git a/net/batman-adv/soft-interface.h b/net/batman-adv/soft-interface.h
index fe9b0f7..5eecc6e 100644
--- a/net/batman-adv/soft-interface.h
+++ b/net/batman-adv/soft-interface.h
@@ -28,5 +28,6 @@ struct net_device *batadv_softif_create(const char *name);
 void batadv_softif_destroy(struct net_device *soft_iface,
 			   struct list_head *head);
 int batadv_softif_is_valid(const struct net_device *net_dev);
+extern struct rtnl_link_ops batadv_link_ops;
 
 #endif /* _NET_BATMAN_ADV_SOFT_INTERFACE_H_ */
-- 
1.7.10.4

Re: [PATCH 6/7] batman-adv: Allow to use rntl_link for device creation/deletion

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

Please don't remove netdev from this discussion without a reason.

On Saturday 01 December 2012 14:11:08 Antonio Quartulli wrote:
> On Sat, Nov 24, 2012 at 01:02:11AM +0100, Sven Eckelmann wrote:
> > The sysfs configuration interface of batman-adv to add/remove
> > soft-interfaces is not deadlock free and doesn't follow the currently
> > common way to create new virtual interfaces.
> > 
> > An additional interface though rtnl_link is introduced which provides easy
> > device creation/deletion with tools like "ip":
> > 
> > $ ip link add dev bat0 type batadv
> > $ ip link del dev bat0
> > 
> > Signed-off-by: Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org>
> 
> Hello Sven,
> 
> why are we adding yet another API? What's the purpose? Is this intended to
> fix the lock bug we get while using sysfs?

Because this is the normal way to create virtual network devices (please feel 
free to correct me). And no, it doesn't fix the sysfs problem because the 
sysfs stuff isn't removed.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Re: [B.A.T.M.A.N.] [PATCH 6/7] batman-adv: Allow to use rntl_link for device creation/deletion

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]

On Sat, Dec 01, 2012 at 02:16:38PM +0100, Sven Eckelmann wrote:
> Please don't remove netdev from this discussion without a reason.
> 
> On Saturday 01 December 2012 14:11:08 Antonio Quartulli wrote:
> > On Sat, Nov 24, 2012 at 01:02:11AM +0100, Sven Eckelmann wrote:
> > > The sysfs configuration interface of batman-adv to add/remove
> > > soft-interfaces is not deadlock free and doesn't follow the currently
> > > common way to create new virtual interfaces.
> > > 
> > > An additional interface though rtnl_link is introduced which provides easy
> > > device creation/deletion with tools like "ip":
> > > 
> > > $ ip link add dev bat0 type batadv
> > > $ ip link del dev bat0
> > > 
> > > Signed-off-by: Sven Eckelmann <sven@narfation.org>
> > 
> > Hello Sven,
> > 
> > why are we adding yet another API? What's the purpose? Is this intended to
> > fix the lock bug we get while using sysfs?
> 
> Because this is the normal way to create virtual network devices (please feel 
> free to correct me).

Well, I've seen different iface types using many tools, e.g. vconfig, tunctl,
brctl..
Not that this justifies the fact that we should do the same (imho having a
standard and unified way for creating interfaces would be the best option).

But, to be honest, I think it should better discuss how to entirely
moving/changing the existent API to a "better one" or to a "new one", instead of
starting to maintain two of them from now on with no plan, don't you think so?

> And no, it doesn't fix the sysfs problem because the 
> sysfs stuff isn't removed.


Yeah, I had the same in mind, that's why I was asking for the purpose of this.


Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 6/7] batman-adv: Allow to use rntl_link for device creation/deletion

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

On Saturday 01 December 2012 14:28:02 Antonio Quartulli wrote:
> > Because this is the normal way to create virtual network devices (please
> > feel free to correct me).
> 
> Well, I've seen different iface types using many tools, e.g. vconfig,
> tunctl, brctl..
> Not that this justifies the fact that we should do the same (imho having a
> standard and unified way for creating interfaces would be the best option).

The device creation and enslaving using vconfig, tunctl and brctl can be 
replaced using ip.

> But, to be honest, I think it should better discuss how to entirely
> moving/changing the existent API to a "better one" or to a "new one",
> instead of starting to maintain two of them from now on with no plan, don't
> you think so?

I leave this discussion to the maintainers of batman-adv. Btw. removing the 
old one without a time of coexistence sounds like a bad move. And therefore 
maintaining of both interfaces like it is done in other network devices seems 
to be necessary.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Re: Re: [B.A.T.M.A.N.] [PATCH 6/7] batman-adv: Allow to use rntl_link for device creation/deletion

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

On Sat, Dec 01, 2012 at 02:39:26PM +0100, Sven Eckelmann wrote:
> On Saturday 01 December 2012 14:28:02 Antonio Quartulli wrote:
> > > Because this is the normal way to create virtual network devices (please
> > > feel free to correct me).
> > 
> > Well, I've seen different iface types using many tools, e.g. vconfig,
> > tunctl, brctl..
> > Not that this justifies the fact that we should do the same (imho having a
> > standard and unified way for creating interfaces would be the best option).
> 
> The device creation and enslaving using vconfig, tunctl and brctl can be 
> replaced using ip.

True.

> 
> > But, to be honest, I think it should better discuss how to entirely
> > moving/changing the existent API to a "better one" or to a "new one",
> > instead of starting to maintain two of them from now on with no plan, don't
> > you think so?
> 
> I leave this discussion to the maintainers of batman-adv. 

I started this discussion here because you are still part of them (even if not
listed in MAINTAINERS).

> Btw. removing the 
> old one without a time of coexistence sounds like a bad move. And therefore 
> maintaining of both interfaces like it is done in other network devices seems 
> to be necessary.
> 

Exactly, this is what I wanted to discuss as "a plan".
Anyway, I discussed about this together with the others and it seems
that a proper solution now is to wait before merging this patchset and fix the
current sysfs/rtnl_lock problem first. What do you think?

Adding a new API without fixing the current one doesn't sound like a good move.


Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 7/7] batman-adv: Allow to modify slaves of soft-interfaces through rntl_link

[Sven Eckelmann]

The sysfs configuration interface of batman-adv to add/remove slaves of an
soft-iface is not deadlock free and doesn't follow the currently common way to
modify slaves of an interface.

An additional configuration interface though rtnl_link is introduced which
provides easy device adding/removing with tools like "ip":
$ ip link set dev eth0 master bat0
$ ip link set dev eth0 nomaster

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/soft-interface.c |   39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index fcb510e..8d4ee5c 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -476,6 +476,41 @@ free_bat_counters:
 	return ret;
 }
 
+/**
+ * batadv_add_slave - Add a slave interface to a batadv_soft_interface
+ * @dev: batadv_soft_interface used as master interface
+ * @slave_dev: net_device which should become the slave interface
+ */
+static int batadv_add_slave(struct net_device *dev,
+			    struct net_device *slave_dev)
+{
+	struct batadv_hard_iface *hard_iface;
+
+	hard_iface = batadv_hardif_get_by_netdev(slave_dev);
+	if (!hard_iface || hard_iface->soft_iface != NULL)
+		return -EINVAL;
+
+	return batadv_hardif_enable_interface(hard_iface, dev->name);
+}
+
+/**
+ * batadv_del_slave - Delete a slave interface from a batadv_soft_interface
+ * @dev: batadv_soft_interface used as master interface
+ * @slave_dev: net_device which should be removed from the master interface
+ */
+static int batadv_del_slave(struct net_device *dev,
+			    struct net_device *slave_dev)
+{
+	struct batadv_hard_iface *hard_iface;
+
+	hard_iface = batadv_hardif_get_by_netdev(slave_dev);
+	if (!hard_iface || hard_iface->soft_iface != dev)
+		return -EINVAL;
+
+	batadv_hardif_disable_interface(hard_iface, BATADV_IF_CLEANUP_KEEP);
+	return 0;
+}
+
 static const struct net_device_ops batadv_netdev_ops = {
 	.ndo_init = batadv_softif_init,
 	.ndo_open = batadv_interface_open,
@@ -484,7 +519,9 @@ static const struct net_device_ops batadv_netdev_ops = {
 	.ndo_set_mac_address = batadv_interface_set_mac_addr,
 	.ndo_change_mtu = batadv_interface_change_mtu,
 	.ndo_start_xmit = batadv_interface_tx,
-	.ndo_validate_addr = eth_validate_addr
+	.ndo_validate_addr = eth_validate_addr,
+	.ndo_add_slave = batadv_add_slave,
+	.ndo_del_slave = batadv_del_slave,
 };
 
 /**
-- 
1.7.10.4

Re: [PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[David Miller]

No pull request?

Re: [PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 145 bytes --]

On Saturday 24 November 2012 15:59:02 David Miller wrote:
> No pull request?

They didn't apply it yet. (sry for Cc'ing you)

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[Pau Koning]

On Sat, Nov 24, 2012 at 1:02 AM, Sven Eckelmann <sven@narfation.org> wrote:
> The initialization of an net_device object should be done in the
> init/constructor function and not from the outside after the register_netdevice
> was done to avoid race conditions.

This patchset seems to be similar. Shouldn't it have been part of
3.8-rc1 or got rejected? Neither of it happened in the last month

Re: [PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[Marek Lindner]

On Monday, December 24, 2012 22:22:44 Pau Koning wrote:
> On Sat, Nov 24, 2012 at 1:02 AM, Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org> wrote:
> > The initialization of an net_device object should be done in the
> > init/constructor function and not from the outside after the
> > register_netdevice was done to avoid race conditions.
> 
> This patchset seems to be similar. Shouldn't it have been part of
> 3.8-rc1 or got rejected? Neither of it happened in the last month

It was discussed and a decision was made. Please read all mails in this thread 
before making noise.

Thanks,
Marek

Re: [PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[Pau Koning]

On Mon, Dec 24, 2012 at 3:37 PM, Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org> wrote:
> On Monday, December 24, 2012 22:22:44 Pau Koning wrote:
>> On Sat, Nov 24, 2012 at 1:02 AM, Sven Eckelmann <sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org> wrote:
>> > The initialization of an net_device object should be done in the
>> > init/constructor function and not from the outside after the
>> > register_netdevice was done to avoid race conditions.
>>
>> This patchset seems to be similar. Shouldn't it have been part of
>> 3.8-rc1 or got rejected? Neither of it happened in the last month
>
> It was discussed and a decision was made. Please read all mails in this thread
> before making noise.

What decision? I only see something about Patch 6 and Patch 7: Antonio
Quartulli doesn't want rtnetlink support in batman-adv. I don't see
any other statement about the rest of the patchset.

Re: [B.A.T.M.A.N.] [PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[Marek Lindner]

On Monday, December 24, 2012 23:36:03 Pau Koning wrote:
> On Mon, Dec 24, 2012 at 3:37 PM, Marek Lindner <lindner_marek@yahoo.de> 
wrote:
> > On Monday, December 24, 2012 22:22:44 Pau Koning wrote:
> >> On Sat, Nov 24, 2012 at 1:02 AM, Sven Eckelmann <sven@narfation.org> 
wrote:
> >> > The initialization of an net_device object should be done in the
> >> > init/constructor function and not from the outside after the
> >> > register_netdevice was done to avoid race conditions.
> >> 
> >> This patchset seems to be similar. Shouldn't it have been part of
> >> 3.8-rc1 or got rejected? Neither of it happened in the last month
> > 
> > It was discussed and a decision was made. Please read all mails in this
> > thread before making noise.
> 
> What decision? I only see something about Patch 6 and Patch 7: Antonio
> Quartulli doesn't want rtnetlink support in batman-adv. I don't see
> any other statement about the rest of the patchset.

Quoting Antonio:

>>>
Anyway, I discussed about this together with the others and it seems
that a proper solution now is to wait before merging this patchset and fix the
current sysfs/rtnl_lock problem first. What do you think?

Adding a new API without fixing the current one doesn't sound like a good 
move.
<<<

What isn't clear ?

Cheers,
Marek

Re: [B.A.T.M.A.N.] [PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[Pau Koning]

>> What decision? I only see something about Patch 6 and Patch 7: Antonio
>> Quartulli doesn't want rtnetlink support in batman-adv. I don't see
>> any other statement about the rest of the patchset.
>
> Quoting Antonio:
>
>>>>
> Anyway, I discussed about this together with the others and it seems
> that a proper solution now is to wait before merging this patchset and fix the
> current sysfs/rtnl_lock problem first. What do you think?
>
> Adding a new API without fixing the current one doesn't sound like a good
> move.
> <<<
>
> What isn't clear ?

AFAIK patch 6 and 7 are the "new" rtnetlink API. The rest can be
categorized as fixes for bugs and for the wrong way to implementation
things. But I've discussed it with the others and now we found out:
batman-adv doesn't care about the rest of the kernel and bugs are not
relevant compared to egocentric statements.

Re: [PATCH 1/7] batman-adv: Move soft-interface initialization to ndo_init

[Marek Lindner]

On Tuesday, December 25, 2012 07:18:37 Pau Koning wrote:
> >> What decision? I only see something about Patch 6 and Patch 7: Antonio
> >> Quartulli doesn't want rtnetlink support in batman-adv. I don't see
> >> any other statement about the rest of the patchset.
> > 
> > Quoting Antonio:
> > 
> > 
> > Anyway, I discussed about this together with the others and it seems
> > that a proper solution now is to wait before merging this patchset and
> > fix the current sysfs/rtnl_lock problem first. What do you think?
> > 
> > Adding a new API without fixing the current one doesn't sound like a good
> > move.
> > <<<
> > 
> > What isn't clear ?
> 
> AFAIK patch 6 and 7 are the "new" rtnetlink API. The rest can be
> categorized as fixes for bugs and for the wrong way to implementation
> things.

After looking at the patches again, I'd say only patch1 & patch2 could be 
categorized as "fixes" while the remaining patches are preparation patches for 
the netlink stuff which is added in the last step. Whether patch1 & patch2 
actually fix problems batman-adv already has today or whether they are 
necessary once you use netlink is unclear to me. If they are unrelated to this 
patchset they'd be sent separately, wouldn't they ?

Cheers,
Marek