[B.A.T.M.A.N.] [PATCH maint] batman-adv: set network coding packet handlers in batadv_recv_handler_init()
Registering and unregistering the packet handlers on softif creation and destruction is obviously broken when multiple softif are used (and causes the second softif creation to fail). Instead, just set the network coding handlers in the __init function (like all other handlers).
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- main.c | 2 ++ network-coding.c | 12 ++---------- network-coding.h | 8 ++++++++ 3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/main.c b/main.c index 08125f3..87ef89a 100644 --- a/main.c +++ b/main.c @@ -349,6 +349,8 @@ static void batadv_recv_handler_init(void) batadv_rx_handler[BATADV_TT_QUERY] = batadv_recv_tt_query; /* Roaming advertisement */ batadv_rx_handler[BATADV_ROAM_ADV] = batadv_recv_roam_adv; + /* network coding */ + batadv_rx_handler[BATADV_CODED] = batadv_nc_recv_coded_packet; }
int diff --git a/network-coding.c b/network-coding.c index a487d46..6f392fd 100644 --- a/network-coding.c +++ b/network-coding.c @@ -31,8 +31,6 @@ static struct lock_class_key batadv_nc_coding_hash_lock_class_key; static struct lock_class_key batadv_nc_decoding_hash_lock_class_key;
static void batadv_nc_worker(struct work_struct *work); -static int batadv_nc_recv_coded_packet(struct sk_buff *skb, - struct batadv_hard_iface *recv_if);
/** * batadv_nc_start_timer - initialise the nc periodic worker @@ -70,11 +68,6 @@ int batadv_nc_init(struct batadv_priv *bat_priv) batadv_hash_set_lock_class(bat_priv->nc.coding_hash, &batadv_nc_decoding_hash_lock_class_key);
- /* Register our packet type */ - if (batadv_recv_handler_register(BATADV_CODED, - batadv_nc_recv_coded_packet) < 0) - goto err; - INIT_DELAYED_WORK(&bat_priv->nc.work, batadv_nc_worker); batadv_nc_start_timer(bat_priv);
@@ -1657,8 +1650,8 @@ batadv_nc_find_decoding_packet(struct batadv_priv *bat_priv, * @skb: incoming coded packet * @recv_if: pointer to interface this packet was received on */ -static int batadv_nc_recv_coded_packet(struct sk_buff *skb, - struct batadv_hard_iface *recv_if) +int batadv_nc_recv_coded_packet(struct sk_buff *skb, + struct batadv_hard_iface *recv_if) { struct batadv_priv *bat_priv = netdev_priv(recv_if->soft_iface); struct batadv_unicast_packet *unicast_packet; @@ -1726,7 +1719,6 @@ free_nc_packet: */ void batadv_nc_free(struct batadv_priv *bat_priv) { - batadv_recv_handler_unregister(BATADV_CODED); cancel_delayed_work_sync(&bat_priv->nc.work);
batadv_nc_purge_paths(bat_priv, bat_priv->nc.coding_hash, NULL); diff --git a/network-coding.h b/network-coding.h index 85a4ec8..f00cd4d 100644 --- a/network-coding.h +++ b/network-coding.h @@ -35,6 +35,8 @@ void batadv_nc_purge_orig(struct batadv_priv *bat_priv, struct batadv_nc_node *)); void batadv_nc_init_bat_priv(struct batadv_priv *bat_priv); void batadv_nc_init_orig(struct batadv_orig_node *orig_node); +int batadv_nc_recv_coded_packet(struct sk_buff *skb, + struct batadv_hard_iface *recv_if); bool batadv_nc_skb_forward(struct sk_buff *skb, struct batadv_neigh_node *neigh_node); void batadv_nc_skb_store_for_decoding(struct batadv_priv *bat_priv, @@ -85,6 +87,12 @@ static inline void batadv_nc_init_orig(struct batadv_orig_node *orig_node) return; }
+static inline int batadv_nc_recv_coded_packet(struct sk_buff *skb, + struct batadv_hard_iface *recv_if) +{ + return NET_RX_DROP; +} + static inline bool batadv_nc_skb_forward(struct sk_buff *skb, struct batadv_neigh_node *neigh_node) {
batman-adv saves its table of packet handlers as a global state, so handlers must be set up only once (and setting them up a second time will fail).
The recently-added network coding support tries to set up its handler each time a new softif is registered, which obviously fails when more that one softif is used (and in consequence, the softif creation fails).
Fix this by moving the handler setup to batadv_recv_handler_init(), which is called by batman-adv's __init function (and where most other packet handlers are set up).
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- v2: refined commit message
main.c | 2 ++ network-coding.c | 12 ++---------- network-coding.h | 8 ++++++++ 3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/main.c b/main.c index 08125f3..87ef89a 100644 --- a/main.c +++ b/main.c @@ -349,6 +349,8 @@ static void batadv_recv_handler_init(void) batadv_rx_handler[BATADV_TT_QUERY] = batadv_recv_tt_query; /* Roaming advertisement */ batadv_rx_handler[BATADV_ROAM_ADV] = batadv_recv_roam_adv; + /* network coding */ + batadv_rx_handler[BATADV_CODED] = batadv_nc_recv_coded_packet; }
int diff --git a/network-coding.c b/network-coding.c index a487d46..6f392fd 100644 --- a/network-coding.c +++ b/network-coding.c @@ -31,8 +31,6 @@ static struct lock_class_key batadv_nc_coding_hash_lock_class_key; static struct lock_class_key batadv_nc_decoding_hash_lock_class_key;
static void batadv_nc_worker(struct work_struct *work); -static int batadv_nc_recv_coded_packet(struct sk_buff *skb, - struct batadv_hard_iface *recv_if);
/** * batadv_nc_start_timer - initialise the nc periodic worker @@ -70,11 +68,6 @@ int batadv_nc_init(struct batadv_priv *bat_priv) batadv_hash_set_lock_class(bat_priv->nc.coding_hash, &batadv_nc_decoding_hash_lock_class_key);
- /* Register our packet type */ - if (batadv_recv_handler_register(BATADV_CODED, - batadv_nc_recv_coded_packet) < 0) - goto err; - INIT_DELAYED_WORK(&bat_priv->nc.work, batadv_nc_worker); batadv_nc_start_timer(bat_priv);
@@ -1657,8 +1650,8 @@ batadv_nc_find_decoding_packet(struct batadv_priv *bat_priv, * @skb: incoming coded packet * @recv_if: pointer to interface this packet was received on */ -static int batadv_nc_recv_coded_packet(struct sk_buff *skb, - struct batadv_hard_iface *recv_if) +int batadv_nc_recv_coded_packet(struct sk_buff *skb, + struct batadv_hard_iface *recv_if) { struct batadv_priv *bat_priv = netdev_priv(recv_if->soft_iface); struct batadv_unicast_packet *unicast_packet; @@ -1726,7 +1719,6 @@ free_nc_packet: */ void batadv_nc_free(struct batadv_priv *bat_priv) { - batadv_recv_handler_unregister(BATADV_CODED); cancel_delayed_work_sync(&bat_priv->nc.work);
batadv_nc_purge_paths(bat_priv, bat_priv->nc.coding_hash, NULL); diff --git a/network-coding.h b/network-coding.h index 85a4ec8..f00cd4d 100644 --- a/network-coding.h +++ b/network-coding.h @@ -35,6 +35,8 @@ void batadv_nc_purge_orig(struct batadv_priv *bat_priv, struct batadv_nc_node *)); void batadv_nc_init_bat_priv(struct batadv_priv *bat_priv); void batadv_nc_init_orig(struct batadv_orig_node *orig_node); +int batadv_nc_recv_coded_packet(struct sk_buff *skb, + struct batadv_hard_iface *recv_if); bool batadv_nc_skb_forward(struct sk_buff *skb, struct batadv_neigh_node *neigh_node); void batadv_nc_skb_store_for_decoding(struct batadv_priv *bat_priv, @@ -85,6 +87,12 @@ static inline void batadv_nc_init_orig(struct batadv_orig_node *orig_node) return; }
+static inline int batadv_nc_recv_coded_packet(struct sk_buff *skb, + struct batadv_hard_iface *recv_if) +{ + return NET_RX_DROP; +} + static inline bool batadv_nc_skb_forward(struct sk_buff *skb, struct batadv_neigh_node *neigh_node) {
On Thursday 26 September 2013 10:29:49 Matthias Schiffer wrote:
batman-adv saves its table of packet handlers as a global state, so handlers must be set up only once (and setting them up a second time will fail).
The recently-added network coding support tries to set up its handler each time a new softif is registered, which obviously fails when more that one softif is used (and in consequence, the softif creation fails).
Fix this by moving the handler setup to batadv_recv_handler_init(), which is called by batman-adv's __init function (and where most other packet handlers are set up).
Would you mind changing your approach to adding a global nc_init() / nc_free() and do the callback registration there ?
Thanks, Marek
On 09/27/2013 08:07 AM, Marek Lindner wrote:
On Thursday 26 September 2013 10:29:49 Matthias Schiffer wrote:
batman-adv saves its table of packet handlers as a global state, so handlers must be set up only once (and setting them up a second time will fail).
The recently-added network coding support tries to set up its handler each time a new softif is registered, which obviously fails when more that one softif is used (and in consequence, the softif creation fails).
Fix this by moving the handler setup to batadv_recv_handler_init(), which is called by batman-adv's __init function (and where most other packet handlers are set up).
Would you mind changing your approach to adding a global nc_init() / nc_free() and do the callback registration there ?
Thanks, Marek
No problem. Any idea for a good name for the new function? Currently, the init functions are named a bit inconsistently, there is batadv_iv_init, which is called in batadv_init, and many other batadv_*_init called in batadv_mesh_init. I'd prefer renaming all those to batadv_*_mesh_init, but that seems like lots of avoidable changes for a net/stable patch.
Maybe the maint patch could stay like it is, and I'll add another patch on top for master that cleans things up?
On Friday 27 September 2013 10:13:51 Matthias Schiffer wrote:
No problem. Any idea for a good name for the new function? Currently, the init functions are named a bit inconsistently, there is batadv_iv_init, which is called in batadv_init, and many other batadv_*_init called in batadv_mesh_init. I'd prefer renaming all those to batadv_*_mesh_init, but that seems like lots of avoidable changes for a net/stable patch.
Maybe the maint patch could stay like it is, and I'll add another patch on top for master that cleans things up?
Yes, you could come up with a naming convention you introduce with the maint patch for nc_init() / nc_free(). Other renamings could go into next or master.
Cheers, Marek
batman-adv saves its table of packet handlers as a global state, so handlers must be set up only once (and setting them up a second time will fail).
The recently-added network coding support tries to set up its handler each time a new softif is registered, which obviously fails when more that one softif is used (and in consequence, the softif creation fails).
Fix this by splitting up batadv_nc_init into batadv_nc_init (which is called only once) and batadv_nc_mesh_init (which is called for each softif); in addition batadv_nc_free is renamed to batadv_nc_mesh_free to keep naming consistent.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net --- v3: moved things around as Marek suggested
main.c | 5 +++-- network-coding.c | 28 ++++++++++++++++++---------- network-coding.h | 14 ++++++++++---- 3 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/main.c b/main.c index 08125f3..c8e0671 100644 --- a/main.c +++ b/main.c @@ -61,6 +61,7 @@ static int __init batadv_init(void) batadv_recv_handler_init();
batadv_iv_init(); + batadv_nc_init();
batadv_event_workqueue = create_singlethread_workqueue("bat_events");
@@ -138,7 +139,7 @@ int batadv_mesh_init(struct net_device *soft_iface) if (ret < 0) goto err;
- ret = batadv_nc_init(bat_priv); + ret = batadv_nc_mesh_init(bat_priv); if (ret < 0) goto err;
@@ -163,7 +164,7 @@ void batadv_mesh_free(struct net_device *soft_iface) batadv_vis_quit(bat_priv);
batadv_gw_node_purge(bat_priv); - batadv_nc_free(bat_priv); + batadv_nc_mesh_free(bat_priv); batadv_dat_free(bat_priv); batadv_bla_free(bat_priv);
diff --git a/network-coding.c b/network-coding.c index a487d46..4ecc0b6 100644 --- a/network-coding.c +++ b/network-coding.c @@ -35,6 +35,20 @@ static int batadv_nc_recv_coded_packet(struct sk_buff *skb, struct batadv_hard_iface *recv_if);
/** + * batadv_nc_init - one-time initialization for network coding + */ +int __init batadv_nc_init(void) +{ + int ret; + + /* Register our packet type */ + ret = batadv_recv_handler_register(BATADV_CODED, + batadv_nc_recv_coded_packet); + + return ret; +} + +/** * batadv_nc_start_timer - initialise the nc periodic worker * @bat_priv: the bat priv with all the soft interface information */ @@ -45,10 +59,10 @@ static void batadv_nc_start_timer(struct batadv_priv *bat_priv) }
/** - * batadv_nc_init - initialise coding hash table and start house keeping + * batadv_nc_mesh_init - initialise coding hash table and start house keeping * @bat_priv: the bat priv with all the soft interface information */ -int batadv_nc_init(struct batadv_priv *bat_priv) +int batadv_nc_mesh_init(struct batadv_priv *bat_priv) { bat_priv->nc.timestamp_fwd_flush = jiffies; bat_priv->nc.timestamp_sniffed_purge = jiffies; @@ -70,11 +84,6 @@ int batadv_nc_init(struct batadv_priv *bat_priv) batadv_hash_set_lock_class(bat_priv->nc.coding_hash, &batadv_nc_decoding_hash_lock_class_key);
- /* Register our packet type */ - if (batadv_recv_handler_register(BATADV_CODED, - batadv_nc_recv_coded_packet) < 0) - goto err; - INIT_DELAYED_WORK(&bat_priv->nc.work, batadv_nc_worker); batadv_nc_start_timer(bat_priv);
@@ -1721,12 +1730,11 @@ free_nc_packet: }
/** - * batadv_nc_free - clean up network coding memory + * batadv_nc_mesh_free - clean up network coding memory * @bat_priv: the bat priv with all the soft interface information */ -void batadv_nc_free(struct batadv_priv *bat_priv) +void batadv_nc_mesh_free(struct batadv_priv *bat_priv) { - batadv_recv_handler_unregister(BATADV_CODED); cancel_delayed_work_sync(&bat_priv->nc.work);
batadv_nc_purge_paths(bat_priv, bat_priv->nc.coding_hash, NULL); diff --git a/network-coding.h b/network-coding.h index 85a4ec8..ddfa618 100644 --- a/network-coding.h +++ b/network-coding.h @@ -22,8 +22,9 @@
#ifdef CONFIG_BATMAN_ADV_NC
-int batadv_nc_init(struct batadv_priv *bat_priv); -void batadv_nc_free(struct batadv_priv *bat_priv); +int batadv_nc_init(void); +int batadv_nc_mesh_init(struct batadv_priv *bat_priv); +void batadv_nc_mesh_free(struct batadv_priv *bat_priv); void batadv_nc_update_nc_node(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, struct batadv_orig_node *orig_neigh_node, @@ -46,12 +47,17 @@ int batadv_nc_init_debugfs(struct batadv_priv *bat_priv);
#else /* ifdef CONFIG_BATMAN_ADV_NC */
-static inline int batadv_nc_init(struct batadv_priv *bat_priv) +static inline int batadv_nc_init(void) { return 0; }
-static inline void batadv_nc_free(struct batadv_priv *bat_priv) +static inline int batadv_nc_mesh_init(struct batadv_priv *bat_priv) +{ + return 0; +} + +static inline void batadv_nc_mesh_free(struct batadv_priv *bat_priv) { return; }
On Friday 27 September 2013 18:03:39 Matthias Schiffer wrote:
batman-adv saves its table of packet handlers as a global state, so handlers must be set up only once (and setting them up a second time will fail).
The recently-added network coding support tries to set up its handler each time a new softif is registered, which obviously fails when more that one softif is used (and in consequence, the softif creation fails).
Fix this by splitting up batadv_nc_init into batadv_nc_init (which is called only once) and batadv_nc_mesh_init (which is called for each softif); in addition batadv_nc_free is renamed to batadv_nc_mesh_free to keep naming consistent.
Signed-off-by: Matthias Schiffer mschiffer@universe-factory.net
v3: moved things around as Marek suggested
main.c | 5 +++-- network-coding.c | 28 ++++++++++++++++++---------- network-coding.h | 14 ++++++++++---- 3 files changed, 31 insertions(+), 16 deletions(-)
Applied in revision 586cf84.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Marek Lindner -
Matthias Schiffer