| Message ID | 0b55d834953625e28eb8c77b96c08cc0f13448dd.1380184164.git.mschiffer@universe-factory.net |
|---|---|
| State | Accepted, archived |
| Headers | show |
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
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(-)