[B.A.T.M.A.N.] [RFC 00/10] Improving the routing protocol abstraction

[B.A.T.M.A.N.] [RFC 00/10] Improving the routing protocol abstraction

[Antonio Quartulli]

Hello list,

with this RFC I'd like to introduce some new routing API functions meant to
improve the routing protocol abstraction.

This changes have been written while developing batman V. The latter helped me
in understanding what batman iv and v have in common and what not.

The main problem was the metric: the two protocols use different metric domains
and different semantics.
Therefore all the functions handling/printing the metric needed to be
generalised and rearranged to let the protocols decide what to do.

Another issue was the way routing protocols handle the orig and neigh node
structures. Also these two have been changed and some small APIs have been
provided as well.


Moreover, after Simon's RFC about the new multi-interface optimisation, we saw
the need for a better abstraction so that mechanisms like that could easily be
re-used by new algorithms (like batman v) with little effort.



The new API functions are the following:
+ metric related:
	- bat_metric_get
	- bat_metric_is_similar
	- bat_metric_compare

+ orig_node related:
	- bat_orig_print: print the originator table
	- bat_orig_add_if
	- bat_orig_del_if
	- bat_orig_free

Any feedback will surely be welcome :-)


Cheers,

p.s. there are some small "checkpatch glitches" but they will be removed before
this RFC becomes a real patchset :)


Antonio Quartulli (10):
  batman-adv: make struct batadv_neigh_node algorithm agnostic
  batman-adv: make struct batadv_orig_node algorithm agnostic
  batman-adv: add bat_orig_print function API
  batman-adv: add bat_metric_get API function
  batman-adv: add bat_metric_is_similar API function
  batman-adv: add bat_metric_compare API function
  batman-adv: adapt bonding to use the new API functions
  batman-adv: adapt the gateway feature to use the new API functions
  batman-adv: adapt the neighbor purging routine to use the new API
    functions
  batman-adv: provide orig_node routing API

 bat_iv_ogm.c        | 373 +++++++++++++++++++++++++++++++++++++++++++---------
 gateway_client.c    |  74 ++++++-----
 main.c              |   5 +-
 main.h              |   7 +
 network-coding.c    |   8 +-
 originator.c        | 238 ++++++++-------------------------
 originator.h        |   5 +-
 routing.c           |  37 ++++--
 routing.h           |   3 +-
 translation-table.c |   4 +-
 types.h             | 106 +++++++++------
 11 files changed, 518 insertions(+), 342 deletions(-)

-- 
1.8.1.5

[B.A.T.M.A.N.] [RFC 01/10] batman-adv: make struct batadv_neigh_node algorithm agnostic

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

some of the fields in struct batadv_neigh_node are strictly
related to the B.A.T.M.A.N. IV algorithm. In order to
make the struct usable by any routing algorithm it has to be
split and made more generic

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c        | 65 ++++++++++++++++++++++++++++-------------------------
 gateway_client.c    | 16 ++++++-------
 network-coding.c    |  8 ++++---
 originator.c        | 28 ++++++++++++-----------
 originator.h        |  3 ++-
 routing.c           |  9 ++++----
 translation-table.c |  4 ++--
 types.h             | 42 ++++++++++++++++++++--------------
 8 files changed, 96 insertions(+), 79 deletions(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 8d87f87..d885aa8 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -76,20 +76,18 @@ batadv_iv_ogm_neigh_new(struct batadv_hard_iface *hard_iface,
 			struct batadv_orig_node *orig_node,
 			struct batadv_orig_node *orig_neigh)
 {
+	struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
 	struct batadv_neigh_node *neigh_node;
 
-	neigh_node = batadv_neigh_node_new(hard_iface, neigh_addr);
+	neigh_node = batadv_neigh_node_new(hard_iface, neigh_addr, orig_node);
 	if (!neigh_node)
 		goto out;
 
-	INIT_LIST_HEAD(&neigh_node->bonding_list);
+	spin_lock_init(&neigh_node->bat_iv.lq_update_lock);
 
-	neigh_node->orig_node = orig_neigh;
-	neigh_node->if_incoming = hard_iface;
-
-	spin_lock_bh(&orig_node->neigh_list_lock);
-	hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list);
-	spin_unlock_bh(&orig_node->neigh_list_lock);
+	batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
+		   "Creating new neighbor %pM for orig_node %pM on interface %s\n",
+		   neigh_addr, orig_node->orig, hard_iface->net_dev->name);
 
 out:
 	return neigh_node;
@@ -737,12 +735,12 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
 		if (is_duplicate)
 			continue;
 
-		spin_lock_bh(&tmp_neigh_node->lq_update_lock);
-		batadv_ring_buffer_set(tmp_neigh_node->tq_recv,
-				       &tmp_neigh_node->tq_index, 0);
-		tq_avg = batadv_ring_buffer_avg(tmp_neigh_node->tq_recv);
-		tmp_neigh_node->tq_avg = tq_avg;
-		spin_unlock_bh(&tmp_neigh_node->lq_update_lock);
+		spin_lock_bh(&tmp_neigh_node->bat_iv.lq_update_lock);
+		batadv_ring_buffer_set(tmp_neigh_node->bat_iv.tq_recv,
+				       &tmp_neigh_node->bat_iv.tq_index, 0);
+		tq_avg = batadv_ring_buffer_avg(tmp_neigh_node->bat_iv.tq_recv);
+		tmp_neigh_node->bat_iv.tq_avg = tq_avg;
+		spin_unlock_bh(&tmp_neigh_node->bat_iv.lq_update_lock);
 	}
 
 	if (!neigh_node) {
@@ -767,12 +765,13 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
 
 	neigh_node->last_seen = jiffies;
 
-	spin_lock_bh(&neigh_node->lq_update_lock);
-	batadv_ring_buffer_set(neigh_node->tq_recv,
-			       &neigh_node->tq_index,
+	spin_lock_bh(&neigh_node->bat_iv.lq_update_lock);
+	batadv_ring_buffer_set(neigh_node->bat_iv.tq_recv,
+			       &neigh_node->bat_iv.tq_index,
 			       batadv_ogm_packet->tq);
-	neigh_node->tq_avg = batadv_ring_buffer_avg(neigh_node->tq_recv);
-	spin_unlock_bh(&neigh_node->lq_update_lock);
+	tq_avg = batadv_ring_buffer_avg(neigh_node->bat_iv.tq_recv);
+	neigh_node->bat_iv.tq_avg = tq_avg;
+	spin_unlock_bh(&neigh_node->bat_iv.lq_update_lock);
 
 	if (!is_duplicate) {
 		orig_node->last_ttl = batadv_ogm_packet->header.ttl;
@@ -789,13 +788,13 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
 		goto out;
 
 	/* if this neighbor does not offer a better TQ we won't consider it */
-	if (router && (router->tq_avg > neigh_node->tq_avg))
+	if (router && (router->bat_iv.tq_avg > neigh_node->bat_iv.tq_avg))
 		goto out;
 
 	/* if the TQ is the same and the link not more symmetric we
 	 * won't consider it either
 	 */
-	if (router && (neigh_node->tq_avg == router->tq_avg)) {
+	if (router && (neigh_node->bat_iv.tq_avg == router->bat_iv.tq_avg)) {
 		orig_node_tmp = router->orig_node;
 		spin_lock_bh(&orig_node_tmp->ogm_cnt_lock);
 		if_num = router->if_incoming->if_num;
@@ -874,7 +873,7 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node,
 	/* find packet count of corresponding one hop neighbor */
 	spin_lock_bh(&orig_node->ogm_cnt_lock);
 	orig_eq_count = orig_neigh_node->bcast_own_sum[if_incoming->if_num];
-	neigh_rq_count = neigh_node->real_packet_count;
+	neigh_rq_count = neigh_node->bat_iv.real_packet_count;
 	spin_unlock_bh(&orig_node->ogm_cnt_lock);
 
 	/* pay attention to not get a value bigger than 100 % */
@@ -955,6 +954,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 	uint32_t seqno = ntohl(batadv_ogm_packet->seqno);
 	uint8_t *neigh_addr;
 	uint8_t packet_count;
+	unsigned long (*bitmap)[];
 
 	orig_node = batadv_get_orig_node(bat_priv, batadv_ogm_packet->orig);
 	if (!orig_node)
@@ -972,7 +972,8 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(tmp_neigh_node,
 				 &orig_node->neigh_list, list) {
-		is_duplicate |= batadv_test_bit(tmp_neigh_node->real_bits,
+		bitmap = &tmp_neigh_node->bat_iv.real_bits;
+		is_duplicate |= batadv_test_bit(*bitmap,
 						orig_node->last_real_seqno,
 						seqno);
 
@@ -984,13 +985,13 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 			set_mark = 0;
 
 		/* if the window moved, set the update flag. */
-		need_update |= batadv_bit_get_packet(bat_priv,
-						     tmp_neigh_node->real_bits,
+		bitmap = &tmp_neigh_node->bat_iv.real_bits;
+		need_update |= batadv_bit_get_packet(bat_priv, *bitmap,
 						     seq_diff, set_mark);
 
-		packet_count = bitmap_weight(tmp_neigh_node->real_bits,
+		packet_count = bitmap_weight(tmp_neigh_node->bat_iv.real_bits,
 					     BATADV_TQ_LOCAL_WINDOW_SIZE);
-		tmp_neigh_node->real_packet_count = packet_count;
+		tmp_neigh_node->bat_iv.real_packet_count = packet_count;
 	}
 	rcu_read_unlock();
 
@@ -1016,7 +1017,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
 {
 	struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface);
 	struct batadv_hard_iface *hard_iface;
-	struct batadv_orig_node *orig_neigh_node, *orig_node;
+	struct batadv_orig_node *orig_neigh_node, *orig_node, *orig_node_tmp;
 	struct batadv_neigh_node *router = NULL, *router_router = NULL;
 	struct batadv_neigh_node *orig_neigh_router = NULL;
 	int has_directlink_flag;
@@ -1166,10 +1167,12 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
 	}
 
 	router = batadv_orig_node_get_router(orig_node);
-	if (router)
-		router_router = batadv_orig_node_get_router(router->orig_node);
+	if (router) {
+		orig_node_tmp = router->orig_node;
+		router_router = batadv_orig_node_get_router(orig_node_tmp);
+	}
 
-	if ((router && router->tq_avg != 0) &&
+	if ((router && router->bat_iv.tq_avg != 0) &&
 	    (batadv_compare_eth(router->addr, ethhdr->h_source)))
 		is_from_best_next_hop = true;
 
diff --git a/gateway_client.c b/gateway_client.c
index 278e8f6..69488b2 100644
--- a/gateway_client.c
+++ b/gateway_client.c
@@ -137,7 +137,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
 		if (!atomic_inc_not_zero(&gw_node->refcount))
 			goto next;
 
-		tq_avg = router->tq_avg;
+		tq_avg = router->bat_iv.tq_avg;
 
 		switch (atomic_read(&bat_priv->gw_sel_class)) {
 		case 1: /* fast connection */
@@ -234,7 +234,7 @@ void batadv_gw_election(struct batadv_priv *bat_priv)
 			   next_gw->bandwidth_down / 10,
 			   next_gw->bandwidth_down % 10,
 			   next_gw->bandwidth_up / 10,
-			   next_gw->bandwidth_up % 10, router->tq_avg);
+			   next_gw->bandwidth_up % 10, router->bat_iv.tq_avg);
 		batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_ADD,
 				    gw_addr);
 	} else {
@@ -244,7 +244,7 @@ void batadv_gw_election(struct batadv_priv *bat_priv)
 			   next_gw->bandwidth_down / 10,
 			   next_gw->bandwidth_down % 10,
 			   next_gw->bandwidth_up / 10,
-			   next_gw->bandwidth_up % 10, router->tq_avg);
+			   next_gw->bandwidth_up % 10, router->bat_iv.tq_avg);
 		batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_CHANGE,
 				    gw_addr);
 	}
@@ -283,8 +283,8 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv,
 	if (!router_orig)
 		goto out;
 
-	gw_tq_avg = router_gw->tq_avg;
-	orig_tq_avg = router_orig->tq_avg;
+	gw_tq_avg = router_gw->bat_iv.tq_avg;
+	orig_tq_avg = router_orig->bat_iv.tq_avg;
 
 	/* the TQ value has to be better */
 	if (orig_tq_avg < gw_tq_avg)
@@ -506,7 +506,7 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv,
 	ret = seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n",
 			 (curr_gw == gw_node ? "=>" : "  "),
 			 gw_node->orig_node->orig,
-			 router->tq_avg, router->addr,
+			 router->bat_iv.tq_avg, router->addr,
 			 router->if_incoming->net_dev->name,
 			 gw_node->bandwidth_down / 10,
 			 gw_node->bandwidth_down % 10,
@@ -759,7 +759,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
 		if (!neigh_curr)
 			goto out;
 
-		curr_tq_avg = neigh_curr->tq_avg;
+		curr_tq_avg = neigh_curr->bat_iv.tq_avg;
 		break;
 	case BATADV_GW_MODE_OFF:
 	default:
@@ -770,7 +770,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
 	if (!neigh_old)
 		goto out;
 
-	if (curr_tq_avg - neigh_old->tq_avg > BATADV_GW_THRESHOLD)
+	if (curr_tq_avg - neigh_old->bat_iv.tq_avg > BATADV_GW_THRESHOLD)
 		out_of_range = true;
 
 out:
diff --git a/network-coding.c b/network-coding.c
index ef88a1f..173a96e 100644
--- a/network-coding.c
+++ b/network-coding.c
@@ -994,7 +994,7 @@ static bool batadv_nc_code_packets(struct batadv_priv *bat_priv,
 				   struct batadv_nc_packet *nc_packet,
 				   struct batadv_neigh_node *neigh_node)
 {
-	uint8_t tq_weighted_neigh, tq_weighted_coding;
+	uint8_t tq_weighted_neigh, tq_weighted_coding, tq_tmp;
 	struct sk_buff *skb_dest, *skb_src;
 	struct batadv_unicast_packet *packet1;
 	struct batadv_unicast_packet *packet2;
@@ -1019,8 +1019,10 @@ static bool batadv_nc_code_packets(struct batadv_priv *bat_priv,
 	if (!router_coding)
 		goto out;
 
-	tq_weighted_neigh = batadv_nc_random_weight_tq(router_neigh->tq_avg);
-	tq_weighted_coding = batadv_nc_random_weight_tq(router_coding->tq_avg);
+	tq_tmp = batadv_nc_random_weight_tq(router_neigh->bat_iv.tq_avg);
+	tq_weighted_neigh = tq_tmp;
+	tq_tmp = batadv_nc_random_weight_tq(router_coding->bat_iv.tq_avg);
+	tq_weighted_coding = tq_tmp;
 
 	/* Select one destination for the MAC-header dst-field based on
 	 * weighted TQ-values.
diff --git a/originator.c b/originator.c
index c878a00..3d312de 100644
--- a/originator.c
+++ b/originator.c
@@ -174,27 +174,28 @@ batadv_orig_node_get_router(struct batadv_orig_node *orig_node)
 
 struct batadv_neigh_node *
 batadv_neigh_node_new(struct batadv_hard_iface *hard_iface,
-		      const uint8_t *neigh_addr)
+		      const uint8_t *neigh_addr,
+		      struct batadv_orig_node *orig_node)
 {
-	struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
 	struct batadv_neigh_node *neigh_node;
 
 	neigh_node = kzalloc(sizeof(*neigh_node), GFP_ATOMIC);
 	if (!neigh_node)
 		goto out;
 
-	INIT_HLIST_NODE(&neigh_node->list);
-
 	memcpy(neigh_node->addr, neigh_addr, ETH_ALEN);
-	spin_lock_init(&neigh_node->lq_update_lock);
+	neigh_node->if_incoming = hard_iface;
+	neigh_node->orig_node = orig_node;
+
+	spin_lock_bh(&orig_node->neigh_list_lock);
+	hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list);
+	spin_unlock_bh(&orig_node->neigh_list_lock);
+
+	INIT_LIST_HEAD(&neigh_node->bonding_list);
 
 	/* extra reference for return */
 	atomic_set(&neigh_node->refcount, 2);
 
-	batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
-		   "Creating new neighbor %pM on interface %s\n", neigh_addr,
-		   hard_iface->net_dev->name);
-
 out:
 	return neigh_node;
 }
@@ -432,7 +433,8 @@ batadv_purge_orig_neighbors(struct batadv_priv *bat_priv,
 			batadv_neigh_node_free_ref(neigh_node);
 		} else {
 			if ((!*best_neigh_node) ||
-			    (neigh_node->tq_avg > (*best_neigh_node)->tq_avg))
+			    (neigh_node->bat_iv.tq_avg >
+			     (*best_neigh_node)->bat_iv.tq_avg))
 				*best_neigh_node = neigh_node;
 		}
 	}
@@ -553,7 +555,7 @@ int batadv_orig_seq_print_text(struct seq_file *seq, void *offset)
 			if (!neigh_node)
 				continue;
 
-			if (neigh_node->tq_avg == 0)
+			if (neigh_node->bat_iv.tq_avg == 0)
 				goto next;
 
 			last_seen_jiffies = jiffies - orig_node->last_seen;
@@ -563,7 +565,7 @@ int batadv_orig_seq_print_text(struct seq_file *seq, void *offset)
 
 			seq_printf(seq, "%pM %4i.%03is   (%3i) %pM [%10s]:",
 				   orig_node->orig, last_seen_secs,
-				   last_seen_msecs, neigh_node->tq_avg,
+				   last_seen_msecs, neigh_node->bat_iv.tq_avg,
 				   neigh_node->addr,
 				   neigh_node->if_incoming->net_dev->name);
 
@@ -571,7 +573,7 @@ int batadv_orig_seq_print_text(struct seq_file *seq, void *offset)
 						 &orig_node->neigh_list, list) {
 				seq_printf(seq, " %pM (%3i)",
 					   neigh_node_tmp->addr,
-					   neigh_node_tmp->tq_avg);
+					   neigh_node_tmp->bat_iv.tq_avg);
 			}
 
 			seq_puts(seq, "\n");
diff --git a/originator.h b/originator.h
index cc6d686..06e5a68 100644
--- a/originator.h
+++ b/originator.h
@@ -31,7 +31,8 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
 					      const uint8_t *addr);
 struct batadv_neigh_node *
 batadv_neigh_node_new(struct batadv_hard_iface *hard_iface,
-		      const uint8_t *neigh_addr);
+		      const uint8_t *neigh_addr,
+		      struct batadv_orig_node *orig_node);
 void batadv_neigh_node_free_ref(struct batadv_neigh_node *neigh_node);
 struct batadv_neigh_node *
 batadv_orig_node_get_router(struct batadv_orig_node *orig_node);
diff --git a/routing.c b/routing.c
index 2b35205..99f6844 100644
--- a/routing.c
+++ b/routing.c
@@ -133,7 +133,8 @@ void batadv_bonding_candidate_add(struct batadv_orig_node *orig_node,
 		goto candidate_del;
 
 	/* ... and is good enough to be considered */
-	if (neigh_node->tq_avg < router->tq_avg - BATADV_BONDING_TQ_THRESHOLD)
+	if (neigh_node->bat_iv.tq_avg <
+	    router->bat_iv.tq_avg - BATADV_BONDING_TQ_THRESHOLD)
 		goto candidate_del;
 
 	/* check if we have another candidate with the same mac address or
@@ -470,8 +471,7 @@ batadv_find_bond_router(struct batadv_orig_node *primary_orig,
 	 * does not exist as rcu version
 	 */
 	list_del_rcu(&primary_orig->bond_list);
-	list_add_rcu(&primary_orig->bond_list,
-		     &router->bonding_list);
+	list_add_rcu(&primary_orig->bond_list, &router->bonding_list);
 	spin_unlock_bh(&primary_orig->neigh_list_lock);
 
 out:
@@ -502,7 +502,8 @@ batadv_find_ifalter_router(struct batadv_orig_node *primary_orig,
 		if (tmp_neigh_node->if_incoming == recv_if)
 			continue;
 
-		if (router && tmp_neigh_node->tq_avg <= router->tq_avg)
+		if (router &&
+		    tmp_neigh_node->bat_iv.tq_avg <= router->bat_iv.tq_avg)
 			continue;
 
 		if (!atomic_inc_not_zero(&tmp_neigh_node->refcount))
diff --git a/translation-table.c b/translation-table.c
index 9401ca8..a256cdb 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1229,9 +1229,9 @@ batadv_transtable_best_orig(struct batadv_tt_global_entry *tt_global_entry)
 		if (!router)
 			continue;
 
-		if (router->tq_avg > best_tq) {
+		if (router->bat_iv.tq_avg > best_tq) {
 			best_entry = orig_entry;
-			best_tq = router->tq_avg;
+			best_tq = router->bat_iv.tq_avg;
 		}
 
 		batadv_neigh_node_free_ref(router);
diff --git a/types.h b/types.h
index a4d6d9f..c7d5fa1 100644
--- a/types.h
+++ b/types.h
@@ -257,40 +257,48 @@ struct batadv_gw_node {
 };
 
 /**
- * struct batadv_neigh_node - structure for single hop neighbors
- * @list: list node for batadv_orig_node::neigh_list
- * @addr: mac address of neigh node
+ * struct batadv_neigh_bat_iv - structure for single hop neighbors
  * @tq_recv: ring buffer of received TQ values from this neigh node
  * @tq_index: ring buffer index
  * @tq_avg: averaged tq of all tq values in the ring buffer (tq_recv)
- * @last_ttl: last received ttl from this neigh node
- * @bonding_list: list node for batadv_orig_node::bond_list
- * @last_seen: when last packet via this neighbor was received
  * @real_bits: bitfield containing the number of OGMs received from this neigh
  *  node (relative to orig_node->last_real_seqno)
  * @real_packet_count: counted result of real_bits
- * @orig_node: pointer to corresponding orig_node
- * @if_incoming: pointer to incoming hard interface
  * @lq_update_lock: lock protecting tq_recv & tq_index
  * @refcount: number of contexts the object is used
- * @rcu: struct used for freeing in an RCU-safe manner
  */
-struct batadv_neigh_node {
-	struct hlist_node list;
-	uint8_t addr[ETH_ALEN];
+struct batadv_neigh_bat_iv {
 	uint8_t tq_recv[BATADV_TQ_GLOBAL_WINDOW_SIZE];
 	uint8_t tq_index;
 	uint8_t tq_avg;
-	uint8_t last_ttl;
-	struct list_head bonding_list;
-	unsigned long last_seen;
 	DECLARE_BITMAP(real_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
 	uint8_t real_packet_count;
-	struct batadv_orig_node *orig_node;
-	struct batadv_hard_iface *if_incoming;
 	spinlock_t lq_update_lock; /* protects tq_recv & tq_index */
+};
+
+/**
+ * struct batadv_neigh_node - structure for single hops neighbors
+ * @list: list node for batadv_orig_node::neigh_list
+ * @orig_node: pointer to corresponding orig_node
+ * @addr: the MAC address of the neighboring interface
+ * @if_incoming: pointer to incoming hard interface
+ * @last_seen: when last packet via this neighbor was received
+ * @last_ttl: last received ttl from this neigh node
+ * @bonding_list: list node for batadv_orig_node::bond_list
+ * @rcu: struct used for freeing in an RCU-safe manner
+ * @priv: pointer to the routing algorithm private data
+ */
+struct batadv_neigh_node {
+	struct hlist_node list;
+	struct batadv_orig_node *orig_node;
+	uint8_t addr[ETH_ALEN];
+	struct batadv_hard_iface *if_incoming;
+	unsigned long last_seen;
+	uint8_t last_ttl;
+	struct list_head bonding_list;
 	atomic_t refcount;
 	struct rcu_head rcu;
+	struct batadv_neigh_bat_iv bat_iv;
 };
 
 /**
-- 
1.8.1.5

[B.A.T.M.A.N.] [RFC 02/10] batman-adv: make struct batadv_orig_node algorithm agnostic

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

some of the struct batadv_orig_node members are B.A.T.M.A.N. IV
specific and therefore they are moved in a algorithm specific
substruct in order to make batadv_orig_node routing algorithm
agnostic

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c | 108 ++++++++++++++++++++++++++++++++++++++++-------------------
 originator.c |  83 +++++++++++++++++----------------------------
 originator.h |   2 +-
 routing.c    |  10 ++++--
 types.h      |  50 +++++++++++++++------------
 5 files changed, 142 insertions(+), 111 deletions(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index d885aa8..8e8dc5d 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -70,6 +70,43 @@ static uint8_t batadv_ring_buffer_avg(const uint8_t lq_recv[])
 
 	return (uint8_t)(sum / count);
 }
+
+static struct batadv_orig_node *
+batadv_iv_ogm_orig_get(struct batadv_priv *bat_priv, const uint8_t *addr)
+{
+	struct batadv_orig_node *orig_node;
+	int size;
+
+	orig_node = batadv_orig_hash_find(bat_priv, addr);
+	if (orig_node)
+		return orig_node;
+
+	orig_node = batadv_orig_node_new(bat_priv, addr);
+	if (!orig_node)
+		return NULL;
+
+	spin_lock_init(&orig_node->bat_iv.ogm_cnt_lock);
+
+	size = bat_priv->num_ifaces * sizeof(unsigned long) * BATADV_NUM_WORDS;
+	orig_node->bat_iv.bcast_own = kzalloc(size, GFP_ATOMIC);
+	if (!orig_node->bat_iv.bcast_own)
+		goto free_orig_node;
+
+	size = bat_priv->num_ifaces * sizeof(uint8_t);
+	orig_node->bat_iv.bcast_own_sum = kzalloc(size, GFP_ATOMIC);
+	if (!orig_node->bat_iv.bcast_own_sum)
+		goto free_bcast_own;
+
+	return orig_node;
+
+free_bcast_own:
+	kfree(orig_node->bat_iv.bcast_own);
+free_orig_node:
+	batadv_orig_node_free_ref(orig_node);
+
+	return NULL;
+}
+
 static struct batadv_neigh_node *
 batadv_iv_ogm_neigh_new(struct batadv_hard_iface *hard_iface,
 			const uint8_t *neigh_addr,
@@ -647,14 +684,14 @@ batadv_iv_ogm_slide_own_bcast_window(struct batadv_hard_iface *hard_iface)
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
-			spin_lock_bh(&orig_node->ogm_cnt_lock);
+			spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 			word_index = hard_iface->if_num * BATADV_NUM_WORDS;
-			word = &(orig_node->bcast_own[word_index]);
+			word = &(orig_node->bat_iv.bcast_own[word_index]);
 
 			batadv_bit_get_packet(bat_priv, word, 1, 0);
-			w = &orig_node->bcast_own_sum[hard_iface->if_num];
+			w = &orig_node->bat_iv.bcast_own_sum[hard_iface->if_num];
 			*w = bitmap_weight(word, BATADV_TQ_LOCAL_WINDOW_SIZE);
-			spin_unlock_bh(&orig_node->ogm_cnt_lock);
+			spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 		}
 		rcu_read_unlock();
 	}
@@ -746,7 +783,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
 	if (!neigh_node) {
 		struct batadv_orig_node *orig_tmp;
 
-		orig_tmp = batadv_get_orig_node(bat_priv, ethhdr->h_source);
+		orig_tmp = batadv_iv_ogm_orig_get(bat_priv, ethhdr->h_source);
 		if (!orig_tmp)
 			goto unlock;
 
@@ -796,16 +833,16 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
 	 */
 	if (router && (neigh_node->bat_iv.tq_avg == router->bat_iv.tq_avg)) {
 		orig_node_tmp = router->orig_node;
-		spin_lock_bh(&orig_node_tmp->ogm_cnt_lock);
+		spin_lock_bh(&orig_node_tmp->bat_iv.ogm_cnt_lock);
 		if_num = router->if_incoming->if_num;
-		sum_orig = orig_node_tmp->bcast_own_sum[if_num];
-		spin_unlock_bh(&orig_node_tmp->ogm_cnt_lock);
+		sum_orig = orig_node_tmp->bat_iv.bcast_own_sum[if_num];
+		spin_unlock_bh(&orig_node_tmp->bat_iv.ogm_cnt_lock);
 
 		orig_node_tmp = neigh_node->orig_node;
-		spin_lock_bh(&orig_node_tmp->ogm_cnt_lock);
+		spin_lock_bh(&orig_node_tmp->bat_iv.ogm_cnt_lock);
 		if_num = neigh_node->if_incoming->if_num;
-		sum_neigh = orig_node_tmp->bcast_own_sum[if_num];
-		spin_unlock_bh(&orig_node_tmp->ogm_cnt_lock);
+		sum_neigh = orig_node_tmp->bat_iv.bcast_own_sum[if_num];
+		spin_unlock_bh(&orig_node_tmp->bat_iv.ogm_cnt_lock);
 
 		if (sum_orig >= sum_neigh)
 			goto out;
@@ -833,7 +870,7 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node,
 	uint8_t total_count;
 	uint8_t orig_eq_count, neigh_rq_count, neigh_rq_inv, tq_own;
 	unsigned int neigh_rq_inv_cube, neigh_rq_max_cube;
-	int tq_asym_penalty, inv_asym_penalty, ret = 0;
+	int tq_asym_penalty, inv_asym_penalty, if_num, ret = 0;
 	unsigned int combined_tq;
 
 	/* find corresponding one hop neighbor */
@@ -871,10 +908,11 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node,
 	orig_node->last_seen = jiffies;
 
 	/* find packet count of corresponding one hop neighbor */
-	spin_lock_bh(&orig_node->ogm_cnt_lock);
-	orig_eq_count = orig_neigh_node->bcast_own_sum[if_incoming->if_num];
+	spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
+	if_num = if_incoming->if_num;
+	orig_eq_count = orig_neigh_node->bat_iv.bcast_own_sum[if_num];
 	neigh_rq_count = neigh_node->bat_iv.real_packet_count;
-	spin_unlock_bh(&orig_node->ogm_cnt_lock);
+	spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 
 	/* pay attention to not get a value bigger than 100 % */
 	if (orig_eq_count > neigh_rq_count)
@@ -951,16 +989,16 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 	int32_t seq_diff;
 	int need_update = 0;
 	int set_mark, ret = -1;
-	uint32_t seqno = ntohl(batadv_ogm_packet->seqno);
+	uint32_t last_real_seqno, seqno = ntohl(batadv_ogm_packet->seqno);
 	uint8_t *neigh_addr;
 	uint8_t packet_count;
 	unsigned long (*bitmap)[];
 
-	orig_node = batadv_get_orig_node(bat_priv, batadv_ogm_packet->orig);
+	orig_node = batadv_iv_ogm_orig_get(bat_priv, batadv_ogm_packet->orig);
 	if (!orig_node)
 		return 0;
 
-	spin_lock_bh(&orig_node->ogm_cnt_lock);
+	spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 	seq_diff = seqno - orig_node->last_real_seqno;
 
 	/* signalize caller that the packet is to be dropped. */
@@ -973,8 +1011,8 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 	hlist_for_each_entry_rcu(tmp_neigh_node,
 				 &orig_node->neigh_list, list) {
 		bitmap = &tmp_neigh_node->bat_iv.real_bits;
-		is_duplicate |= batadv_test_bit(*bitmap,
-						orig_node->last_real_seqno,
+		last_real_seqno = orig_node->last_real_seqno;
+		is_duplicate |= batadv_test_bit(*bitmap, last_real_seqno,
 						seqno);
 
 		neigh_addr = tmp_neigh_node->addr;
@@ -1005,7 +1043,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
 	ret = is_duplicate;
 
 out:
-	spin_unlock_bh(&orig_node->ogm_cnt_lock);
+	spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 	batadv_orig_node_free_ref(orig_node);
 	return ret;
 }
@@ -1026,8 +1064,8 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
 	bool is_single_hop_neigh = false;
 	bool is_from_best_next_hop = false;
 	int is_duplicate, sameseq, simlar_ttl;
-	uint32_t if_incoming_seqno;
-	uint8_t *prev_sender;
+	uint32_t seqno, if_incoming_seqno;
+	uint8_t ttl, *prev_sender;
 
 	/* Silently drop when the batman packet is actually not a
 	 * correct packet.
@@ -1100,8 +1138,8 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
 		int16_t if_num;
 		uint8_t *weight;
 
-		orig_neigh_node = batadv_get_orig_node(bat_priv,
-						       ethhdr->h_source);
+		orig_neigh_node = batadv_iv_ogm_orig_get(bat_priv,
+							 ethhdr->h_source);
 		if (!orig_neigh_node)
 			return;
 
@@ -1115,15 +1153,15 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
 			if_num = if_incoming->if_num;
 			offset = if_num * BATADV_NUM_WORDS;
 
-			spin_lock_bh(&orig_neigh_node->ogm_cnt_lock);
-			word = &(orig_neigh_node->bcast_own[offset]);
+			spin_lock_bh(&orig_neigh_node->bat_iv.ogm_cnt_lock);
+			word = &(orig_neigh_node->bat_iv.bcast_own[offset]);
 			bit_pos = if_incoming_seqno - 2;
 			bit_pos -= ntohl(batadv_ogm_packet->seqno);
 			batadv_set_bit(word, bit_pos);
-			weight = &orig_neigh_node->bcast_own_sum[if_num];
+			weight = &orig_neigh_node->bat_iv.bcast_own_sum[if_num];
 			*weight = bitmap_weight(word,
 						BATADV_TQ_LOCAL_WINDOW_SIZE);
-			spin_unlock_bh(&orig_neigh_node->ogm_cnt_lock);
+			spin_unlock_bh(&orig_neigh_node->bat_iv.ogm_cnt_lock);
 		}
 
 		batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
@@ -1146,7 +1184,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
 		return;
 	}
 
-	orig_node = batadv_get_orig_node(bat_priv, batadv_ogm_packet->orig);
+	orig_node = batadv_iv_ogm_orig_get(bat_priv, batadv_ogm_packet->orig);
 	if (!orig_node)
 		return;
 
@@ -1196,8 +1234,8 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
 	if (is_single_hop_neigh)
 		orig_neigh_node = orig_node;
 	else
-		orig_neigh_node = batadv_get_orig_node(bat_priv,
-						       ethhdr->h_source);
+		orig_neigh_node = batadv_iv_ogm_orig_get(bat_priv,
+							 ethhdr->h_source);
 
 	if (!orig_neigh_node)
 		goto out;
@@ -1226,8 +1264,10 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
 	/* update ranking if it is not a duplicate or has the same
 	 * seqno and similar ttl as the non-duplicate
 	 */
-	sameseq = orig_node->last_real_seqno == ntohl(batadv_ogm_packet->seqno);
-	simlar_ttl = orig_node->last_ttl - 3 <= batadv_ogm_packet->header.ttl;
+	seqno = ntohl(batadv_ogm_packet->seqno);
+	sameseq = orig_node->last_real_seqno == seqno;
+	ttl = batadv_ogm_packet->header.ttl;
+	simlar_ttl = orig_node->last_ttl - 3 <= ttl;
 	if (is_bidirect && (!is_duplicate || (sameseq && simlar_ttl)))
 		batadv_iv_ogm_orig_update(bat_priv, orig_node, ethhdr,
 					  batadv_ogm_packet, if_incoming,
diff --git a/originator.c b/originator.c
index 3d312de..0225ae2 100644
--- a/originator.c
+++ b/originator.c
@@ -235,8 +235,8 @@ static void batadv_orig_node_free_rcu(struct rcu_head *rcu)
 				  "originator timed out");
 
 	kfree(orig_node->tt_buff);
-	kfree(orig_node->bcast_own);
-	kfree(orig_node->bcast_own_sum);
+	kfree(orig_node->bat_iv.bcast_own);
+	kfree(orig_node->bat_iv.bcast_own_sum);
 	kfree(orig_node);
 }
 
@@ -297,18 +297,13 @@ void batadv_originator_free(struct batadv_priv *bat_priv)
 /* this function finds or creates an originator entry for the given
  * address if it does not exits
  */
-struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
+struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv,
 					      const uint8_t *addr)
 {
 	struct batadv_orig_node *orig_node;
 	struct batadv_orig_node_vlan *vlan;
-	int size, i;
-	int hash_added;
 	unsigned long reset_time;
-
-	orig_node = batadv_orig_hash_find(bat_priv, addr);
-	if (orig_node)
-		return orig_node;
+	int i, hash_added;
 
 	batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
 		   "Creating new originator: %pM\n", addr);
@@ -320,8 +315,6 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
 	INIT_HLIST_HEAD(&orig_node->neigh_list);
 	INIT_LIST_HEAD(&orig_node->bond_list);
 	INIT_LIST_HEAD(&orig_node->vlan_list);
-	spin_lock_init(&orig_node->ogm_cnt_lock);
-	spin_lock_init(&orig_node->bcast_seqno_lock);
 	spin_lock_init(&orig_node->neigh_list_lock);
 	spin_lock_init(&orig_node->tt_buff_lock);
 	spin_lock_init(&orig_node->vlan_list_lock);
@@ -339,11 +332,6 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
 	atomic_set(&orig_node->last_ttvn, 0);
 	orig_node->tt_buff = NULL;
 	orig_node->tt_buff_len = 0;
-	reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
-	orig_node->bcast_seqno_reset = reset_time;
-	orig_node->batman_seqno_reset = reset_time;
-
-	atomic_set(&orig_node->bond_candidates, 0);
 
 	/* create a vlan object for the "untagged" LAN */
 	vlan = batadv_orig_node_vlan_new(orig_node, BATADV_NO_FLAGS);
@@ -352,14 +340,7 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
 	/* there is nothing else to do with the vlan.. */
 	batadv_orig_node_vlan_free_ref(vlan);
 
-	size = bat_priv->num_ifaces * sizeof(unsigned long) * BATADV_NUM_WORDS;
-
-	orig_node->bcast_own = kzalloc(size, GFP_ATOMIC);
-	if (!orig_node->bcast_own)
-		goto free_vlan;
-
-	size = bat_priv->num_ifaces * sizeof(uint8_t);
-	orig_node->bcast_own_sum = kzalloc(size, GFP_ATOMIC);
+	atomic_set(&orig_node->bond_candidates, 0);
 
 	for (i = 0; i < BATADV_FRAG_BUFFER_COUNT; i++) {
 		INIT_HLIST_HEAD(&orig_node->fragments[i].head);
@@ -367,22 +348,20 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
 		orig_node->fragments[i].size = 0;
 	}
 
-	if (!orig_node->bcast_own_sum)
-		goto free_bcast_own;
+	spin_lock_init(&orig_node->bcast_seqno_lock);
+	reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
+	orig_node->bcast_seqno_reset = reset_time;
+	orig_node->batman_seqno_reset = reset_time;
 
 	hash_added = batadv_hash_add(bat_priv->orig_hash, batadv_compare_orig,
 				     batadv_choose_orig, orig_node,
 				     &orig_node->hash_entry);
-	if (hash_added != 0)
-		goto free_bcast_own_sum;
+	if (hash_added != 0) {
+		kfree(orig_node);
+		return NULL;
+	}
 
 	return orig_node;
-free_bcast_own_sum:
-	kfree(orig_node->bcast_own_sum);
-free_bcast_own:
-	kfree(orig_node->bcast_own);
-free_vlan:
-	batadv_orig_node_vlan_free_ref(vlan);
 free_orig_node:
 	kfree(orig_node);
 	return NULL;
@@ -606,18 +585,18 @@ static int batadv_orig_node_add_if(struct batadv_orig_node *orig_node,
 	if (!data_ptr)
 		return -ENOMEM;
 
-	memcpy(data_ptr, orig_node->bcast_own, old_size);
-	kfree(orig_node->bcast_own);
-	orig_node->bcast_own = data_ptr;
+	memcpy(data_ptr, orig_node->bat_iv.bcast_own, old_size);
+	kfree(orig_node->bat_iv.bcast_own);
+	orig_node->bat_iv.bcast_own = data_ptr;
 
 	data_ptr = kmalloc(max_if_num * sizeof(uint8_t), GFP_ATOMIC);
 	if (!data_ptr)
 		return -ENOMEM;
 
-	memcpy(data_ptr, orig_node->bcast_own_sum,
+	memcpy(data_ptr, orig_node->bat_iv.bcast_own_sum,
 	       (max_if_num - 1) * sizeof(uint8_t));
-	kfree(orig_node->bcast_own_sum);
-	orig_node->bcast_own_sum = data_ptr;
+	kfree(orig_node->bat_iv.bcast_own_sum);
+	orig_node->bat_iv.bcast_own_sum = data_ptr;
 
 	return 0;
 }
@@ -640,9 +619,9 @@ int batadv_orig_hash_add_if(struct batadv_hard_iface *hard_iface,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
-			spin_lock_bh(&orig_node->ogm_cnt_lock);
+			spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 			ret = batadv_orig_node_add_if(orig_node, max_if_num);
-			spin_unlock_bh(&orig_node->ogm_cnt_lock);
+			spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 
 			if (ret == -ENOMEM)
 				goto err;
@@ -673,16 +652,16 @@ static int batadv_orig_node_del_if(struct batadv_orig_node *orig_node,
 		return -ENOMEM;
 
 	/* copy first part */
-	memcpy(data_ptr, orig_node->bcast_own, del_if_num * chunk_size);
+	memcpy(data_ptr, orig_node->bat_iv.bcast_own, del_if_num * chunk_size);
 
 	/* copy second part */
 	memcpy((char *)data_ptr + del_if_num * chunk_size,
-	       orig_node->bcast_own + ((del_if_num + 1) * chunk_size),
+	       orig_node->bat_iv.bcast_own + ((del_if_num + 1) * chunk_size),
 	       (max_if_num - del_if_num) * chunk_size);
 
 free_bcast_own:
-	kfree(orig_node->bcast_own);
-	orig_node->bcast_own = data_ptr;
+	kfree(orig_node->bat_iv.bcast_own);
+	orig_node->bat_iv.bcast_own = data_ptr;
 
 	if (max_if_num == 0)
 		goto free_own_sum;
@@ -691,16 +670,16 @@ free_bcast_own:
 	if (!data_ptr)
 		return -ENOMEM;
 
-	memcpy(data_ptr, orig_node->bcast_own_sum,
+	memcpy(data_ptr, orig_node->bat_iv.bcast_own_sum,
 	       del_if_num * sizeof(uint8_t));
 
 	memcpy((char *)data_ptr + del_if_num * sizeof(uint8_t),
-	       orig_node->bcast_own_sum + ((del_if_num + 1) * sizeof(uint8_t)),
+	       orig_node->bat_iv.bcast_own_sum + ((del_if_num + 1) * sizeof(uint8_t)),
 	       (max_if_num - del_if_num) * sizeof(uint8_t));
 
 free_own_sum:
-	kfree(orig_node->bcast_own_sum);
-	orig_node->bcast_own_sum = data_ptr;
+	kfree(orig_node->bat_iv.bcast_own_sum);
+	orig_node->bat_iv.bcast_own_sum = data_ptr;
 
 	return 0;
 }
@@ -724,10 +703,10 @@ int batadv_orig_hash_del_if(struct batadv_hard_iface *hard_iface,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
-			spin_lock_bh(&orig_node->ogm_cnt_lock);
+			spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 			ret = batadv_orig_node_del_if(orig_node, max_if_num,
 						      hard_iface->if_num);
-			spin_unlock_bh(&orig_node->ogm_cnt_lock);
+			spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 
 			if (ret == -ENOMEM)
 				goto err;
diff --git a/originator.h b/originator.h
index 06e5a68..a765a2f 100644
--- a/originator.h
+++ b/originator.h
@@ -27,7 +27,7 @@ void batadv_originator_free(struct batadv_priv *bat_priv);
 void batadv_purge_orig_ref(struct batadv_priv *bat_priv);
 void batadv_orig_node_free_ref(struct batadv_orig_node *orig_node);
 void batadv_orig_node_free_ref_now(struct batadv_orig_node *orig_node);
-struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
+struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv,
 					      const uint8_t *addr);
 struct batadv_neigh_node *
 batadv_neigh_node_new(struct batadv_hard_iface *hard_iface,
diff --git a/routing.c b/routing.c
index 99f6844..46de5c7 100644
--- a/routing.c
+++ b/routing.c
@@ -1075,6 +1075,7 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
 	int hdr_size = sizeof(*bcast_packet);
 	int ret = NET_RX_DROP;
 	int32_t seq_diff;
+	uint32_t seqno;
 
 	/* drop packet if it has not necessary minimum size */
 	if (unlikely(!pskb_may_pull(skb, hdr_size)))
@@ -1111,11 +1112,13 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
 	spin_lock_bh(&orig_node->bcast_seqno_lock);
 
 	/* check whether the packet is a duplicate */
-	if (batadv_test_bit(orig_node->bcast_bits, orig_node->last_bcast_seqno,
+	if (batadv_test_bit(orig_node->bcast_bits,
+			    orig_node->last_bcast_seqno,
 			    ntohl(bcast_packet->seqno)))
 		goto spin_unlock;
 
-	seq_diff = ntohl(bcast_packet->seqno) - orig_node->last_bcast_seqno;
+	seqno = orig_node->last_bcast_seqno;
+	seq_diff = ntohl(bcast_packet->seqno) - seqno;
 
 	/* check whether the packet is old and the host just restarted. */
 	if (batadv_window_protected(bat_priv, seq_diff,
@@ -1125,7 +1128,8 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
 	/* mark broadcast in flood history, update window position
 	 * if required.
 	 */
-	if (batadv_bit_get_packet(bat_priv, orig_node->bcast_bits, seq_diff, 1))
+	if (batadv_bit_get_packet(bat_priv, orig_node->bcast_bits,
+				  seq_diff, 1))
 		orig_node->last_bcast_seqno = ntohl(bcast_packet->seqno);
 
 	spin_unlock_bh(&orig_node->bcast_seqno_lock);
diff --git a/types.h b/types.h
index c7d5fa1..fde4d68 100644
--- a/types.h
+++ b/types.h
@@ -133,18 +133,29 @@ struct batadv_orig_node_vlan {
 };
 
 /**
+ * struct batadv_orig_bat_iv - B.A.T.M.A.N. IV private orig_node members
+ * @bcast_own: bitfield containing the number of our OGMs this orig_node
+ *  rebroadcasted "back" to us (relative to last_real_seqno)
+ * @bcast_own_sum: counted result of bcast_own
+ * @ogm_cnt_lock: lock protecting bcast_own, bcast_own_sum,
+ *  neigh_node->real_bits & neigh_node->real_packet_count
+ */
+struct batadv_orig_bat_iv {
+	unsigned long *bcast_own;
+	uint8_t *bcast_own_sum;
+	spinlock_t ogm_cnt_lock;
+};
+
+/**
  * struct batadv_orig_node - structure for orig_list maintaining nodes of mesh
  * @orig: originator ethernet address
  * @primary_addr: hosts primary interface address
  * @router: router that should be used to reach this originator
- * @batadv_dat_addr_t:  address of the orig node in the distributed hash
- * @bcast_own: bitfield containing the number of our OGMs this orig_node
- *  rebroadcasted "back" to us (relative to last_real_seqno)
- * @bcast_own_sum: counted result of bcast_own
  * @last_seen: time when last packet from this node was received
- * @bcast_seqno_reset: time when the broadcast seqno window was reset
- * @batman_seqno_reset: time when the batman seqno window was reset
+ * @last_ttl: ttl of last received packet
+ * @batadv_dat_addr_t:  address of the orig node in the distributed hash
  * @capabilities: announced capabilities of this originator
+ * @last_real_seqno: last and best known sequence number
  * @last_ttvn: last seen translation table version number
  * @tt_buff: last tt changeset this node received from the orig node
  * @tt_buff_len: length of the last tt changeset this node received from the
@@ -152,19 +163,17 @@ struct batadv_orig_node_vlan {
  * @tt_buff_lock: lock that protects tt_buff and tt_buff_len
  * @tt_initialised: bool keeping track of whether or not this node have received
  *  any translation table information from the orig node yet
- * @last_real_seqno: last and best known sequence number
- * @last_ttl: ttl of last received packet
  * @bcast_bits: bitfield containing the info which payload broadcast originated
  *  from this orig node this host already has seen (relative to
  *  last_bcast_seqno)
  * @last_bcast_seqno: last broadcast sequence number received by this host
+ * @bcast_seqno_lock: lock protecting bcast_bits & last_bcast_seqno
+ * @bcast_seqno_reset: time when the broadcast seqno window was reset
+ * @batman_seqno_reset: time when the batman seqno window was reset
  * @neigh_list: list of potential next hop neighbor towards this orig node
  * @neigh_list_lock: lock protecting neigh_list, router and bonding_list
  * @hash_entry: hlist node for batadv_priv::orig_hash
  * @bat_priv: pointer to soft_iface this orig node belongs to
- * @ogm_cnt_lock: lock protecting bcast_own, bcast_own_sum,
- *  neigh_node->real_bits & neigh_node->real_packet_count
- * @bcast_seqno_lock: lock protecting bcast_bits & last_bcast_seqno
  * @bond_candidates: how many candidates are available
  * @bond_list: list of bonding candidates
  * @refcount: number of contexts the object is used
@@ -177,29 +186,30 @@ struct batadv_orig_node_vlan {
  * @vlan_list: a list of orig_node_vlan structs, one per VLAN served by the
  *  originator represented by this object
  * @vlan_list_lock: lock protecting vlan_list
+ * @bat_iv: B.A.T.M.A.N. IV private structure
  */
 struct batadv_orig_node {
 	uint8_t orig[ETH_ALEN];
 	uint8_t primary_addr[ETH_ALEN];
 	struct batadv_neigh_node __rcu *router; /* rcu protected pointer */
+	unsigned long last_seen;
+	uint8_t last_ttl;
 #ifdef CONFIG_BATMAN_ADV_DAT
 	batadv_dat_addr_t dat_addr;
 #endif
-	unsigned long *bcast_own;
-	uint8_t *bcast_own_sum;
-	unsigned long last_seen;
-	unsigned long bcast_seqno_reset;
-	unsigned long batman_seqno_reset;
 	uint8_t capabilities;
+	uint32_t last_real_seqno;
 	atomic_t last_ttvn;
 	unsigned char *tt_buff;
 	int16_t tt_buff_len;
 	spinlock_t tt_buff_lock; /* protects tt_buff & tt_buff_len */
 	bool tt_initialised;
-	uint32_t last_real_seqno;
-	uint8_t last_ttl;
 	DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
 	uint32_t last_bcast_seqno;
+	/* bcast_seqno_lock protects: bcast_bits & last_bcast_seqno */
+	spinlock_t bcast_seqno_lock;
+	unsigned long bcast_seqno_reset;
+	unsigned long batman_seqno_reset;
 	struct hlist_head neigh_list;
 	/* neigh_list_lock protects: neigh_list, router & bonding_list */
 	spinlock_t neigh_list_lock;
@@ -208,9 +218,6 @@ struct batadv_orig_node {
 	/* ogm_cnt_lock protects: bcast_own, bcast_own_sum,
 	 * neigh_node->real_bits & neigh_node->real_packet_count
 	 */
-	spinlock_t ogm_cnt_lock;
-	/* bcast_seqno_lock protects: bcast_bits & last_bcast_seqno */
-	spinlock_t bcast_seqno_lock;
 	atomic_t bond_candidates;
 	struct list_head bond_list;
 	atomic_t refcount;
@@ -224,6 +231,7 @@ struct batadv_orig_node {
 	struct batadv_frag_table_entry fragments[BATADV_FRAG_BUFFER_COUNT];
 	struct list_head vlan_list;
 	spinlock_t vlan_list_lock; /* protects vlan_list */
+	struct batadv_orig_bat_iv bat_iv;
 };
 
 /**
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [RFC 02/10] batman-adv: make struct batadv_orig_node algorithm agnostic

[Simon Wunderlich]

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

On Wed, May 29, 2013 at 12:20:41AM +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> some of the struct batadv_orig_node members are B.A.T.M.A.N. IV
> specific and therefore they are moved in a algorithm specific
> substruct in order to make batadv_orig_node routing algorithm
> agnostic
> 
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> ---
>  bat_iv_ogm.c | 108 ++++++++++++++++++++++++++++++++++++++++-------------------
>  originator.c |  83 +++++++++++++++++----------------------------
>  originator.h |   2 +-
>  routing.c    |  10 ++++--
>  types.h      |  50 +++++++++++++++------------
>  5 files changed, 142 insertions(+), 111 deletions(-)
> 
> diff --git a/originator.c b/originator.c
> index 3d312de..0225ae2 100644
> --- a/originator.c
> +++ b/originator.c
> @@ -297,18 +297,13 @@ void batadv_originator_free(struct batadv_priv *bat_priv)
>  /* this function finds or creates an originator entry for the given
>   * address if it does not exits
>   */
> -struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
> +struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv,
>  					      const uint8_t *addr)
>  {
>  	struct batadv_orig_node *orig_node;
>  	struct batadv_orig_node_vlan *vlan;
> -	int size, i;
> -	int hash_added;
>  	unsigned long reset_time;
> -
> -	orig_node = batadv_orig_hash_find(bat_priv, addr);
> -	if (orig_node)
> -		return orig_node;
> +	int i, hash_added;
>  
>  	batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
>  		   "Creating new originator: %pM\n", addr);
> @@ -320,8 +315,6 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
>  	INIT_HLIST_HEAD(&orig_node->neigh_list);
>  	INIT_LIST_HEAD(&orig_node->bond_list);
>  	INIT_LIST_HEAD(&orig_node->vlan_list);
> -	spin_lock_init(&orig_node->ogm_cnt_lock);
> -	spin_lock_init(&orig_node->bcast_seqno_lock);
>  	spin_lock_init(&orig_node->neigh_list_lock);
>  	spin_lock_init(&orig_node->tt_buff_lock);
>  	spin_lock_init(&orig_node->vlan_list_lock);
> @@ -339,11 +332,6 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
>  	atomic_set(&orig_node->last_ttvn, 0);
>  	orig_node->tt_buff = NULL;
>  	orig_node->tt_buff_len = 0;
> -	reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
> -	orig_node->bcast_seqno_reset = reset_time;
> -	orig_node->batman_seqno_reset = reset_time;
> -
> -	atomic_set(&orig_node->bond_candidates, 0);
>  
>  	/* create a vlan object for the "untagged" LAN */
>  	vlan = batadv_orig_node_vlan_new(orig_node, BATADV_NO_FLAGS);
> @@ -352,14 +340,7 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
>  	/* there is nothing else to do with the vlan.. */
>  	batadv_orig_node_vlan_free_ref(vlan);
>  
> -	size = bat_priv->num_ifaces * sizeof(unsigned long) * BATADV_NUM_WORDS;
> -
> -	orig_node->bcast_own = kzalloc(size, GFP_ATOMIC);
> -	if (!orig_node->bcast_own)
> -		goto free_vlan;
> -
> -	size = bat_priv->num_ifaces * sizeof(uint8_t);
> -	orig_node->bcast_own_sum = kzalloc(size, GFP_ATOMIC);
> +	atomic_set(&orig_node->bond_candidates, 0);
>  
>  	for (i = 0; i < BATADV_FRAG_BUFFER_COUNT; i++) {
>  		INIT_HLIST_HEAD(&orig_node->fragments[i].head);
> @@ -367,22 +348,20 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
>  		orig_node->fragments[i].size = 0;
>  	}
>  
> -	if (!orig_node->bcast_own_sum)
> -		goto free_bcast_own;
> +	spin_lock_init(&orig_node->bcast_seqno_lock);
> +	reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
> +	orig_node->bcast_seqno_reset = reset_time;
> +	orig_node->batman_seqno_reset = reset_time;

Why are you moving this stuff around in the function? I don't see the purpose ... reset_time
or bcast_seqno_lock are just moved, but not changed.
>  
>  	hash_added = batadv_hash_add(bat_priv->orig_hash, batadv_compare_orig,
>  				     batadv_choose_orig, orig_node,
>  				     &orig_node->hash_entry);
> -	if (hash_added != 0)
> -		goto free_bcast_own_sum;
> +	if (hash_added != 0) {
> +		kfree(orig_node);
> +		return NULL;
> +	}
>  
>  	return orig_node;
> -free_bcast_own_sum:
> -	kfree(orig_node->bcast_own_sum);
> -free_bcast_own:
> -	kfree(orig_node->bcast_own);
> -free_vlan:
> -	batadv_orig_node_vlan_free_ref(vlan);
>  free_orig_node:
>  	kfree(orig_node);
>  	return NULL;
> diff --git a/types.h b/types.h
> index c7d5fa1..fde4d68 100644
> --- a/types.h
> +++ b/types.h
> @@ -133,18 +133,29 @@ struct batadv_orig_node_vlan {
>  };
>  
>  /**
> + * struct batadv_orig_bat_iv - B.A.T.M.A.N. IV private orig_node members
> + * @bcast_own: bitfield containing the number of our OGMs this orig_node
> + *  rebroadcasted "back" to us (relative to last_real_seqno)
> + * @bcast_own_sum: counted result of bcast_own
> + * @ogm_cnt_lock: lock protecting bcast_own, bcast_own_sum,
> + *  neigh_node->real_bits & neigh_node->real_packet_count
> + */
> +struct batadv_orig_bat_iv {
> +	unsigned long *bcast_own;
> +	uint8_t *bcast_own_sum;
> +	spinlock_t ogm_cnt_lock;
> +};
> +
> +/**
>   * struct batadv_orig_node - structure for orig_list maintaining nodes of mesh
>   * @orig: originator ethernet address
>   * @primary_addr: hosts primary interface address
>   * @router: router that should be used to reach this originator
> - * @batadv_dat_addr_t:  address of the orig node in the distributed hash
> - * @bcast_own: bitfield containing the number of our OGMs this orig_node
> - *  rebroadcasted "back" to us (relative to last_real_seqno)
> - * @bcast_own_sum: counted result of bcast_own
>   * @last_seen: time when last packet from this node was received
> - * @bcast_seqno_reset: time when the broadcast seqno window was reset
> - * @batman_seqno_reset: time when the batman seqno window was reset
> + * @last_ttl: ttl of last received packet
> + * @batadv_dat_addr_t:  address of the orig node in the distributed hash
>   * @capabilities: announced capabilities of this originator
> + * @last_real_seqno: last and best known sequence number
>   * @last_ttvn: last seen translation table version number
>   * @tt_buff: last tt changeset this node received from the orig node
>   * @tt_buff_len: length of the last tt changeset this node received from the
> @@ -152,19 +163,17 @@ struct batadv_orig_node_vlan {
>   * @tt_buff_lock: lock that protects tt_buff and tt_buff_len
>   * @tt_initialised: bool keeping track of whether or not this node have received
>   *  any translation table information from the orig node yet
> - * @last_real_seqno: last and best known sequence number
> - * @last_ttl: ttl of last received packet
>   * @bcast_bits: bitfield containing the info which payload broadcast originated
>   *  from this orig node this host already has seen (relative to
>   *  last_bcast_seqno)
>   * @last_bcast_seqno: last broadcast sequence number received by this host
> + * @bcast_seqno_lock: lock protecting bcast_bits & last_bcast_seqno
> + * @bcast_seqno_reset: time when the broadcast seqno window was reset
> + * @batman_seqno_reset: time when the batman seqno window was reset
>   * @neigh_list: list of potential next hop neighbor towards this orig node
>   * @neigh_list_lock: lock protecting neigh_list, router and bonding_list
>   * @hash_entry: hlist node for batadv_priv::orig_hash
>   * @bat_priv: pointer to soft_iface this orig node belongs to
> - * @ogm_cnt_lock: lock protecting bcast_own, bcast_own_sum,
> - *  neigh_node->real_bits & neigh_node->real_packet_count
> - * @bcast_seqno_lock: lock protecting bcast_bits & last_bcast_seqno
>   * @bond_candidates: how many candidates are available
>   * @bond_list: list of bonding candidates
>   * @refcount: number of contexts the object is used
> @@ -177,29 +186,30 @@ struct batadv_orig_node_vlan {
>   * @vlan_list: a list of orig_node_vlan structs, one per VLAN served by the
>   *  originator represented by this object
>   * @vlan_list_lock: lock protecting vlan_list
> + * @bat_iv: B.A.T.M.A.N. IV private structure
>   */
>  struct batadv_orig_node {
>  	uint8_t orig[ETH_ALEN];
>  	uint8_t primary_addr[ETH_ALEN];
>  	struct batadv_neigh_node __rcu *router; /* rcu protected pointer */
> +	unsigned long last_seen;
> +	uint8_t last_ttl;
>  #ifdef CONFIG_BATMAN_ADV_DAT
>  	batadv_dat_addr_t dat_addr;
>  #endif
> -	unsigned long *bcast_own;
> -	uint8_t *bcast_own_sum;
> -	unsigned long last_seen;
> -	unsigned long bcast_seqno_reset;
> -	unsigned long batman_seqno_reset;
>  	uint8_t capabilities;
> +	uint32_t last_real_seqno;
>  	atomic_t last_ttvn;
>  	unsigned char *tt_buff;
>  	int16_t tt_buff_len;
>  	spinlock_t tt_buff_lock; /* protects tt_buff & tt_buff_len */
>  	bool tt_initialised;
> -	uint32_t last_real_seqno;
> -	uint8_t last_ttl;
>  	DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
>  	uint32_t last_bcast_seqno;
> +	/* bcast_seqno_lock protects: bcast_bits & last_bcast_seqno */
> +	spinlock_t bcast_seqno_lock;
> +	unsigned long bcast_seqno_reset;
> +	unsigned long batman_seqno_reset;
>  	struct hlist_head neigh_list;
>  	/* neigh_list_lock protects: neigh_list, router & bonding_list */
>  	spinlock_t neigh_list_lock;
> @@ -208,9 +218,6 @@ struct batadv_orig_node {
>  	/* ogm_cnt_lock protects: bcast_own, bcast_own_sum,
>  	 * neigh_node->real_bits & neigh_node->real_packet_count
>  	 */
> -	spinlock_t ogm_cnt_lock;
> -	/* bcast_seqno_lock protects: bcast_bits & last_bcast_seqno */
> -	spinlock_t bcast_seqno_lock;
>  	atomic_t bond_candidates;
>  	struct list_head bond_list;
>  	atomic_t refcount;
> @@ -224,6 +231,7 @@ struct batadv_orig_node {
>  	struct batadv_frag_table_entry fragments[BATADV_FRAG_BUFFER_COUNT];
>  	struct list_head vlan_list;
>  	spinlock_t vlan_list_lock; /* protects vlan_list */
> +	struct batadv_orig_bat_iv bat_iv;
>  };

Same here: there are various fields which are just moved around for no obvious
reason (bcast_seqno_lock, batman_seqno_reset, bcast_seqno_reset, last_seen, last_ttl,
...)


>  
>  /**
> -- 
> 1.8.1.5
> 

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

Re: [B.A.T.M.A.N.] [RFC 02/10] batman-adv: make struct batadv_orig_node algorithm agnostic

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 04:09:51PM +0200, Simon Wunderlich wrote:
> > -	if (!orig_node->bcast_own_sum)
> > -		goto free_bcast_own;
> > +	spin_lock_init(&orig_node->bcast_seqno_lock);
> > +	reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
> > +	orig_node->bcast_seqno_reset = reset_time;
> > +	orig_node->batman_seqno_reset = reset_time;
> 
> Why are you moving this stuff around in the function? I don't see the purpose ... reset_time
> or bcast_seqno_lock are just moved, but not changed.

well, if you look some lines above you see the vlan stuff being
allocated....hence this is a rebase error :) while merging this change with tt-vlan I
introduced the error.
[...]

> >  	struct list_head vlan_list;
> >  	spinlock_t vlan_list_lock; /* protects vlan_list */
> > +	struct batadv_orig_bat_iv bat_iv;
> >  };
> 
> Same here: there are various fields which are just moved around for no obvious
> reason (bcast_seqno_lock, batman_seqno_reset, bcast_seqno_reset, last_seen, last_ttl,
> ...)

yeah sorry. as before, probably an issue triggered by a rebase.
Luckily this is just an rfc and not the final patchset :)

Thanks!



-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

[B.A.T.M.A.N.] [RFC 03/10] batman-adv: add bat_orig_print function API

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Each routing protocol has its own metric and private
variables, therefore it is useful to introduce a new API
for originator information printing.

This API needs to be implemented by each protocol in order
to provide its specific originator table output.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 originator.c | 65 ++++++++--------------------------------------------------
 types.h      |  3 +++
 3 files changed, 79 insertions(+), 56 deletions(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 8e8dc5d..87005f7 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -1367,6 +1367,72 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
 	return NET_RX_SUCCESS;
 }
 
+/**
+ * batadv_iv_ogm_orig_print - print the originator table
+ * @bat_priv: the bat priv with all the soft interface information
+ * @seq: debugfs table seq_file struct
+ */
+static void batadv_iv_ogm_orig_print(struct batadv_priv *bat_priv,
+				     struct seq_file *seq)
+{
+	struct batadv_hashtable *hash = bat_priv->orig_hash;
+	struct hlist_head *head;
+	struct batadv_orig_node *orig_node;
+	struct batadv_neigh_node *neigh_node, *neigh_node_tmp;
+	int batman_count = 0;
+	int last_seen_secs;
+	int last_seen_msecs;
+	unsigned long last_seen_jiffies;
+	uint32_t i;
+
+
+	seq_printf(seq, "  %-15s %s (%s/%i) %17s [%10s]: %20s ...\n",
+		   "Originator", "last-seen", "#", BATADV_TQ_MAX_VALUE,
+		   "Nexthop", "outgoingIF", "Potential nexthops");
+
+	for (i = 0; i < hash->size; i++) {
+		head = &hash->table[i];
+
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
+			neigh_node = batadv_orig_node_get_router(orig_node);
+			if (!neigh_node)
+				continue;
+
+			if (neigh_node->bat_iv.tq_avg == 0)
+				goto next;
+
+			last_seen_jiffies = jiffies - orig_node->last_seen;
+			last_seen_msecs = jiffies_to_msecs(last_seen_jiffies);
+			last_seen_secs = last_seen_msecs / 1000;
+			last_seen_msecs = last_seen_msecs % 1000;
+
+			seq_printf(seq, "%pM %4i.%03is   (%3i) %pM [%10s]:",
+				   orig_node->orig, last_seen_secs,
+				   last_seen_msecs, neigh_node->bat_iv.tq_avg,
+				   neigh_node->addr,
+				   neigh_node->if_incoming->net_dev->name);
+
+			hlist_for_each_entry_rcu(neigh_node_tmp,
+						 &orig_node->neigh_list, list) {
+				seq_printf(seq, " %pM (%3i)",
+					   neigh_node_tmp->addr,
+					   neigh_node_tmp->bat_iv.tq_avg);
+			}
+
+			seq_puts(seq, "\n");
+			batman_count++;
+
+next:
+			batadv_neigh_node_free_ref(neigh_node);
+		}
+		rcu_read_unlock();
+	}
+
+	if (batman_count == 0)
+		seq_puts(seq, "No batman nodes in range ...\n");
+}
+
 static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.name = "BATMAN_IV",
 	.bat_iface_enable = batadv_iv_ogm_iface_enable,
@@ -1375,6 +1441,7 @@ static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.bat_primary_iface_set = batadv_iv_ogm_primary_iface_set,
 	.bat_ogm_schedule = batadv_iv_ogm_schedule,
 	.bat_ogm_emit = batadv_iv_ogm_emit,
+	.bat_orig_print = batadv_iv_ogm_orig_print,
 };
 
 int __init batadv_iv_init(void)
diff --git a/originator.c b/originator.c
index 0225ae2..3f05e08 100644
--- a/originator.c
+++ b/originator.c
@@ -503,73 +503,26 @@ int batadv_orig_seq_print_text(struct seq_file *seq, void *offset)
 {
 	struct net_device *net_dev = (struct net_device *)seq->private;
 	struct batadv_priv *bat_priv = netdev_priv(net_dev);
-	struct batadv_hashtable *hash = bat_priv->orig_hash;
-	struct hlist_head *head;
 	struct batadv_hard_iface *primary_if;
-	struct batadv_orig_node *orig_node;
-	struct batadv_neigh_node *neigh_node, *neigh_node_tmp;
-	int batman_count = 0;
-	int last_seen_secs;
-	int last_seen_msecs;
-	unsigned long last_seen_jiffies;
-	uint32_t i;
 
 	primary_if = batadv_seq_print_text_primary_if_get(seq);
 	if (!primary_if)
-		goto out;
+		return 0;
 
-	seq_printf(seq, "[B.A.T.M.A.N. adv %s, MainIF/MAC: %s/%pM (%s)]\n",
+	seq_printf(seq, "[B.A.T.M.A.N. adv %s, MainIF/MAC: %s/%pM (%s %s)]\n",
 		   BATADV_SOURCE_VERSION, primary_if->net_dev->name,
-		   primary_if->net_dev->dev_addr, net_dev->name);
-	seq_printf(seq, "  %-15s %s (%s/%i) %17s [%10s]: %20s ...\n",
-		   "Originator", "last-seen", "#", BATADV_TQ_MAX_VALUE,
-		   "Nexthop", "outgoingIF", "Potential nexthops");
+		   primary_if->net_dev->dev_addr, net_dev->name,
+		   bat_priv->bat_algo_ops->name);
 
-	for (i = 0; i < hash->size; i++) {
-		head = &hash->table[i];
+	batadv_hardif_free_ref(primary_if);
 
-		rcu_read_lock();
-		hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
-			neigh_node = batadv_orig_node_get_router(orig_node);
-			if (!neigh_node)
-				continue;
-
-			if (neigh_node->bat_iv.tq_avg == 0)
-				goto next;
-
-			last_seen_jiffies = jiffies - orig_node->last_seen;
-			last_seen_msecs = jiffies_to_msecs(last_seen_jiffies);
-			last_seen_secs = last_seen_msecs / 1000;
-			last_seen_msecs = last_seen_msecs % 1000;
-
-			seq_printf(seq, "%pM %4i.%03is   (%3i) %pM [%10s]:",
-				   orig_node->orig, last_seen_secs,
-				   last_seen_msecs, neigh_node->bat_iv.tq_avg,
-				   neigh_node->addr,
-				   neigh_node->if_incoming->net_dev->name);
-
-			hlist_for_each_entry_rcu(neigh_node_tmp,
-						 &orig_node->neigh_list, list) {
-				seq_printf(seq, " %pM (%3i)",
-					   neigh_node_tmp->addr,
-					   neigh_node_tmp->bat_iv.tq_avg);
-			}
-
-			seq_puts(seq, "\n");
-			batman_count++;
-
-next:
-			batadv_neigh_node_free_ref(neigh_node);
-		}
-		rcu_read_unlock();
+	if (!bat_priv->bat_algo_ops->bat_orig_print) {
+		seq_puts(seq, "No printing function for this routing protocol\n");
+		return 0;
 	}
 
-	if (batman_count == 0)
-		seq_puts(seq, "No batman nodes in range ...\n");
+	bat_priv->bat_algo_ops->bat_orig_print(bat_priv, seq);
 
-out:
-	if (primary_if)
-		batadv_hardif_free_ref(primary_if);
 	return 0;
 }
 
diff --git a/types.h b/types.h
index fde4d68..d75c77a 100644
--- a/types.h
+++ b/types.h
@@ -982,6 +982,7 @@ struct batadv_forw_packet {
  * @bat_primary_iface_set: called when primary interface is selected / changed
  * @bat_ogm_schedule: prepare a new outgoing OGM for the send queue
  * @bat_ogm_emit: send scheduled OGM
+ * @bat_orig_print: print the originator table
  */
 struct batadv_algo_ops {
 	struct hlist_node list;
@@ -992,6 +993,8 @@ struct batadv_algo_ops {
 	void (*bat_primary_iface_set)(struct batadv_hard_iface *hard_iface);
 	void (*bat_ogm_schedule)(struct batadv_hard_iface *hard_iface);
 	void (*bat_ogm_emit)(struct batadv_forw_packet *forw_packet);
+	/* orig_node handling API */
+	void (*bat_orig_print)(struct batadv_priv *priv, struct seq_file *seq);
 };
 
 /**
-- 
1.8.1.5

[B.A.T.M.A.N.] [RFC 04/10] batman-adv: add bat_get_metric API function

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Each routing algorithm may store its metric value in a
different place, therefore a new API function is needed in
order to ask the algorithm to return the metric of a given
originator

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c | 9 +++++++++
 main.c       | 3 ++-
 types.h      | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 87005f7..abf4cd3 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -1433,6 +1433,14 @@ next:
 		seq_puts(seq, "No batman nodes in range ...\n");
 }
 
+static uint32_t batadv_iv_ogm_metric_get(struct batadv_neigh_node *neigh_node)
+{
+	if (!neigh_node)
+		return 0;
+
+	return neigh_node->bat_iv.tq_avg;
+}
+
 static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.name = "BATMAN_IV",
 	.bat_iface_enable = batadv_iv_ogm_iface_enable,
@@ -1441,6 +1449,7 @@ static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.bat_primary_iface_set = batadv_iv_ogm_primary_iface_set,
 	.bat_ogm_schedule = batadv_iv_ogm_schedule,
 	.bat_ogm_emit = batadv_iv_ogm_emit,
+	.bat_metric_get = batadv_iv_ogm_metric_get,
 	.bat_orig_print = batadv_iv_ogm_orig_print,
 };
 
diff --git a/main.c b/main.c
index e99c66b..e87105b 100644
--- a/main.c
+++ b/main.c
@@ -444,7 +444,8 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops)
 	    !bat_algo_ops->bat_iface_update_mac ||
 	    !bat_algo_ops->bat_primary_iface_set ||
 	    !bat_algo_ops->bat_ogm_schedule ||
-	    !bat_algo_ops->bat_ogm_emit) {
+	    !bat_algo_ops->bat_ogm_emit ||
+	    !bat_algo_ops->bat_metric_get) {
 		pr_info("Routing algo '%s' does not implement required ops\n",
 			bat_algo_ops->name);
 		ret = -EINVAL;
diff --git a/types.h b/types.h
index d75c77a..815d52d 100644
--- a/types.h
+++ b/types.h
@@ -993,6 +993,7 @@ struct batadv_algo_ops {
 	void (*bat_primary_iface_set)(struct batadv_hard_iface *hard_iface);
 	void (*bat_ogm_schedule)(struct batadv_hard_iface *hard_iface);
 	void (*bat_ogm_emit)(struct batadv_forw_packet *forw_packet);
+	uint32_t (*bat_metric_get)(struct batadv_neigh_node *neigh_node);
 	/* orig_node handling API */
 	void (*bat_orig_print)(struct batadv_priv *priv, struct seq_file *seq);
 };
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [RFC 04/10] batman-adv: add bat_get_metric API function

[Simon Wunderlich]

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

On Wed, May 29, 2013 at 12:20:43AM +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> Each routing algorithm may store its metric value in a
> different place, therefore a new API function is needed in
> order to ask the algorithm to return the metric of a given
> originator

This looks like a duplicate of the ogm_metric_get() patch in the same series
with the same number ... 

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

Re: [B.A.T.M.A.N.] [RFC 04/10] batman-adv: add bat_get_metric API function

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 04:17:58PM +0200, Simon Wunderlich wrote:
> On Wed, May 29, 2013 at 12:20:43AM +0200, Antonio Quartulli wrote:
> > From: Antonio Quartulli <antonio@open-mesh.com>
> > 
> > Each routing algorithm may store its metric value in a
> > different place, therefore a new API function is needed in
> > order to ask the algorithm to return the metric of a given
> > originator
> 
> This looks like a duplicate of the ogm_metric_get() patch in the same series
> with the same number ... 

mh, you are right. one patch less :-)
I don't know how git could allow this. And if you are wondering: yes I had a lot
of fun with my rebase.

Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

[B.A.T.M.A.N.] [RFC 04/10] batman-adv: add bat_metric_get API function

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Each routing algorithm may store its metric value in a
different place, therefore a new API function is needed in
order to ask the algorithm to return the metric of a given
originator

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c | 9 +++++++++
 main.c       | 3 ++-
 types.h      | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 87005f7..abf4cd3 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -1433,6 +1433,14 @@ next:
 		seq_puts(seq, "No batman nodes in range ...\n");
 }
 
+static uint32_t batadv_iv_ogm_metric_get(struct batadv_neigh_node *neigh_node)
+{
+	if (!neigh_node)
+		return 0;
+
+	return neigh_node->bat_iv.tq_avg;
+}
+
 static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.name = "BATMAN_IV",
 	.bat_iface_enable = batadv_iv_ogm_iface_enable,
@@ -1441,6 +1449,7 @@ static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.bat_primary_iface_set = batadv_iv_ogm_primary_iface_set,
 	.bat_ogm_schedule = batadv_iv_ogm_schedule,
 	.bat_ogm_emit = batadv_iv_ogm_emit,
+	.bat_metric_get = batadv_iv_ogm_metric_get,
 	.bat_orig_print = batadv_iv_ogm_orig_print,
 };
 
diff --git a/main.c b/main.c
index e99c66b..e87105b 100644
--- a/main.c
+++ b/main.c
@@ -444,7 +444,8 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops)
 	    !bat_algo_ops->bat_iface_update_mac ||
 	    !bat_algo_ops->bat_primary_iface_set ||
 	    !bat_algo_ops->bat_ogm_schedule ||
-	    !bat_algo_ops->bat_ogm_emit) {
+	    !bat_algo_ops->bat_ogm_emit ||
+	    !bat_algo_ops->bat_metric_get) {
 		pr_info("Routing algo '%s' does not implement required ops\n",
 			bat_algo_ops->name);
 		ret = -EINVAL;
diff --git a/types.h b/types.h
index d75c77a..815d52d 100644
--- a/types.h
+++ b/types.h
@@ -993,6 +993,7 @@ struct batadv_algo_ops {
 	void (*bat_primary_iface_set)(struct batadv_hard_iface *hard_iface);
 	void (*bat_ogm_schedule)(struct batadv_hard_iface *hard_iface);
 	void (*bat_ogm_emit)(struct batadv_forw_packet *forw_packet);
+	uint32_t (*bat_metric_get)(struct batadv_neigh_node *neigh_node);
 	/* orig_node handling API */
 	void (*bat_orig_print)(struct batadv_priv *priv, struct seq_file *seq);
 };
-- 
1.8.1.5

[B.A.T.M.A.N.] [RFC 05/10] batman-adv: add bat_metric_is_similar API function

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Each routing protocol has its own metric semantic and
therefore is the protocol itself the only component able to
compare two metrics to check similarity similarity.

This new API allows each routing protocol to implement its
own logic and make the external code protocol agnostic.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c | 7 +++++++
 main.c       | 3 ++-
 main.h       | 6 ++++++
 types.h      | 3 +++
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index abf4cd3..18c9ae8 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -1441,6 +1441,12 @@ static uint32_t batadv_iv_ogm_metric_get(struct batadv_neigh_node *neigh_node)
 	return neigh_node->bat_iv.tq_avg;
 }
 
+static bool batadv_iv_ogm_metric_is_similar(uint32_t metric,
+					    uint32_t new_metric)
+{
+	return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
+}
+
 static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.name = "BATMAN_IV",
 	.bat_iface_enable = batadv_iv_ogm_iface_enable,
@@ -1450,6 +1456,7 @@ static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.bat_ogm_schedule = batadv_iv_ogm_schedule,
 	.bat_ogm_emit = batadv_iv_ogm_emit,
 	.bat_metric_get = batadv_iv_ogm_metric_get,
+	.bat_metric_is_similar = batadv_iv_ogm_metric_is_similar,
 	.bat_orig_print = batadv_iv_ogm_orig_print,
 };
 
diff --git a/main.c b/main.c
index e87105b..d81f6d6 100644
--- a/main.c
+++ b/main.c
@@ -445,7 +445,8 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops)
 	    !bat_algo_ops->bat_primary_iface_set ||
 	    !bat_algo_ops->bat_ogm_schedule ||
 	    !bat_algo_ops->bat_ogm_emit ||
-	    !bat_algo_ops->bat_metric_get) {
+	    !bat_algo_ops->bat_metric_get ||
+	    !bat_algo_ops->bat_metric_is_similar) {
 		pr_info("Routing algo '%s' does not implement required ops\n",
 			bat_algo_ops->name);
 		ret = -EINVAL;
diff --git a/main.h b/main.h
index fe9a952..1d3611f 100644
--- a/main.h
+++ b/main.h
@@ -86,6 +86,12 @@
 /* numbers of originator to contact for any PUT/GET DHT operation */
 #define BATADV_DAT_CANDIDATES_NUM 3
 
+/**
+ * BATADV_TQ_SIMILARITY_THRESHOLD - TQ points that a secondary metric can differ
+ *  at most from the primary one in order to be still considered acceptable
+ */
+#define BATADV_TQ_SIMILARITY_THRESHOLD 50
+
 /* how much worse secondary interfaces may be to be considered as bonding
  * candidates
  */
diff --git a/types.h b/types.h
index 815d52d..a2492a4 100644
--- a/types.h
+++ b/types.h
@@ -982,6 +982,8 @@ struct batadv_forw_packet {
  * @bat_primary_iface_set: called when primary interface is selected / changed
  * @bat_ogm_schedule: prepare a new outgoing OGM for the send queue
  * @bat_ogm_emit: send scheduled OGM
+ * @bat_metric_is_similar: check if new_metric is good enough to be comparable
+ *  with metric
  * @bat_orig_print: print the originator table
  */
 struct batadv_algo_ops {
@@ -994,6 +996,7 @@ struct batadv_algo_ops {
 	void (*bat_ogm_schedule)(struct batadv_hard_iface *hard_iface);
 	void (*bat_ogm_emit)(struct batadv_forw_packet *forw_packet);
 	uint32_t (*bat_metric_get)(struct batadv_neigh_node *neigh_node);
+	bool (*bat_metric_is_similar)(uint32_t metric, uint32_t new_metric);
 	/* orig_node handling API */
 	void (*bat_orig_print)(struct batadv_priv *priv, struct seq_file *seq);
 };
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [RFC 05/10] batman-adv: add bat_metric_is_similar API function

[Simon Wunderlich]

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

On Wed, May 29, 2013 at 12:20:45AM +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> Each routing protocol has its own metric semantic and
> therefore is the protocol itself the only component able to
> compare two metrics to check similarity similarity.
> 
> This new API allows each routing protocol to implement its
> own logic and make the external code protocol agnostic.
> 
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> ---
>  bat_iv_ogm.c | 7 +++++++
>  main.c       | 3 ++-
>  main.h       | 6 ++++++
>  types.h      | 3 +++
>  4 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
> index abf4cd3..18c9ae8 100644
> --- a/bat_iv_ogm.c
> +++ b/bat_iv_ogm.c
> @@ -1441,6 +1441,12 @@ static uint32_t batadv_iv_ogm_metric_get(struct batadv_neigh_node *neigh_node)
>  	return neigh_node->bat_iv.tq_avg;
>  }
>  
> +static bool batadv_iv_ogm_metric_is_similar(uint32_t metric,
> +					    uint32_t new_metric)
> +{
> +	return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);

You might want to use abs(metric - new_metric) here, otherwise is_similar(a, b) output
might differ from is_similar(b, a).
> +}
> +

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

Re: [B.A.T.M.A.N.] [RFC 05/10] batman-adv: add bat_metric_is_similar API function

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 04:16:57PM +0200, Simon Wunderlich wrote:
> On Wed, May 29, 2013 at 12:20:45AM +0200, Antonio Quartulli wrote:
> > From: Antonio Quartulli <antonio@open-mesh.com>
> > 
> > Each routing protocol has its own metric semantic and
> > therefore is the protocol itself the only component able to
> > compare two metrics to check similarity similarity.
> > 
> > This new API allows each routing protocol to implement its
> > own logic and make the external code protocol agnostic.
> > 
> > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> > ---
> >  bat_iv_ogm.c | 7 +++++++
> >  main.c       | 3 ++-
> >  main.h       | 6 ++++++
> >  types.h      | 3 +++
> >  4 files changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
> > index abf4cd3..18c9ae8 100644
> > --- a/bat_iv_ogm.c
> > +++ b/bat_iv_ogm.c
> > @@ -1441,6 +1441,12 @@ static uint32_t batadv_iv_ogm_metric_get(struct batadv_neigh_node *neigh_node)
> >  	return neigh_node->bat_iv.tq_avg;
> >  }
> >  
> > +static bool batadv_iv_ogm_metric_is_similar(uint32_t metric,
> > +					    uint32_t new_metric)
> > +{
> > +	return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
> 
> You might want to use abs(metric - new_metric) here, otherwise is_similar(a, b) output
> might differ from is_similar(b, a).

Mh..imho the name of the function is bad because this has been done on purpose.

The idea is that we want to see if 'b' is at least
as good as 'a', therefore what we want to check if is b is greater than
'a - threshold' only.

Imagine that 'b' is greater than (a + threshold), for me the function has to
return true, because the metric b is at least as good as a, but if I introduce
the abs() the function would return false.

Example:

a=190
b=240
threshold=20

a - b = -50 < 20 => b is at least as good as a!

using abs:

abs(a - b) = 50 < 20 => NO! b is worse than a....and this is not true.


this situation can happen in the code because usually 'a' will represents some
kind of current metric and this is not supposed to be the best ever (maybe we
still have to switch to a new best).

I hope I made it clear.


For me "similar" was something like: "ok, metric b comes from a link that can be
used where we now have the link represented by a".


Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

Re: [B.A.T.M.A.N.] [RFC 05/10] batman-adv: add bat_metric_is_similar API function

[Simon Wunderlich]

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

On Wed, May 29, 2013 at 04:28:51PM +0200, Antonio Quartulli wrote:
> On Wed, May 29, 2013 at 04:16:57PM +0200, Simon Wunderlich wrote:
> > On Wed, May 29, 2013 at 12:20:45AM +0200, Antonio Quartulli wrote:
> > > From: Antonio Quartulli <antonio@open-mesh.com>
> > > 
> > > Each routing protocol has its own metric semantic and
> > > therefore is the protocol itself the only component able to
> > > compare two metrics to check similarity similarity.
> > > 
> > > This new API allows each routing protocol to implement its
> > > own logic and make the external code protocol agnostic.
> > > 
> > > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> > > ---
> > >  bat_iv_ogm.c | 7 +++++++
> > >  main.c       | 3 ++-
> > >  main.h       | 6 ++++++
> > >  types.h      | 3 +++
> > >  4 files changed, 18 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
> > > index abf4cd3..18c9ae8 100644
> > > --- a/bat_iv_ogm.c
> > > +++ b/bat_iv_ogm.c
> > > @@ -1441,6 +1441,12 @@ static uint32_t batadv_iv_ogm_metric_get(struct batadv_neigh_node *neigh_node)
> > >  	return neigh_node->bat_iv.tq_avg;
> > >  }
> > >  
> > > +static bool batadv_iv_ogm_metric_is_similar(uint32_t metric,
> > > +					    uint32_t new_metric)
> > > +{
> > > +	return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
> > 
> > You might want to use abs(metric - new_metric) here, otherwise is_similar(a, b) output
> > might differ from is_similar(b, a).
> 
> Mh..imho the name of the function is bad because this has been done on purpose.

agreed, the function name is not really good. You could rename it to something like
"is_almost_or_better()" if you keep the current semantics, although this is not an
"easy name" either. Or rename it to "similar_or_greater()". Something like that

Although ...
> 
> The idea is that we want to see if 'b' is at least
> as good as 'a', therefore what we want to check if is b is greater than
> 'a - threshold' only.
> 
> Imagine that 'b' is greater than (a + threshold), for me the function has to
> return true, because the metric b is at least as good as a, but if I introduce
> the abs() the function would return false.
> 
> Example:
> 
> a=190
> b=240
> threshold=20
> 
> a - b = -50 < 20 => b is at least as good as a!
> 
> using abs:
> 
> abs(a - b) = 50 < 20 => NO! b is worse than a....and this is not true.
> 
> 
> this situation can happen in the code because usually 'a' will represents some
> kind of current metric and this is not supposed to be the best ever (maybe we
> still have to switch to a new best).

... we actually compare to the biggest value (e.g. highest gateway rank, highest
bonding candidate), so I wonder if this can really happen. So adding abs() would
just make the semantics more clear without breaking the current code when we simply
replace. Although we need to check all occurences again, I'm not completely sure
that it's always compared to the greatest member.

Cheers,
	Simon


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

Re: [B.A.T.M.A.N.] [RFC 05/10] batman-adv: add bat_metric_is_similar API function

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 04:55:19PM +0200, Simon Wunderlich wrote:
> On Wed, May 29, 2013 at 04:28:51PM +0200, Antonio Quartulli wrote:
> > On Wed, May 29, 2013 at 04:16:57PM +0200, Simon Wunderlich wrote:
> > > On Wed, May 29, 2013 at 12:20:45AM +0200, Antonio Quartulli wrote:
> > > > From: Antonio Quartulli <antonio@open-mesh.com>
> > > > 
> > > > Each routing protocol has its own metric semantic and
> > > > therefore is the protocol itself the only component able to
> > > > compare two metrics to check similarity similarity.
> > > > 
> > > > This new API allows each routing protocol to implement its
> > > > own logic and make the external code protocol agnostic.
> > > > 
> > > > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> > > > ---
> > > >  bat_iv_ogm.c | 7 +++++++
> > > >  main.c       | 3 ++-
> > > >  main.h       | 6 ++++++
> > > >  types.h      | 3 +++
> > > >  4 files changed, 18 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
> > > > index abf4cd3..18c9ae8 100644
> > > > --- a/bat_iv_ogm.c
> > > > +++ b/bat_iv_ogm.c
> > > > @@ -1441,6 +1441,12 @@ static uint32_t batadv_iv_ogm_metric_get(struct batadv_neigh_node *neigh_node)
> > > >  	return neigh_node->bat_iv.tq_avg;
> > > >  }
> > > >  
> > > > +static bool batadv_iv_ogm_metric_is_similar(uint32_t metric,
> > > > +					    uint32_t new_metric)
> > > > +{
> > > > +	return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
> > > 
> > > You might want to use abs(metric - new_metric) here, otherwise is_similar(a, b) output
> > > might differ from is_similar(b, a).
> > 
> > Mh..imho the name of the function is bad because this has been done on purpose.
> 
> agreed, the function name is not really good. You could rename it to something like
> "is_almost_or_better()" if you keep the current semantics, although this is not an
> "easy name" either. Or rename it to "similar_or_greater()". Something like that
> 
> Although ...
> > 
> > The idea is that we want to see if 'b' is at least
> > as good as 'a', therefore what we want to check if is b is greater than
> > 'a - threshold' only.
> > 
> > Imagine that 'b' is greater than (a + threshold), for me the function has to
> > return true, because the metric b is at least as good as a, but if I introduce
> > the abs() the function would return false.
> > 
> > Example:
> > 
> > a=190
> > b=240
> > threshold=20
> > 
> > a - b = -50 < 20 => b is at least as good as a!
> > 
> > using abs:
> > 
> > abs(a - b) = 50 < 20 => NO! b is worse than a....and this is not true.
> > 
> > 
> > this situation can happen in the code because usually 'a' will represents some
> > kind of current metric and this is not supposed to be the best ever (maybe we
> > still have to switch to a new best).
> 
> ... we actually compare to the biggest value (e.g. highest gateway rank, highest
> bonding candidate), so I wonder if this can really happen. So adding abs() would
> just make the semantics more clear without breaking the current code when we simply
> replace. Although we need to check all occurences again, I'm not completely sure
> that it's always compared to the greatest member.

well the current code uses the semantic that I implemented in the API (IIRC) and
this is why I've done so: I wanted to keep the very same behaviour.

I'm also not entirely sure that we always compare to the greatest member.

What you are thinking about is the compare() function taking a threshold as
parameter. We can do that with the compare() API if you want, but I think we
should keep this "~is_similar()" API in order to avoid behavioural changes in
the code.


Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

Re: [B.A.T.M.A.N.] [RFC 05/10] batman-adv: add bat_metric_is_similar API function

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 04:57:54PM +0200, Antonio Quartulli wrote:
> > > > > +static bool batadv_iv_ogm_metric_is_similar(uint32_t metric,
> > > > > +					    uint32_t new_metric)
> > > > > +{
> > > > > +	return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
> > > > 
> > > > You might want to use abs(metric - new_metric) here, otherwise is_similar(a, b) output
> > > > might differ from is_similar(b, a).
> > > 
> > > Mh..imho the name of the function is bad because this has been done on purpose.
> > 
> > agreed, the function name is not really good. You could rename it to something like
> > "is_almost_or_better()" if you keep the current semantics, although this is not an
> > "easy name" either. Or rename it to "similar_or_greater()". Something like that
> > 
> > Although ...
> > > 
> > > The idea is that we want to see if 'b' is at least
> > > as good as 'a', therefore what we want to check if is b is greater than
> > > 'a - threshold' only.
> > > 
> > > Imagine that 'b' is greater than (a + threshold), for me the function has to
> > > return true, because the metric b is at least as good as a, but if I introduce
> > > the abs() the function would return false.
> > > 
> > > Example:
> > > 
> > > a=190
> > > b=240
> > > threshold=20
> > > 
> > > a - b = -50 < 20 => b is at least as good as a!
> > > 
> > > using abs:
> > > 
> > > abs(a - b) = 50 < 20 => NO! b is worse than a....and this is not true.
> > > 
> > > 
> > > this situation can happen in the code because usually 'a' will represents some
> > > kind of current metric and this is not supposed to be the best ever (maybe we
> > > still have to switch to a new best).
> > 
> > ... we actually compare to the biggest value (e.g. highest gateway rank, highest
> > bonding candidate), so I wonder if this can really happen. So adding abs() would
> > just make the semantics more clear without breaking the current code when we simply
> > replace. Although we need to check all occurences again, I'm not completely sure
> > that it's always compared to the greatest member.
> 
> well the current code uses the semantic that I implemented in the API (IIRC) and
> this is why I've done so: I wanted to keep the very same behaviour.
> 
> I'm also not entirely sure that we always compare to the greatest member.
> 
> What you are thinking about is the compare() function taking a threshold as
> parameter. We can do that with the compare() API if you want, but I think we
> should keep this "~is_similar()" API in order to avoid behavioural changes in
> the code.
> 


so I'll go for "bat_metric_is_alike_or_better"...we couldn't find a better
name..but if you have one :) let me know


Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

[B.A.T.M.A.N.] [RFC 06/10] batman-adv: add bat_metric_compare API function

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Add an API function to allow the routing protocol to
implement its own metric sorting logic.

In this way each routing protocol can play with its metric
semantic without exposing any detail to the rest of the
code.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c | 14 ++++++++++++++
 main.c       |  3 ++-
 types.h      |  2 ++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 18c9ae8..97aa2fe 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -1447,6 +1447,19 @@ static bool batadv_iv_ogm_metric_is_similar(uint32_t metric,
 	return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
 }
 
+/**
+ * batadv_iv_ogm_metric_compare - implement the bat_metric_compare API
+ * @metric1: the first metric to compare
+ * @metric2: the second metric to compare
+ *
+ * Return a value smaller, equal or greater than zero if metric1 is respectively
+ * smaller, equal or greater than metric2
+ */
+static int batadv_iv_ogm_metric_compare(uint32_t metric1, uint32_t metric2)
+{
+	return metric1 - metric2;
+}
+
 static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.name = "BATMAN_IV",
 	.bat_iface_enable = batadv_iv_ogm_iface_enable,
@@ -1457,6 +1470,7 @@ static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.bat_ogm_emit = batadv_iv_ogm_emit,
 	.bat_metric_get = batadv_iv_ogm_metric_get,
 	.bat_metric_is_similar = batadv_iv_ogm_metric_is_similar,
+	.bat_metric_compare = batadv_iv_ogm_metric_compare,
 	.bat_orig_print = batadv_iv_ogm_orig_print,
 };
 
diff --git a/main.c b/main.c
index d81f6d6..a5de41e 100644
--- a/main.c
+++ b/main.c
@@ -446,7 +446,8 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops)
 	    !bat_algo_ops->bat_ogm_schedule ||
 	    !bat_algo_ops->bat_ogm_emit ||
 	    !bat_algo_ops->bat_metric_get ||
-	    !bat_algo_ops->bat_metric_is_similar) {
+	    !bat_algo_ops->bat_metric_is_similar ||
+	    !bat_algo_ops->bat_metric_compare) {
 		pr_info("Routing algo '%s' does not implement required ops\n",
 			bat_algo_ops->name);
 		ret = -EINVAL;
diff --git a/types.h b/types.h
index a2492a4..e2b2aaa 100644
--- a/types.h
+++ b/types.h
@@ -984,6 +984,7 @@ struct batadv_forw_packet {
  * @bat_ogm_emit: send scheduled OGM
  * @bat_metric_is_similar: check if new_metric is good enough to be comparable
  *  with metric
+ * @bat_metric_compare: compare two metric values
  * @bat_orig_print: print the originator table
  */
 struct batadv_algo_ops {
@@ -997,6 +998,7 @@ struct batadv_algo_ops {
 	void (*bat_ogm_emit)(struct batadv_forw_packet *forw_packet);
 	uint32_t (*bat_metric_get)(struct batadv_neigh_node *neigh_node);
 	bool (*bat_metric_is_similar)(uint32_t metric, uint32_t new_metric);
+	int (*bat_metric_compare)(uint32_t metric, uint32_t new_metric);
 	/* orig_node handling API */
 	void (*bat_orig_print)(struct batadv_priv *priv, struct seq_file *seq);
 };
-- 
1.8.1.5

[B.A.T.M.A.N.] [RFC 07/10] batman-adv: adapt bonding to use the new API functions

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c |  2 +-
 routing.c    | 24 +++++++++++++++++-------
 routing.h    |  3 ++-
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 97aa2fe..a061e8a 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -815,7 +815,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
 		neigh_node->last_ttl = batadv_ogm_packet->header.ttl;
 	}
 
-	batadv_bonding_candidate_add(orig_node, neigh_node);
+	batadv_bonding_candidate_add(bat_priv, orig_node, neigh_node);
 
 	/* if this neighbor already is our next hop there is nothing
 	 * to change
diff --git a/routing.c b/routing.c
index 46de5c7..d718fb9 100644
--- a/routing.c
+++ b/routing.c
@@ -115,11 +115,14 @@ out:
 	return;
 }
 
-void batadv_bonding_candidate_add(struct batadv_orig_node *orig_node,
+void batadv_bonding_candidate_add(struct batadv_priv *bat_priv,
+				  struct batadv_orig_node *orig_node,
 				  struct batadv_neigh_node *neigh_node)
 {
+	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
 	struct batadv_neigh_node *tmp_neigh_node, *router = NULL;
 	uint8_t interference_candidate = 0;
+	uint32_t router_metric, neigh_metric;
 
 	spin_lock_bh(&orig_node->neigh_list_lock);
 
@@ -133,8 +136,9 @@ void batadv_bonding_candidate_add(struct batadv_orig_node *orig_node,
 		goto candidate_del;
 
 	/* ... and is good enough to be considered */
-	if (neigh_node->bat_iv.tq_avg <
-	    router->bat_iv.tq_avg - BATADV_BONDING_TQ_THRESHOLD)
+	router_metric = bao->bat_metric_get(router);
+	neigh_metric = bao->bat_metric_get(neigh_node);
+	if (bao->bat_metric_is_similar(neigh_metric, router_metric))
 		goto candidate_del;
 
 	/* check if we have another candidate with the same mac address or
@@ -486,11 +490,14 @@ out:
  * Increases the returned router's refcount
  */
 static struct batadv_neigh_node *
-batadv_find_ifalter_router(struct batadv_orig_node *primary_orig,
+batadv_find_ifalter_router(struct batadv_priv *bat_priv,
+			   struct batadv_orig_node *primary_orig,
 			   const struct batadv_hard_iface *recv_if)
 {
-	struct batadv_neigh_node *tmp_neigh_node;
 	struct batadv_neigh_node *router = NULL, *first_candidate = NULL;
+	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
+	struct batadv_neigh_node *tmp_neigh_node;
+	uint32_t tmp_metric, router_metric;
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(tmp_neigh_node, &primary_orig->bond_list,
@@ -502,8 +509,9 @@ batadv_find_ifalter_router(struct batadv_orig_node *primary_orig,
 		if (tmp_neigh_node->if_incoming == recv_if)
 			continue;
 
+		tmp_metric = bao->bat_metric_get(tmp_neigh_node);
 		if (router &&
-		    tmp_neigh_node->bat_iv.tq_avg <= router->bat_iv.tq_avg)
+		    bao->bat_metric_compare(tmp_metric, router_metric) <= 0)
 			continue;
 
 		if (!atomic_inc_not_zero(&tmp_neigh_node->refcount))
@@ -515,6 +523,7 @@ batadv_find_ifalter_router(struct batadv_orig_node *primary_orig,
 
 		/* we found a better router (or at least one valid router) */
 		router = tmp_neigh_node;
+		router_metric = bao->bat_metric_get(router);
 	}
 
 	/* use the first candidate if nothing was found. */
@@ -637,7 +646,8 @@ batadv_find_router(struct batadv_priv *bat_priv,
 	if (bonding_enabled)
 		router = batadv_find_bond_router(primary_orig_node, recv_if);
 	else
-		router = batadv_find_ifalter_router(primary_orig_node, recv_if);
+		router = batadv_find_ifalter_router(bat_priv, primary_orig_node,
+						    recv_if);
 
 return_router:
 	if (router && router->if_incoming->if_status != BATADV_IF_ACTIVE)
diff --git a/routing.h b/routing.h
index 55d637a..19544dd 100644
--- a/routing.h
+++ b/routing.h
@@ -48,7 +48,8 @@ batadv_find_router(struct batadv_priv *bat_priv,
 		   const struct batadv_hard_iface *recv_if);
 void batadv_bonding_candidate_del(struct batadv_orig_node *orig_node,
 				  struct batadv_neigh_node *neigh_node);
-void batadv_bonding_candidate_add(struct batadv_orig_node *orig_node,
+void batadv_bonding_candidate_add(struct batadv_priv *bat_priv,
+				  struct batadv_orig_node *orig_node,
 				  struct batadv_neigh_node *neigh_node);
 void batadv_bonding_save_primary(const struct batadv_orig_node *orig_node,
 				 struct batadv_orig_node *orig_neigh_node,
-- 
1.8.1.5

[B.A.T.M.A.N.] [RFC 08/10] batman-adv: adapt the gateway feature to use the new API functions

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 gateway_client.c | 74 +++++++++++++++++++++++++++++++-------------------------
 main.h           |  1 +
 2 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/gateway_client.c b/gateway_client.c
index 69488b2..0a9f1d0 100644
--- a/gateway_client.c
+++ b/gateway_client.c
@@ -113,13 +113,13 @@ void batadv_gw_deselect(struct batadv_priv *bat_priv)
 static struct batadv_gw_node *
 batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
 {
-	struct batadv_neigh_node *router;
+	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
 	struct batadv_gw_node *gw_node, *curr_gw = NULL;
 	uint32_t max_gw_factor = 0, tmp_gw_factor = 0;
-	uint32_t gw_divisor;
-	uint8_t max_tq = 0;
-	uint8_t tq_avg;
 	struct batadv_orig_node *orig_node;
+	struct batadv_neigh_node *router;
+	uint32_t metric, max_metric = 0;
+	uint32_t gw_divisor;
 
 	gw_divisor = BATADV_TQ_LOCAL_WINDOW_SIZE * BATADV_TQ_LOCAL_WINDOW_SIZE;
 	gw_divisor *= 64;
@@ -137,18 +137,19 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
 		if (!atomic_inc_not_zero(&gw_node->refcount))
 			goto next;
 
-		tq_avg = router->bat_iv.tq_avg;
+		metric = bao->bat_metric_get(router);
 
 		switch (atomic_read(&bat_priv->gw_sel_class)) {
 		case 1: /* fast connection */
-			tmp_gw_factor = tq_avg * tq_avg;
+			tmp_gw_factor = metric * metric;
 			tmp_gw_factor *= gw_node->bandwidth_down;
 			tmp_gw_factor *= 100 * 100;
 			tmp_gw_factor /= gw_divisor;
 
 			if ((tmp_gw_factor > max_gw_factor) ||
 			    ((tmp_gw_factor == max_gw_factor) &&
-			     (tq_avg > max_tq))) {
+			     (bao->bat_metric_compare(metric,
+						      max_metric) > 0))) {
 				if (curr_gw)
 					batadv_gw_node_free_ref(curr_gw);
 				curr_gw = gw_node;
@@ -163,7 +164,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
 			  *     soon as a better gateway appears which has
 			  *     $routing_class more tq points)
 			  */
-			if (tq_avg > max_tq) {
+			if (bao->bat_metric_compare(metric, max_metric) > 0) {
 				if (curr_gw)
 					batadv_gw_node_free_ref(curr_gw);
 				curr_gw = gw_node;
@@ -172,8 +173,8 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
 			break;
 		}
 
-		if (tq_avg > max_tq)
-			max_tq = tq_avg;
+		if (bao->bat_metric_compare(metric, max_metric) > 0)
+			max_metric = metric;
 
 		if (tmp_gw_factor > max_gw_factor)
 			max_gw_factor = tmp_gw_factor;
@@ -229,22 +230,24 @@ void batadv_gw_election(struct batadv_priv *bat_priv)
 				    NULL);
 	} else if ((!curr_gw) && (next_gw)) {
 		batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
-			   "Adding route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, tq: %i)\n",
+			   "Adding route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, metric: %u)\n",
 			   next_gw->orig_node->orig,
 			   next_gw->bandwidth_down / 10,
 			   next_gw->bandwidth_down % 10,
 			   next_gw->bandwidth_up / 10,
-			   next_gw->bandwidth_up % 10, router->bat_iv.tq_avg);
+			   next_gw->bandwidth_up % 10,
+			   bat_priv->bat_algo_ops->bat_metric_get(router));
 		batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_ADD,
 				    gw_addr);
 	} else {
 		batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
-			   "Changing route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, tq: %i)\n",
+			   "Changing route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, metric: %u)\n",
 			   next_gw->orig_node->orig,
 			   next_gw->bandwidth_down / 10,
 			   next_gw->bandwidth_down % 10,
 			   next_gw->bandwidth_up / 10,
-			   next_gw->bandwidth_up % 10, router->bat_iv.tq_avg);
+			   next_gw->bandwidth_up % 10,
+			   bat_priv->bat_algo_ops->bat_metric_get(router));
 		batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_CHANGE,
 				    gw_addr);
 	}
@@ -263,9 +266,10 @@ out:
 void batadv_gw_check_election(struct batadv_priv *bat_priv,
 			      struct batadv_orig_node *orig_node)
 {
-	struct batadv_orig_node *curr_gw_orig;
 	struct batadv_neigh_node *router_gw = NULL, *router_orig = NULL;
-	uint8_t gw_tq_avg, orig_tq_avg;
+	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
+	struct batadv_orig_node *curr_gw_orig;
+	uint16_t gw_metric, orig_metric;
 
 	curr_gw_orig = batadv_gw_get_selected_orig(bat_priv);
 	if (!curr_gw_orig)
@@ -283,23 +287,25 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv,
 	if (!router_orig)
 		goto out;
 
-	gw_tq_avg = router_gw->bat_iv.tq_avg;
-	orig_tq_avg = router_orig->bat_iv.tq_avg;
+	gw_metric = bao->bat_metric_get(router_gw);
+	orig_metric = bao->bat_metric_get(router_orig);
 
-	/* the TQ value has to be better */
-	if (orig_tq_avg < gw_tq_avg)
+	/* the metric has to be better */
+	if (bao->bat_metric_compare(orig_metric, gw_metric) > 0)
 		goto out;
 
 	/* if the routing class is greater than 3 the value tells us how much
-	 * greater the TQ value of the new gateway must be
+	 * greater the metric of the new gateway must be.
+	 *
+	 * FIXME: This comparison is strictly TQ related.
 	 */
 	if ((atomic_read(&bat_priv->gw_sel_class) > 3) &&
-	    (orig_tq_avg - gw_tq_avg < atomic_read(&bat_priv->gw_sel_class)))
+	    (orig_metric - gw_metric < atomic_read(&bat_priv->gw_sel_class)))
 		goto out;
 
 	batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
-		   "Restarting gateway selection: better gateway found (tq curr: %i, tq new: %i)\n",
-		   gw_tq_avg, orig_tq_avg);
+		   "Restarting gateway selection: better gateway found (metric curr: %u, metric new: %u)\n",
+		   gw_metric, orig_metric);
 
 deselect:
 	batadv_gw_deselect(bat_priv);
@@ -503,11 +509,11 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv,
 
 	curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
 
-	ret = seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n",
+	ret = seq_printf(seq, "%s %pM (%10u) %pM [%10s]: %u.%u/%u.%u MBit\n",
 			 (curr_gw == gw_node ? "=>" : "  "),
 			 gw_node->orig_node->orig,
-			 router->bat_iv.tq_avg, router->addr,
-			 router->if_incoming->net_dev->name,
+			 bat_priv->bat_algo_ops->bat_metric_get(router),
+			 router->addr, router->if_incoming->net_dev->name,
 			 gw_node->bandwidth_down / 10,
 			 gw_node->bandwidth_down % 10,
 			 gw_node->bandwidth_up / 10,
@@ -533,8 +539,8 @@ int batadv_gw_client_seq_print_text(struct seq_file *seq, void *offset)
 		goto out;
 
 	seq_printf(seq,
-		   "      %-12s (%s/%i) %17s [%10s]: advertised uplink bandwidth ... [B.A.T.M.A.N. adv %s, MainIF/MAC: %s/%pM (%s)]\n",
-		   "Gateway", "#", BATADV_TQ_MAX_VALUE, "Nexthop", "outgoingIF",
+		   "      %-12s (%-4s) %17s [%10s]: advertised uplink bandwidth ... [B.A.T.M.A.N. adv %s, MainIF/MAC: %s/%pM (%s)]\n",
+		   "Gateway", "#", "Nexthop", "outgoingIF",
 		   BATADV_SOURCE_VERSION, primary_if->net_dev->name,
 		   primary_if->net_dev->dev_addr, net_dev->name);
 
@@ -707,12 +713,13 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
 bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
 			    struct sk_buff *skb, struct ethhdr *ethhdr)
 {
+	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
 	struct batadv_neigh_node *neigh_curr = NULL, *neigh_old = NULL;
 	struct batadv_orig_node *orig_dst_node = NULL;
 	struct batadv_gw_node *gw_node = NULL, *curr_gw = NULL;
 	bool ret, out_of_range = false;
 	unsigned int header_len = 0;
-	uint8_t curr_tq_avg;
+	uint32_t curr_metric;
 	unsigned short vid;
 
 	vid = batadv_get_vid(skb, 0);
@@ -739,7 +746,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
 		/* If we are a GW then we are our best GW. We can artificially
 		 * set the tq towards ourself as the maximum value
 		 */
-		curr_tq_avg = BATADV_TQ_MAX_VALUE;
+		curr_metric = BATADV_MAX_METRIC;
 		break;
 	case BATADV_GW_MODE_CLIENT:
 		curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
@@ -759,7 +766,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
 		if (!neigh_curr)
 			goto out;
 
-		curr_tq_avg = neigh_curr->bat_iv.tq_avg;
+		curr_metric = bao->bat_metric_get(neigh_curr);
 		break;
 	case BATADV_GW_MODE_OFF:
 	default:
@@ -770,7 +777,8 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
 	if (!neigh_old)
 		goto out;
 
-	if (curr_tq_avg - neigh_old->bat_iv.tq_avg > BATADV_GW_THRESHOLD)
+	if (bao->bat_metric_is_similar(bao->bat_metric_get(neigh_old),
+				       curr_metric))
 		out_of_range = true;
 
 out:
diff --git a/main.h b/main.h
index 1d3611f..8d69641 100644
--- a/main.h
+++ b/main.h
@@ -31,6 +31,7 @@
 
 /* B.A.T.M.A.N. parameters */
 
+#define BATADV_MAX_METRIC 0xffffffff
 #define BATADV_TQ_MAX_VALUE 255
 #define BATADV_JITTER 20
 
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [RFC 08/10] batman-adv: adapt the gateway feature to use the new API functions

[Simon Wunderlich]

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

On Wed, May 29, 2013 at 12:20:48AM +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> ---
>  gateway_client.c | 74 +++++++++++++++++++++++++++++++-------------------------
>  main.h           |  1 +
>  2 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/gateway_client.c b/gateway_client.c
> index 69488b2..0a9f1d0 100644
> --- a/gateway_client.c
> +++ b/gateway_client.c
> @@ -113,13 +113,13 @@ void batadv_gw_deselect(struct batadv_priv *bat_priv)
>  static struct batadv_gw_node *
>  batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
>  {
> -	struct batadv_neigh_node *router;
> +	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
>  	struct batadv_gw_node *gw_node, *curr_gw = NULL;
>  	uint32_t max_gw_factor = 0, tmp_gw_factor = 0;
> -	uint32_t gw_divisor;
> -	uint8_t max_tq = 0;
> -	uint8_t tq_avg;
>  	struct batadv_orig_node *orig_node;
> +	struct batadv_neigh_node *router;
> +	uint32_t metric, max_metric = 0;
> +	uint32_t gw_divisor;
>  
>  	gw_divisor = BATADV_TQ_LOCAL_WINDOW_SIZE * BATADV_TQ_LOCAL_WINDOW_SIZE;
>  	gw_divisor *= 64;
> @@ -137,18 +137,19 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
>  		if (!atomic_inc_not_zero(&gw_node->refcount))
>  			goto next;
>  
> -		tq_avg = router->bat_iv.tq_avg;
> +		metric = bao->bat_metric_get(router);
>  
>  		switch (atomic_read(&bat_priv->gw_sel_class)) {
>  		case 1: /* fast connection */
> -			tmp_gw_factor = tq_avg * tq_avg;
> +			tmp_gw_factor = metric * metric;

Hmm, that is rather strange ... I think fiddling with the metric directly
this way is weird when abstracting. For example:
 1.) Assuming we don't know how the metric looks like, we can't just
     multiplicate them. A logarithmic scaled metric or even arbritrary
     metric would look different compared to the linear metrics as we
     use now.
 2.) This might overflow because metric is u32 and tmp_gw_factor is too.
     It should work for batman IV where the metric is <256, but might
     not for BATMAN V.

I think this "bandwidth magic" should be abstracted as well somehow, if
we want to keep on using it that way.

>  			tmp_gw_factor *= gw_node->bandwidth_down;
>  			tmp_gw_factor *= 100 * 100;
>  			tmp_gw_factor /= gw_divisor;
>  
>  			if ((tmp_gw_factor > max_gw_factor) ||
>  			    ((tmp_gw_factor == max_gw_factor) &&
> -			     (tq_avg > max_tq))) {
> +			     (bao->bat_metric_compare(metric,
> +						      max_metric) > 0))) {
>  				if (curr_gw)
>  					batadv_gw_node_free_ref(curr_gw);
>  				curr_gw = gw_node;
> @@ -163,7 +164,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
>  			  *     soon as a better gateway appears which has
>  			  *     $routing_class more tq points)
>  			  */
> -			if (tq_avg > max_tq) {
> +			if (bao->bat_metric_compare(metric, max_metric) > 0) {
>  				if (curr_gw)
>  					batadv_gw_node_free_ref(curr_gw);
>  				curr_gw = gw_node;
> @@ -172,8 +173,8 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
>  			break;
>  		}
>  
> -		if (tq_avg > max_tq)
> -			max_tq = tq_avg;
> +		if (bao->bat_metric_compare(metric, max_metric) > 0)
> +			max_metric = metric;
>  
>  		if (tmp_gw_factor > max_gw_factor)
>  			max_gw_factor = tmp_gw_factor;
> @@ -229,22 +230,24 @@ void batadv_gw_election(struct batadv_priv *bat_priv)
>  				    NULL);
>  	} else if ((!curr_gw) && (next_gw)) {
>  		batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
> -			   "Adding route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, tq: %i)\n",
> +			   "Adding route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, metric: %u)\n",
>  			   next_gw->orig_node->orig,
>  			   next_gw->bandwidth_down / 10,
>  			   next_gw->bandwidth_down % 10,
>  			   next_gw->bandwidth_up / 10,
> -			   next_gw->bandwidth_up % 10, router->bat_iv.tq_avg);
> +			   next_gw->bandwidth_up % 10,
> +			   bat_priv->bat_algo_ops->bat_metric_get(router));
>  		batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_ADD,
>  				    gw_addr);
>  	} else {
>  		batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
> -			   "Changing route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, tq: %i)\n",
> +			   "Changing route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, metric: %u)\n",
>  			   next_gw->orig_node->orig,
>  			   next_gw->bandwidth_down / 10,
>  			   next_gw->bandwidth_down % 10,
>  			   next_gw->bandwidth_up / 10,
> -			   next_gw->bandwidth_up % 10, router->bat_iv.tq_avg);
> +			   next_gw->bandwidth_up % 10,
> +			   bat_priv->bat_algo_ops->bat_metric_get(router));
>  		batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_CHANGE,
>  				    gw_addr);
>  	}
> @@ -263,9 +266,10 @@ out:
>  void batadv_gw_check_election(struct batadv_priv *bat_priv,
>  			      struct batadv_orig_node *orig_node)
>  {
> -	struct batadv_orig_node *curr_gw_orig;
>  	struct batadv_neigh_node *router_gw = NULL, *router_orig = NULL;
> -	uint8_t gw_tq_avg, orig_tq_avg;
> +	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
> +	struct batadv_orig_node *curr_gw_orig;
> +	uint16_t gw_metric, orig_metric;
>  
>  	curr_gw_orig = batadv_gw_get_selected_orig(bat_priv);
>  	if (!curr_gw_orig)
> @@ -283,23 +287,25 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv,
>  	if (!router_orig)
>  		goto out;
>  
> -	gw_tq_avg = router_gw->bat_iv.tq_avg;
> -	orig_tq_avg = router_orig->bat_iv.tq_avg;
> +	gw_metric = bao->bat_metric_get(router_gw);
> +	orig_metric = bao->bat_metric_get(router_orig);
>  
> -	/* the TQ value has to be better */
> -	if (orig_tq_avg < gw_tq_avg)
> +	/* the metric has to be better */
> +	if (bao->bat_metric_compare(orig_metric, gw_metric) > 0)
>  		goto out;
>  
>  	/* if the routing class is greater than 3 the value tells us how much
> -	 * greater the TQ value of the new gateway must be
> +	 * greater the metric of the new gateway must be.
> +	 *
> +	 * FIXME: This comparison is strictly TQ related.
>  	 */
>  	if ((atomic_read(&bat_priv->gw_sel_class) > 3) &&
> -	    (orig_tq_avg - gw_tq_avg < atomic_read(&bat_priv->gw_sel_class)))
> +	    (orig_metric - gw_metric < atomic_read(&bat_priv->gw_sel_class)))
>  		goto out;
>  
>  	batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
> -		   "Restarting gateway selection: better gateway found (tq curr: %i, tq new: %i)\n",
> -		   gw_tq_avg, orig_tq_avg);
> +		   "Restarting gateway selection: better gateway found (metric curr: %u, metric new: %u)\n",
> +		   gw_metric, orig_metric);
>  
>  deselect:
>  	batadv_gw_deselect(bat_priv);
> @@ -503,11 +509,11 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv,
>  
>  	curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
>  
> -	ret = seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n",
> +	ret = seq_printf(seq, "%s %pM (%10u) %pM [%10s]: %u.%u/%u.%u MBit\n",
>  			 (curr_gw == gw_node ? "=>" : "  "),
>  			 gw_node->orig_node->orig,
> -			 router->bat_iv.tq_avg, router->addr,
> -			 router->if_incoming->net_dev->name,
> +			 bat_priv->bat_algo_ops->bat_metric_get(router),
> +			 router->addr, router->if_incoming->net_dev->name,
>  			 gw_node->bandwidth_down / 10,
>  			 gw_node->bandwidth_down % 10,
>  			 gw_node->bandwidth_up / 10,
> @@ -533,8 +539,8 @@ int batadv_gw_client_seq_print_text(struct seq_file *seq, void *offset)
>  		goto out;
>  
>  	seq_printf(seq,
> -		   "      %-12s (%s/%i) %17s [%10s]: advertised uplink bandwidth ... [B.A.T.M.A.N. adv %s, MainIF/MAC: %s/%pM (%s)]\n",
> -		   "Gateway", "#", BATADV_TQ_MAX_VALUE, "Nexthop", "outgoingIF",
> +		   "      %-12s (%-4s) %17s [%10s]: advertised uplink bandwidth ... [B.A.T.M.A.N. adv %s, MainIF/MAC: %s/%pM (%s)]\n",
> +		   "Gateway", "#", "Nexthop", "outgoingIF",
>  		   BATADV_SOURCE_VERSION, primary_if->net_dev->name,
>  		   primary_if->net_dev->dev_addr, net_dev->name);
>  
> @@ -707,12 +713,13 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len)
>  bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
>  			    struct sk_buff *skb, struct ethhdr *ethhdr)
>  {
> +	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
>  	struct batadv_neigh_node *neigh_curr = NULL, *neigh_old = NULL;
>  	struct batadv_orig_node *orig_dst_node = NULL;
>  	struct batadv_gw_node *gw_node = NULL, *curr_gw = NULL;
>  	bool ret, out_of_range = false;
>  	unsigned int header_len = 0;
> -	uint8_t curr_tq_avg;
> +	uint32_t curr_metric;
>  	unsigned short vid;
>  
>  	vid = batadv_get_vid(skb, 0);
> @@ -739,7 +746,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
>  		/* If we are a GW then we are our best GW. We can artificially
>  		 * set the tq towards ourself as the maximum value
>  		 */
> -		curr_tq_avg = BATADV_TQ_MAX_VALUE;
> +		curr_metric = BATADV_MAX_METRIC;
>  		break;
>  	case BATADV_GW_MODE_CLIENT:
>  		curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
> @@ -759,7 +766,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
>  		if (!neigh_curr)
>  			goto out;
>  
> -		curr_tq_avg = neigh_curr->bat_iv.tq_avg;
> +		curr_metric = bao->bat_metric_get(neigh_curr);
>  		break;
>  	case BATADV_GW_MODE_OFF:
>  	default:
> @@ -770,7 +777,8 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
>  	if (!neigh_old)
>  		goto out;
>  
> -	if (curr_tq_avg - neigh_old->bat_iv.tq_avg > BATADV_GW_THRESHOLD)
> +	if (bao->bat_metric_is_similar(bao->bat_metric_get(neigh_old),
> +				       curr_metric))
>  		out_of_range = true;

Hmm ... if you add the abs for metric_is_similar as suggested for the patch introducing
the function, this one would fail. For BATMAN IV, curr_metric would be 0xffffffff and
the neigh_old would be something < 256, making this function fail for the BATADV_GW_MODE_SERVER
case. 

Actually BATADV_GW_MODE_SERVER could just set out_of_range and goto out, I don't understand
the purpose of this "artificially setting the tq towards ourself" is good for.

>  
>  out:
> diff --git a/main.h b/main.h
> index 1d3611f..8d69641 100644
> --- a/main.h
> +++ b/main.h
> @@ -31,6 +31,7 @@
>  
>  /* B.A.T.M.A.N. parameters */
>  
> +#define BATADV_MAX_METRIC 0xffffffff
>  #define BATADV_TQ_MAX_VALUE 255
>  #define BATADV_JITTER 20
>  
> -- 
> 1.8.1.5
> 

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

Re: [B.A.T.M.A.N.] [RFC 08/10] batman-adv: adapt the gateway feature to use the new API functions

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 04:32:21PM +0200, Simon Wunderlich wrote:
> On Wed, May 29, 2013 at 12:20:48AM +0200, Antonio Quartulli wrote:
> > From: Antonio Quartulli <antonio@open-mesh.com>
> > 
> > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> > ---
> >  gateway_client.c | 74 +++++++++++++++++++++++++++++++-------------------------
> >  main.h           |  1 +
> >  2 files changed, 42 insertions(+), 33 deletions(-)
> > 
> > diff --git a/gateway_client.c b/gateway_client.c
> > index 69488b2..0a9f1d0 100644
> > --- a/gateway_client.c
> > +++ b/gateway_client.c
> > @@ -113,13 +113,13 @@ void batadv_gw_deselect(struct batadv_priv *bat_priv)
> >  static struct batadv_gw_node *
> >  batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
> >  {
> > -	struct batadv_neigh_node *router;
> > +	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
> >  	struct batadv_gw_node *gw_node, *curr_gw = NULL;
> >  	uint32_t max_gw_factor = 0, tmp_gw_factor = 0;
> > -	uint32_t gw_divisor;
> > -	uint8_t max_tq = 0;
> > -	uint8_t tq_avg;
> >  	struct batadv_orig_node *orig_node;
> > +	struct batadv_neigh_node *router;
> > +	uint32_t metric, max_metric = 0;
> > +	uint32_t gw_divisor;
> >  
> >  	gw_divisor = BATADV_TQ_LOCAL_WINDOW_SIZE * BATADV_TQ_LOCAL_WINDOW_SIZE;
> >  	gw_divisor *= 64;
> > @@ -137,18 +137,19 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
> >  		if (!atomic_inc_not_zero(&gw_node->refcount))
> >  			goto next;
> >  
> > -		tq_avg = router->bat_iv.tq_avg;
> > +		metric = bao->bat_metric_get(router);
> >  
> >  		switch (atomic_read(&bat_priv->gw_sel_class)) {
> >  		case 1: /* fast connection */
> > -			tmp_gw_factor = tq_avg * tq_avg;
> > +			tmp_gw_factor = metric * metric;
> 
> Hmm, that is rather strange ... I think fiddling with the metric directly
> this way is weird when abstracting. For example:
>  1.) Assuming we don't know how the metric looks like, we can't just
>      multiplicate them. A logarithmic scaled metric or even arbritrary
>      metric would look different compared to the linear metrics as we
>      use now.
>  2.) This might overflow because metric is u32 and tmp_gw_factor is too.
>      It should work for batman IV where the metric is <256, but might
>      not for BATMAN V.
> 
> I think this "bandwidth magic" should be abstracted as well somehow, if
> we want to keep on using it that way.

I totally agree. Thanks for your feedback, but I don't really know how I could
abstract this behaviour. This is totally GW related, but hardcoded around the
metric semantic. here we would need a routing API that should be implemented by
the GW component....mhhh..suggestions? :D

[...]

> >  
> > -	if (curr_tq_avg - neigh_old->bat_iv.tq_avg > BATADV_GW_THRESHOLD)
> > +	if (bao->bat_metric_is_similar(bao->bat_metric_get(neigh_old),
> > +				       curr_metric))
> >  		out_of_range = true;
> 
> Hmm ... if you add the abs for metric_is_similar as suggested for the patch introducing
> the function, this one would fail. For BATMAN IV, curr_metric would be 0xffffffff and
> the neigh_old would be something < 256, making this function fail for the BATADV_GW_MODE_SERVER
> case. 

mh I see. the problem is in the max value being wrong for the TQ metric....mh we
have to think about that.

> 
> Actually BATADV_GW_MODE_SERVER could just set out_of_range and goto out, I don't understand
> the purpose of this "artificially setting the tq towards ourself" is good for.

This behaviour has been introduced to avoid killing client connections while
they are roaming from node to node. As I explained on IRC this is the wanted
behaviour.

Cheers,



-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

Re: [B.A.T.M.A.N.] [RFC 08/10] batman-adv: adapt the gateway feature to use the new API functions

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 04:48:17PM +0200, Antonio Quartulli wrote:
> On Wed, May 29, 2013 at 04:32:21PM +0200, Simon Wunderlich wrote:
> > On Wed, May 29, 2013 at 12:20:48AM +0200, Antonio Quartulli wrote:

[...]

> >  
> > >  	gw_divisor = BATADV_TQ_LOCAL_WINDOW_SIZE * BATADV_TQ_LOCAL_WINDOW_SIZE;
> > >  	gw_divisor *= 64;
> > > @@ -137,18 +137,19 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)
> > >  		if (!atomic_inc_not_zero(&gw_node->refcount))
> > >  			goto next;
> > >  
> > > -		tq_avg = router->bat_iv.tq_avg;
> > > +		metric = bao->bat_metric_get(router);
> > >  
> > >  		switch (atomic_read(&bat_priv->gw_sel_class)) {
> > >  		case 1: /* fast connection */
> > > -			tmp_gw_factor = tq_avg * tq_avg;
> > > +			tmp_gw_factor = metric * metric;
> > 
> > Hmm, that is rather strange ... I think fiddling with the metric directly
> > this way is weird when abstracting. For example:
> >  1.) Assuming we don't know how the metric looks like, we can't just
> >      multiplicate them. A logarithmic scaled metric or even arbritrary
> >      metric would look different compared to the linear metrics as we
> >      use now.
> >  2.) This might overflow because metric is u32 and tmp_gw_factor is too.
> >      It should work for batman IV where the metric is <256, but might
> >      not for BATMAN V.
> > 
> > I think this "bandwidth magic" should be abstracted as well somehow, if
> > we want to keep on using it that way.
> 
> I totally agree. Thanks for your feedback, but I don't really know how I could
> abstract this behaviour. This is totally GW related, but hardcoded around the
> metric semantic. here we would need a routing API that should be implemented by
> the GW component....mhhh..suggestions? :D
> 

After discussing with Marek on IRC, we agreed that it is probably better to
leave this part of the code untouched.

It has to be rearranged when implementing the new GW logic discussed at WBMv6
and therefore does not really need to be made protocol agnostic now.



Cheers,



-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

[B.A.T.M.A.N.] [RFC 09/10] batman-adv: adapt the neighbor purging routine to use the new API functions

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 originator.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/originator.c b/originator.c
index 3f05e08..f7fc006 100644
--- a/originator.c
+++ b/originator.c
@@ -377,6 +377,8 @@ batadv_purge_orig_neighbors(struct batadv_priv *bat_priv,
 	bool neigh_purged = false;
 	unsigned long last_seen;
 	struct batadv_hard_iface *if_incoming;
+	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
+	uint32_t neigh_metric, best_metric;
 
 	*best_neigh_node = NULL;
 
@@ -411,10 +413,13 @@ batadv_purge_orig_neighbors(struct batadv_priv *bat_priv,
 			batadv_bonding_candidate_del(orig_node, neigh_node);
 			batadv_neigh_node_free_ref(neigh_node);
 		} else {
-			if ((!*best_neigh_node) ||
-			    (neigh_node->bat_iv.tq_avg >
-			     (*best_neigh_node)->bat_iv.tq_avg))
+			neigh_metric = bao->bat_metric_get(neigh_node);
+			if (!*best_neigh_node ||
+			    bao->bat_metric_compare(neigh_metric,
+						    best_metric) > 0) {
 				*best_neigh_node = neigh_node;
+				best_metric = bao->bat_metric_get(neigh_node);
+			}
 		}
 	}
 
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [RFC 00/10] Improving the routing protocol abstraction

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 12:20:39AM +0200, Antonio Quartulli wrote:
> Hello list,

I forgot to say that this patchset is based on top of the TT-VLAN patches.

Cheers²,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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

[B.A.T.M.A.N.] [RFC 10/10] batman-adv: provide orig_node routing API

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Some operations executed on an orig_node depends on the
current routing algorithm being used. To easily make this
mechanism routing algorithm agnostic add a orig_node
specific API that each algorithm can populate with its own
routines.

Such routines are then invoked by the code when needed,
without knowing which routing algorithm is currently in use

With this patch 3 API functions are added:
- orig_free (to free routing depending internal structs)
- orig_add_if (to change the inner state of an orig_node
  when a new hard interface is added)
- orig_del_if (to change the inner state of an orig_node
  when an hard interface is removed)

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 bat_iv_ogm.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 originator.c | 101 ++++++++-------------------------------------------------
 types.h      |   5 +++
 3 files changed, 122 insertions(+), 87 deletions(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index a061e8a..76448de 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -71,6 +71,106 @@ static uint8_t batadv_ring_buffer_avg(const uint8_t lq_recv[])
 	return (uint8_t)(sum / count);
 }
 
+static void batadv_iv_ogm_orig_free(struct batadv_orig_node *orig_node)
+{
+	kfree(orig_node->bat_iv.bcast_own);
+	kfree(orig_node->bat_iv.bcast_own_sum);
+}
+
+static int batadv_iv_ogm_orig_add_if(struct batadv_orig_node *orig_node,
+				     int max_if_num)
+{
+	void *data_ptr;
+	size_t data_size, old_size;
+	int ret = -ENOMEM;
+
+	spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
+
+	data_size = max_if_num * sizeof(unsigned long) * BATADV_NUM_WORDS;
+	old_size = (max_if_num - 1) * sizeof(unsigned long) * BATADV_NUM_WORDS;
+	data_ptr = kmalloc(data_size, GFP_ATOMIC);
+	if (!data_ptr)
+		goto unlock;
+
+	memcpy(data_ptr, orig_node->bat_iv.bcast_own, old_size);
+	kfree(orig_node->bat_iv.bcast_own);
+	orig_node->bat_iv.bcast_own = data_ptr;
+
+	data_ptr = kmalloc(max_if_num * sizeof(uint8_t), GFP_ATOMIC);
+	if (!data_ptr) {
+		kfree(orig_node->bat_iv.bcast_own);
+		goto unlock;
+	}
+
+	memcpy(data_ptr, orig_node->bat_iv.bcast_own_sum,
+	       (max_if_num - 1) * sizeof(uint8_t));
+	kfree(orig_node->bat_iv.bcast_own_sum);
+	orig_node->bat_iv.bcast_own_sum = data_ptr;
+
+	ret = 0;
+
+unlock:
+	spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
+
+	return ret;
+}
+
+static int batadv_iv_ogm_orig_del_if(struct batadv_orig_node *orig_node,
+				     int max_if_num, int del_if_num)
+{
+	void *data_ptr = NULL;
+	int chunk_size,  ret = -ENOMEM;
+
+	spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
+
+	/* last interface was removed */
+	if (max_if_num == 0)
+		goto free_bcast_own;
+
+	chunk_size = sizeof(unsigned long) * BATADV_NUM_WORDS;
+	data_ptr = kmalloc(max_if_num * chunk_size, GFP_ATOMIC);
+	if (!data_ptr)
+		goto unlock;
+
+	/* copy first part */
+	memcpy(data_ptr, orig_node->bat_iv.bcast_own, del_if_num * chunk_size);
+
+	/* copy second part */
+	memcpy((char *)data_ptr + del_if_num * chunk_size,
+	       orig_node->bat_iv.bcast_own + ((del_if_num + 1) * chunk_size),
+	       (max_if_num - del_if_num) * chunk_size);
+
+free_bcast_own:
+	kfree(orig_node->bat_iv.bcast_own);
+	orig_node->bat_iv.bcast_own = data_ptr;
+
+	if (max_if_num == 0)
+		goto free_own_sum;
+
+	data_ptr = kmalloc(max_if_num * sizeof(uint8_t), GFP_ATOMIC);
+	if (!data_ptr) {
+		kfree(orig_node->bat_iv.bcast_own);
+		goto unlock;
+	}
+
+	memcpy(data_ptr, orig_node->bat_iv.bcast_own_sum,
+	       del_if_num * sizeof(uint8_t));
+
+	memcpy((char *)data_ptr + del_if_num * sizeof(uint8_t),
+	       orig_node->bat_iv.bcast_own_sum + ((del_if_num + 1) * sizeof(uint8_t)),
+	       (max_if_num - del_if_num) * sizeof(uint8_t));
+
+free_own_sum:
+	kfree(orig_node->bat_iv.bcast_own_sum);
+	orig_node->bat_iv.bcast_own_sum = data_ptr;
+
+	ret = 0;
+unlock:
+	spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
+
+	return ret;
+}
+
 static struct batadv_orig_node *
 batadv_iv_ogm_orig_get(struct batadv_priv *bat_priv, const uint8_t *addr)
 {
@@ -1472,6 +1572,9 @@ static struct batadv_algo_ops batadv_batman_iv __read_mostly = {
 	.bat_metric_is_similar = batadv_iv_ogm_metric_is_similar,
 	.bat_metric_compare = batadv_iv_ogm_metric_compare,
 	.bat_orig_print = batadv_iv_ogm_orig_print,
+	.bat_orig_free = batadv_iv_ogm_orig_free,
+	.bat_orig_add_if = batadv_iv_ogm_orig_add_if,
+	.bat_orig_del_if = batadv_iv_ogm_orig_del_if,
 };
 
 int __init batadv_iv_init(void)
diff --git a/originator.c b/originator.c
index f7fc006..1bca894 100644
--- a/originator.c
+++ b/originator.c
@@ -234,9 +234,10 @@ static void batadv_orig_node_free_rcu(struct rcu_head *rcu)
 	batadv_tt_global_del_orig(orig_node->bat_priv, orig_node, -1,
 				  "originator timed out");
 
+	if (orig_node->bat_priv->bat_algo_ops->bat_orig_free)
+		orig_node->bat_priv->bat_algo_ops->bat_orig_free(orig_node);
+
 	kfree(orig_node->tt_buff);
-	kfree(orig_node->bat_iv.bcast_own);
-	kfree(orig_node->bat_iv.bcast_own_sum);
 	kfree(orig_node);
 }
 
@@ -531,38 +532,11 @@ int batadv_orig_seq_print_text(struct seq_file *seq, void *offset)
 	return 0;
 }
 
-static int batadv_orig_node_add_if(struct batadv_orig_node *orig_node,
-				   int max_if_num)
-{
-	void *data_ptr;
-	size_t data_size, old_size;
-
-	data_size = max_if_num * sizeof(unsigned long) * BATADV_NUM_WORDS;
-	old_size = (max_if_num - 1) * sizeof(unsigned long) * BATADV_NUM_WORDS;
-	data_ptr = kmalloc(data_size, GFP_ATOMIC);
-	if (!data_ptr)
-		return -ENOMEM;
-
-	memcpy(data_ptr, orig_node->bat_iv.bcast_own, old_size);
-	kfree(orig_node->bat_iv.bcast_own);
-	orig_node->bat_iv.bcast_own = data_ptr;
-
-	data_ptr = kmalloc(max_if_num * sizeof(uint8_t), GFP_ATOMIC);
-	if (!data_ptr)
-		return -ENOMEM;
-
-	memcpy(data_ptr, orig_node->bat_iv.bcast_own_sum,
-	       (max_if_num - 1) * sizeof(uint8_t));
-	kfree(orig_node->bat_iv.bcast_own_sum);
-	orig_node->bat_iv.bcast_own_sum = data_ptr;
-
-	return 0;
-}
-
 int batadv_orig_hash_add_if(struct batadv_hard_iface *hard_iface,
 			    int max_if_num)
 {
 	struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
+	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
 	struct batadv_hashtable *hash = bat_priv->orig_hash;
 	struct hlist_head *head;
 	struct batadv_orig_node *orig_node;
@@ -577,10 +551,10 @@ int batadv_orig_hash_add_if(struct batadv_hard_iface *hard_iface,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
-			spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
-			ret = batadv_orig_node_add_if(orig_node, max_if_num);
-			spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
-
+			ret = 0;
+			if (bao->bat_orig_add_if)
+				ret = bao->bat_orig_add_if(orig_node,
+							   max_if_num);
 			if (ret == -ENOMEM)
 				goto err;
 		}
@@ -594,54 +568,6 @@ err:
 	return -ENOMEM;
 }
 
-static int batadv_orig_node_del_if(struct batadv_orig_node *orig_node,
-				   int max_if_num, int del_if_num)
-{
-	void *data_ptr = NULL;
-	int chunk_size;
-
-	/* last interface was removed */
-	if (max_if_num == 0)
-		goto free_bcast_own;
-
-	chunk_size = sizeof(unsigned long) * BATADV_NUM_WORDS;
-	data_ptr = kmalloc(max_if_num * chunk_size, GFP_ATOMIC);
-	if (!data_ptr)
-		return -ENOMEM;
-
-	/* copy first part */
-	memcpy(data_ptr, orig_node->bat_iv.bcast_own, del_if_num * chunk_size);
-
-	/* copy second part */
-	memcpy((char *)data_ptr + del_if_num * chunk_size,
-	       orig_node->bat_iv.bcast_own + ((del_if_num + 1) * chunk_size),
-	       (max_if_num - del_if_num) * chunk_size);
-
-free_bcast_own:
-	kfree(orig_node->bat_iv.bcast_own);
-	orig_node->bat_iv.bcast_own = data_ptr;
-
-	if (max_if_num == 0)
-		goto free_own_sum;
-
-	data_ptr = kmalloc(max_if_num * sizeof(uint8_t), GFP_ATOMIC);
-	if (!data_ptr)
-		return -ENOMEM;
-
-	memcpy(data_ptr, orig_node->bat_iv.bcast_own_sum,
-	       del_if_num * sizeof(uint8_t));
-
-	memcpy((char *)data_ptr + del_if_num * sizeof(uint8_t),
-	       orig_node->bat_iv.bcast_own_sum + ((del_if_num + 1) * sizeof(uint8_t)),
-	       (max_if_num - del_if_num) * sizeof(uint8_t));
-
-free_own_sum:
-	kfree(orig_node->bat_iv.bcast_own_sum);
-	orig_node->bat_iv.bcast_own_sum = data_ptr;
-
-	return 0;
-}
-
 int batadv_orig_hash_del_if(struct batadv_hard_iface *hard_iface,
 			    int max_if_num)
 {
@@ -650,6 +576,7 @@ int batadv_orig_hash_del_if(struct batadv_hard_iface *hard_iface,
 	struct hlist_head *head;
 	struct batadv_hard_iface *hard_iface_tmp;
 	struct batadv_orig_node *orig_node;
+	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
 	uint32_t i;
 	int ret;
 
@@ -661,11 +588,11 @@ int batadv_orig_hash_del_if(struct batadv_hard_iface *hard_iface,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
-			spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
-			ret = batadv_orig_node_del_if(orig_node, max_if_num,
-						      hard_iface->if_num);
-			spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
-
+			ret = 0;
+			if (bao->bat_orig_del_if)
+				ret = bao->bat_orig_del_if(orig_node,
+							   max_if_num,
+							   hard_iface->if_num);
 			if (ret == -ENOMEM)
 				goto err;
 		}
diff --git a/types.h b/types.h
index e2b2aaa..d042ae7 100644
--- a/types.h
+++ b/types.h
@@ -1001,6 +1001,11 @@ struct batadv_algo_ops {
 	int (*bat_metric_compare)(uint32_t metric, uint32_t new_metric);
 	/* orig_node handling API */
 	void (*bat_orig_print)(struct batadv_priv *priv, struct seq_file *seq);
+	void (*bat_orig_free)(struct batadv_orig_node *orig_node);
+	int (*bat_orig_add_if)(struct batadv_orig_node *orig_node,
+			       int max_if_num);
+	int (*bat_orig_del_if)(struct batadv_orig_node *orig_node,
+			       int max_if_num, int del_if_num);
 };
 
 /**
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [RFC 00/10] Improving the routing protocol abstraction

[Martin Hundebøll]

Hi Antonio,

Great work. We are looking forward to the next generation of Batman 
(shouldn't that be Robin?) :)

On 2013-05-29 00:20, Antonio Quartulli wrote:
> Antonio Quartulli (10):
>    batman-adv: make struct batadv_neigh_node algorithm agnostic
>    batman-adv: make struct batadv_orig_node algorithm agnostic
>    batman-adv: add bat_orig_print function API
>    batman-adv: add bat_metric_get API function
>    batman-adv: add bat_metric_is_similar API function
>    batman-adv: add bat_metric_compare API function
>    batman-adv: adapt bonding to use the new API functions
>    batman-adv: adapt the gateway feature to use the new API functions
>    batman-adv: adapt the neighbor purging routine to use the new API
>      functions
>    batman-adv: provide orig_node routing API

I see that in nc.c you changed the uses of neigh_node->tq_avg to the new 
private algo-specific struct (neigh_node->bat_iv.tq_avg), but there is 
no patch to make nc.c use the new API (bat_algo_ops.bat_metric_get() )...

Is this because you were in doubt wether NC will be usable with bat_v or 
just because you did "make && git-push" ? :)

I am sure NC will need some work in order to support bat_v, and I will 
try to catch up soon...

-- 
Kind Regards
Martin Hundebøll
Frederiks Allé 99, 1.th
8000 Aarhus C
Denmark

+45 61 65 54 61
martin@hundeboll.net

Re: [B.A.T.M.A.N.] [RFC 00/10] Improving the routing protocol abstraction

[Antonio Quartulli]

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

On Wed, May 29, 2013 at 08:20:15AM +0200, Martin Hundebøll wrote:
> Hi Antonio,
> 
> Great work. We are looking forward to the next generation of Batman 
> (shouldn't that be Robin?) :)
> 
> On 2013-05-29 00:20, Antonio Quartulli wrote:
> > Antonio Quartulli (10):
> >    batman-adv: make struct batadv_neigh_node algorithm agnostic
> >    batman-adv: make struct batadv_orig_node algorithm agnostic
> >    batman-adv: add bat_orig_print function API
> >    batman-adv: add bat_metric_get API function
> >    batman-adv: add bat_metric_is_similar API function
> >    batman-adv: add bat_metric_compare API function
> >    batman-adv: adapt bonding to use the new API functions
> >    batman-adv: adapt the gateway feature to use the new API functions
> >    batman-adv: adapt the neighbor purging routine to use the new API
> >      functions
> >    batman-adv: provide orig_node routing API
> 
> I see that in nc.c you changed the uses of neigh_node->tq_avg to the new 
> private algo-specific struct (neigh_node->bat_iv.tq_avg), but there is 
> no patch to make nc.c use the new API (bat_algo_ops.bat_metric_get() )...
> 
> Is this because you were in doubt wether NC will be usable with bat_v or 
> just because you did "make && git-push" ? :)

because I am totally unaware on how this should work in a generic fashion :)
maybe you can try to describe what is needed and then we try together to make
the API comfortable for NC too.

Thanks!

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

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