[B.A.T.M.A.N.] [PATCH v2] batman-adv: B.A.T.M.A.N V - make sure iface is reactivated upon NETDEV_UP event

[B.A.T.M.A.N.] [PATCH v2] batman-adv: B.A.T.M.A.N V - make sure iface is reactivated upon NETDEV_UP event

[Antonio Quartulli]

At the moment there is no explicit reactivation of an hard-interface
upon NETDEV_UP event. In case of B.A.T.M.A.N. IV the interface is
reactivated as soon as the next OGM is scheduled for sending, but this
mechanism does not work with B.A.T.M.A.N. V. The latter does not rely
on the same scheduling mechanism as its predecessor and for this reason
the hard-interface remains deactivated forever after being brought down
once.

This patch fixes the reactivation mechanism by adding a new routing API
which explicitly allows each algorithm to perform any needed operation
upon interface re-activation.

Such API is optional and is implemented by B.A.T.M.A.N. V only and it
just takes care of setting the iface status to ACTIVE

Signed-off-by: Antonio Quartulli <a@unstable.cc>
---

Changes since v1:
- added kerneldoc for bat_iface_activate

 net/batman-adv/bat_v.c          | 12 ++++++++++++
 net/batman-adv/hard-interface.c |  3 +++
 net/batman-adv/types.h          |  3 +++
 3 files changed, 18 insertions(+)

diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index 3315b9a..4026f19 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -32,10 +32,21 @@
 
 #include "bat_v_elp.h"
 #include "bat_v_ogm.h"
+#include "hard-interface.h"
 #include "hash.h"
 #include "originator.h"
 #include "packet.h"
 
+static void batadv_v_iface_activate(struct batadv_hard_iface *hard_iface)
+{
+	/* B.A.T.M.A.N. V does not use any queuing mechanism, therefore it can
+	 * set the interface as ACTIVE right away, without any risk of race
+	 * condition
+	 */
+	if (hard_iface->if_status == BATADV_IF_TO_BE_ACTIVATED)
+		hard_iface->if_status = BATADV_IF_ACTIVE;
+}
+
 static int batadv_v_iface_enable(struct batadv_hard_iface *hard_iface)
 {
 	int ret;
@@ -274,6 +285,7 @@ static bool batadv_v_neigh_is_sob(struct batadv_neigh_node *neigh1,
 
 static struct batadv_algo_ops batadv_batman_v __read_mostly = {
 	.name = "BATMAN_V",
+	.bat_iface_activate = batadv_v_iface_activate,
 	.bat_iface_enable = batadv_v_iface_enable,
 	.bat_iface_disable = batadv_v_iface_disable,
 	.bat_iface_update_mac = batadv_v_iface_update_mac,
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index c61d5b0..0a7deaf 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -407,6 +407,9 @@ batadv_hardif_activate_interface(struct batadv_hard_iface *hard_iface)
 
 	batadv_update_min_mtu(hard_iface->soft_iface);
 
+	if (bat_priv->bat_algo_ops->bat_iface_activate)
+		bat_priv->bat_algo_ops->bat_iface_activate(hard_iface);
+
 out:
 	if (primary_if)
 		batadv_hardif_put(primary_if);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 9abfb3e..443e9b8 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1250,6 +1250,8 @@ struct batadv_forw_packet {
  * struct batadv_algo_ops - mesh algorithm callbacks
  * @list: list node for the batadv_algo_list
  * @name: name of the algorithm
+ * @bat_iface_activate: start routing mechanisms when hard-interface is brought
+ *  up
  * @bat_iface_enable: init routing info when hard-interface is enabled
  * @bat_iface_disable: de-init routing info when hard-interface is disabled
  * @bat_iface_update_mac: (re-)init mac addresses of the protocol information
@@ -1277,6 +1279,7 @@ struct batadv_forw_packet {
 struct batadv_algo_ops {
 	struct hlist_node list;
 	char *name;
+	void (*bat_iface_activate)(struct batadv_hard_iface *hard_iface);
 	int (*bat_iface_enable)(struct batadv_hard_iface *hard_iface);
 	void (*bat_iface_disable)(struct batadv_hard_iface *hard_iface);
 	void (*bat_iface_update_mac)(struct batadv_hard_iface *hard_iface);
-- 
2.8.1

Re: [B.A.T.M.A.N.] [PATCH v2] batman-adv: B.A.T.M.A.N V - make sure iface is reactivated upon NETDEV_UP event

[Sven Eckelmann]

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

On Thursday 14 April 2016 09:37:05 Antonio Quartulli wrote:
> At the moment there is no explicit reactivation of an hard-interface
> upon NETDEV_UP event. In case of B.A.T.M.A.N. IV the interface is
> reactivated as soon as the next OGM is scheduled for sending, but this
> mechanism does not work with B.A.T.M.A.N. V. The latter does not rely
> on the same scheduling mechanism as its predecessor and for this reason
> the hard-interface remains deactivated forever after being brought down
> once.
> 
> This patch fixes the reactivation mechanism by adding a new routing API
> which explicitly allows each algorithm to perform any needed operation
> upon interface re-activation.
> 
> Such API is optional and is implemented by B.A.T.M.A.N. V only and it
> just takes care of setting the iface status to ACTIVE
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
> 
> Changes since v1:
> - added kerneldoc for bat_iface_activate
> 
>  net/batman-adv/bat_v.c          | 12 ++++++++++++
>  net/batman-adv/hard-interface.c |  3 +++
>  net/batman-adv/types.h          |  3 +++
>  3 files changed, 18 insertions(+)

Reviewed-by: Sven Eckelmann <sven@narfation.org>

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

Re: [B.A.T.M.A.N.] [PATCH v2] batman-adv: B.A.T.M.A.N V - make sure iface is reactivated upon NETDEV_UP event

[Antonio Quartulli]

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

Hi all,

it looks like this patch was not interpreted the right way, probably because of
a lack of communication between me and other maintainers or probably because
too much laziness here and there...

Explanation follows below.

On Thu, Apr 14, 2016 at 09:37:05AM +0800, Antonio Quartulli wrote:
> At the moment there is no explicit reactivation of an hard-interface
> upon NETDEV_UP event. In case of B.A.T.M.A.N. IV the interface is
> reactivated as soon as the next OGM is scheduled for sending, but this
> mechanism does not work with B.A.T.M.A.N. V. The latter does not rely
> on the same scheduling mechanism as its predecessor and for this reason
> the hard-interface remains deactivated forever after being brought down
> once.
> 
> This patch fixes the reactivation mechanism by adding a new routing API
> which explicitly allows each algorithm to perform any needed operation
> upon interface re-activation.
> 

Right now we have a piece of infrastructure used by B.A.T.M.A.N IV that
consists in calling batadv_send_outstanding_bat_ogm_packet() periodically,
thus generating the following flow:

-> batadv_send_outstanding_bat_ogm_packet()
	-> bat_algo_ops->bat_ogm_emit()
	-> batadv_schedule_bat_ogm()
		bat_algo_ops->bat_ogm_schedule()

by scrolling over the mentioned functions I'd argue that such flow is rather
tight to the B.A.T.M.A.N. IV algorithm and, although there has been a lot of
effort in making such routines generic (that's why we have API calls in the
middle), such infrastructure is very difficult to be used within B.A.T.M.A.N. V.


On top of that, the logic implemented within this piece of infrastructure
aims to achieve two goals:
1) schedule OGM sending
2) send out aggregated OGMs


The consequence of this was that B.A.T.M.A.N. V has its own implementation for
sending OGMs and lacks support for OGMs aggregation (and queue limitation).


This said, the bug that this patch is trying to fix came up exactly from the
new implementation of point 1) in B.A.T.M.A.N. V.

One could argue that in order to fix this misbehaviour it would be better to
stick to using the already implemented infrastructure, but unfortunately, as
I explained above, this is not possible and therefore this patch can't go
in that direction.


This patch, instead, tries to make the first step towards changing the
infrastructure in a way where B.A.T.M.A.N. IV will adapt to its successor
and not viceversa.


The idea is to have a new API call to be invoked everytime a hard interface is
brought up so that each algorithm can take its own decision about what to do
next.

At this point, the potential next steps that I see are:

1) convert batadv_send_outstanding_bat_ogm_packet() and invoked routines to
B.A.T.M.A.N. IV private code (remove bat_algo_ops->ogm_schedule/ogm_emit())

2) have B.A.T.M.A.N. IV use the new iface_activate() API introduced by this
patch (if needed)

3) make the aggregation code generic so that other protocols can enjoy the same
benefit without having to re-use exactly the same logic

4) implement aggregation in B.A.T.M.A.N. V with queue limitation

I tried to include point 1) in this patch so that it could look more "sane" from
an architectural point of view, but given that this change is a fix (and thus
targets the net tree) I decided to squeeze it and include only what was necessary
to fix the "interface-enabling" issue.


I hope this helps understanding the ratio behind this patch :)

Cheers,


-- 
Antonio Quartulli

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

Re: [B.A.T.M.A.N.] [PATCH v2] batman-adv: B.A.T.M.A.N V - make sure iface is reactivated upon NETDEV_UP event

[Marek Lindner]

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

On Thursday, April 14, 2016 09:37:05 Antonio Quartulli wrote:
> At the moment there is no explicit reactivation of an hard-interface
> upon NETDEV_UP event. In case of B.A.T.M.A.N. IV the interface is
> reactivated as soon as the next OGM is scheduled for sending, but this
> mechanism does not work with B.A.T.M.A.N. V. The latter does not rely
> on the same scheduling mechanism as its predecessor and for this reason
> the hard-interface remains deactivated forever after being brought down
> once.
> 
> This patch fixes the reactivation mechanism by adding a new routing API
> which explicitly allows each algorithm to perform any needed operation
> upon interface re-activation.
> 
> Such API is optional and is implemented by B.A.T.M.A.N. V only and it
> just takes care of setting the iface status to ACTIVE
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
> 
> Changes since v1:
> - added kerneldoc for bat_iface_activate
> 
>  net/batman-adv/bat_v.c          | 12 ++++++++++++
>  net/batman-adv/hard-interface.c |  3 +++
>  net/batman-adv/types.h          |  3 +++
>  3 files changed, 18 insertions(+)

Applied in revision 69cfb8b.

Thanks,
Marek

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