[v2,maint] batman-adv: set network coding packet handlers in batadv_recv_handler_init()
Commit Message
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(-)
Comments
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
@@ -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
@@ -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);
@@ -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)
{