[v3,1/6] batman-adv: Move soft-interface initialization to ndo_init

Message ID 1360573828-24399-1-git-send-email-lindner_marek@yahoo.de (mailing list archive)
State Accepted, archived
Commit 03522f0bb9d04c0834700db95f42f6bbcd5f8dd7
Headers

Commit Message

Marek Lindner Feb. 11, 2013, 9:10 a.m. UTC
  From: Sven Eckelmann <sven@narfation.org>

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@narfation.org>
---
 hard-interface.c |    5 ++
 soft-interface.c |  169 ++++++++++++++++++++++++++----------------------------
 2 files changed, 86 insertions(+), 88 deletions(-)
  

Comments

Marek Lindner Feb. 16, 2013, 9:11 a.m. UTC | #1
On Monday, February 11, 2013 17:10:22 Marek Lindner wrote:
> From: Sven Eckelmann <sven@narfation.org>
> 
> 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@narfation.org>
> ---
>  hard-interface.c |    5 ++
>  soft-interface.c |  169
> ++++++++++++++++++++++++++---------------------------- 2 files changed, 86
> insertions(+), 88 deletions(-)

Applied in revision 03522f0.

Thanks,
Marek
  
Marek Lindner Feb. 16, 2013, 9:15 a.m. UTC | #2
On Monday, February 11, 2013 17:10:24 Marek Lindner wrote:
> From: Sven Eckelmann <sven@narfation.org>
> 
> 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>
> ---
>  hard-interface.c |    8 +++++---
>  hard-interface.h |   13 ++++++++++++-
>  sysfs.c          |    6 ++++--
>  3 files changed, 21 insertions(+), 6 deletions(-)

Applied in revision c3c7e74.

Thanks,
Marek
  

Patch

diff --git a/hard-interface.c b/hard-interface.c
index 368219e..da000e9 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -563,6 +563,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/soft-interface.c b/soft-interface.c
index f93ae42..c5cb0a7 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -402,55 +402,6 @@  static void batadv_set_lockdep_class(struct net_device *dev)
 }
 
 /**
- * batadv_softif_init - Late stage initialization of soft interface
- * @dev: registered network device to modify
- *
- * Returns error code on failures
- */
-static int batadv_softif_init(struct net_device *dev)
-{
-	batadv_set_lockdep_class(dev);
-
-	return 0;
-}
-
-static const struct net_device_ops batadv_netdev_ops = {
-	.ndo_init = batadv_softif_init,
-	.ndo_open = batadv_interface_open,
-	.ndo_stop = batadv_interface_release,
-	.ndo_get_stats = batadv_interface_stats,
-	.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
-};
-
-static void batadv_interface_setup(struct net_device *dev)
-{
-	struct batadv_priv *priv = netdev_priv(dev);
-
-	ether_setup(dev);
-
-	dev->netdev_ops = &batadv_netdev_ops;
-	dev->destructor = free_netdev;
-	dev->tx_queue_len = 0;
-
-	/* can't call min_mtu, because the needed variables
-	 * have not been initialized yet
-	 */
-	dev->mtu = ETH_DATA_LEN;
-	/* reserve more space in the skbuff for our header */
-	dev->hard_header_len = BATADV_HEADER_LEN;
-
-	/* generate random address */
-	eth_hw_addr_random(dev);
-
-	SET_ETHTOOL_OPS(dev, &batadv_ethtool_ops);
-
-	memset(priv, 0, sizeof(*priv));
-}
-
-/**
  * batadv_softif_destroy_finish - cleans up the remains of a softif
  * @work: work queue item
  *
@@ -474,21 +425,22 @@  static void batadv_softif_destroy_finish(struct work_struct *work)
 	rtnl_unlock();
 }
 
-struct net_device *batadv_softif_create(const char *name)
+/**
+ * batadv_softif_init_late - late stage initialization of soft interface
+ * @dev: registered network device to modify
+ *
+ * Returns error code on failures
+ */
+static int batadv_softif_init_late(struct net_device *dev)
 {
-	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,
-				  batadv_interface_setup);
-
-	if (!soft_iface)
-		goto out;
+	batadv_set_lockdep_class(dev);
 
-	bat_priv = netdev_priv(soft_iface);
-	bat_priv->soft_iface = soft_iface;
+	bat_priv = netdev_priv(dev);
+	bat_priv->soft_iface = dev;
 	INIT_WORK(&bat_priv->cleanup_work, batadv_softif_destroy_finish);
 
 	/* batadv_interface_stats() needs to be available as soon as
@@ -496,14 +448,7 @@  struct net_device *batadv_softif_create(const char *name)
 	 */
 	bat_priv->bat_counters = __alloc_percpu(cnt_len, __alignof__(uint64_t));
 	if (!bat_priv->bat_counters)
-		goto free_soft_iface;
-
-	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;
-	}
+		return -ENOMEM;
 
 	atomic_set(&bat_priv->aggregated_ogms, 1);
 	atomic_set(&bat_priv->bonding, 0);
@@ -541,41 +486,89 @@  struct net_device *batadv_softif_create(const char *name)
 	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;
-
 	batadv_nc_init_bat_priv(bat_priv);
 
-	ret = batadv_sysfs_add_meshif(soft_iface);
+	ret = batadv_algo_select(bat_priv, batadv_routing_algo);
 	if (ret < 0)
-		goto unreg_soft_iface;
+		goto free_bat_counters;
 
-	ret = batadv_debugfs_add_meshif(soft_iface);
+	ret = batadv_debugfs_add_meshif(dev);
 	if (ret < 0)
-		goto unreg_sysfs;
+		goto free_bat_counters;
 
-	ret = batadv_mesh_init(soft_iface);
+	ret = batadv_mesh_init(dev);
 	if (ret < 0)
 		goto unreg_debugfs;
 
-	return soft_iface;
+	return 0;
 
 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;
-
+	batadv_debugfs_del_meshif(dev);
 free_bat_counters:
 	free_percpu(bat_priv->bat_counters);
-free_soft_iface:
-	free_netdev(soft_iface);
-out:
-	return NULL;
+
+	return ret;
+}
+
+static const struct net_device_ops batadv_netdev_ops = {
+	.ndo_init = batadv_softif_init_late,
+	.ndo_open = batadv_interface_open,
+	.ndo_stop = batadv_interface_release,
+	.ndo_get_stats = batadv_interface_stats,
+	.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
+};
+
+/**
+ * batadv_softif_init_early - early stage initialization of soft interface
+ * @dev: registered network device to modify
+ */
+static void batadv_softif_init_early(struct net_device *dev)
+{
+	struct batadv_priv *priv = netdev_priv(dev);
+
+	ether_setup(dev);
+
+	dev->netdev_ops = &batadv_netdev_ops;
+	dev->destructor = free_netdev;
+	dev->tx_queue_len = 0;
+
+	/* can't call min_mtu, because the needed variables
+	 * have not been initialized yet
+	 */
+	dev->mtu = ETH_DATA_LEN;
+	/* reserve more space in the skbuff for our header */
+	dev->hard_header_len = BATADV_HEADER_LEN;
+
+	/* generate random address */
+	eth_hw_addr_random(dev);
+
+	SET_ETHTOOL_OPS(dev, &batadv_ethtool_ops);
+
+	memset(priv, 0, sizeof(*priv));
+}
+
+struct net_device *batadv_softif_create(const char *name)
+{
+	struct net_device *soft_iface;
+	int ret;
+
+	soft_iface = alloc_netdev(sizeof(struct batadv_priv), name,
+				  batadv_softif_init_early);
+	if (!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);
+		free_netdev(soft_iface);
+		return NULL;
+	}
+
+	return soft_iface;
 }
 
 void batadv_softif_destroy(struct net_device *soft_iface)