Hello list,
This is the third version of the "improved routing abstraction" patchset. Thanks for the feedback so far!
This patchset is introducing a new set of routing API functions meant to improve the routing protocol abstraction.
Also the orig_node and the neigh_node structure have been heavily modified in order to split their generic members from those which are metric related.
(the rest of the message is what you already had in the RFC cover letter).
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.
This is the second version of this RFC where I introduced some changes to the names of some functions and some other minor changes discussed in the previous RFC thread.
+ metric related: - bat_metric_get - bat_metric_is_equiv_or_better
+ orig_node related: - bat_orig_print: print the originator table - bat_orig_free - bat_orig_add_if - bat_orig_del_if
Changes from v1: - removed bat_metric_compare() API. This is not really useful since we are assuming all the metrics are in the same domain (uint32_t) so a simple comparison is more than enough.
Changes from v2: - initialise best_metric in the neighbor purging routine - don't optimise ntohs. Will be done in a separate patch
Cheers,
*** BLURB HERE ***
Antonio Quartulli (9): 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_equiv_or_better API function batman-adv: adapt bonding 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 batman-adv: adapt the TT component to use the new API functions
bat_iv_ogm.c | 344 +++++++++++++++++++++++++++++++++++++++++++--------- gateway_client.c | 16 +-- main.c | 4 +- main.h | 6 + network-coding.c | 8 +- originator.c | 231 ++++++++--------------------------- originator.h | 5 +- routing.c | 26 ++-- routing.h | 3 +- translation-table.c | 31 +++-- types.h | 83 ++++++++----- 11 files changed, 454 insertions(+), 303 deletions(-)
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 | 64 +++++++++++++++++++++++++++-------------------------- gateway_client.c | 16 +++++++------- network-coding.c | 8 ++++--- originator.c | 28 ++++++++++++----------- originator.h | 3 ++- routing.c | 9 ++++---- translation-table.c | 4 ++-- types.h | 40 +++++++++++++++++++-------------- 8 files changed, 94 insertions(+), 78 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 97b42d3..a0b11b0 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -93,20 +93,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); - - neigh_node->orig_node = orig_neigh; - neigh_node->if_incoming = hard_iface; + spin_lock_init(&neigh_node->bat_iv.lq_update_lock);
- 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; @@ -755,12 +753,12 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, if (dup_status != BATADV_NO_DUP) 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) { @@ -785,12 +783,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 (dup_status == BATADV_NO_DUP) { orig_node->last_ttl = batadv_ogm_packet->header.ttl; @@ -807,13 +806,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; @@ -892,7 +891,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 % */ @@ -975,6 +974,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) @@ -995,7 +995,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, hlist_for_each_entry_rcu(tmp_neigh_node, &orig_node->neigh_list, list) { neigh_addr = tmp_neigh_node->addr; - is_dup = batadv_test_bit(tmp_neigh_node->real_bits, + is_dup = batadv_test_bit(tmp_neigh_node->bat_iv.real_bits, orig_node->last_real_seqno, seqno);
@@ -1011,13 +1011,13 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
/* 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();
@@ -1041,7 +1041,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; @@ -1192,10 +1192,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 36729dd..7299936 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 */ @@ -256,7 +256,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 { @@ -266,7 +266,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); } @@ -305,8 +305,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) @@ -528,7 +528,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, @@ -792,7 +792,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: @@ -803,7 +803,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 ee1d847..3837195 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; } @@ -436,7 +437,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; } } @@ -557,7 +559,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; @@ -567,7 +569,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);
@@ -575,7 +577,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 ce3b313..5b8f087 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 2b4d9ed..2dc214c 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1299,9 +1299,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 0f938c2..0088a53 100644 --- a/types.h +++ b/types.h @@ -263,40 +263,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; + 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; - spinlock_t lq_update_lock; /* protects tq_recv & tq_index */ + 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; };
/**
On Tuesday, August 13, 2013 14:43:44 Antonio Quartulli wrote:
@@ -1011,13 +1011,13 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
/* 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;
You can't make assumptions about the size of bitmap because it is platform specific which is why the variable declaration is a macro in the first place.
@@ -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)
{
That merits some kernel doc! :)
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; }
All variables should be initialized before we add the new entry to the any list.
@@ -436,7 +437,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;
Do you think David will accept that ?
@@ -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
Same here. I suggest to calculate router->bat_iv.tq_avg - BATADV_BONDING_TQ_THRESHOLD in advance and store it in a variable.
@@ -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);
Looks unrelated to the topic of this patch ?
/**
- 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;
- spinlock_t lq_update_lock; /* protects tq_recv & tq_index */
+};
@refcount ?
+/**
- 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;
- spinlock_t lq_update_lock; /* protects tq_recv & tq_index */
- 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;
};
@priv ??? Copy and paste bug I presume ?
Cheers, Marek
On Fri, Aug 16, 2013 at 11:46:41AM +0800, Marek Lindner wrote:
On Tuesday, August 13, 2013 14:43:44 Antonio Quartulli wrote:
@@ -1011,13 +1011,13 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
/* 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;
You can't make assumptions about the size of bitmap because it is platform specific which is why the variable declaration is a macro in the first place.
where do I make such assumption? Variable bitmap is a just storing the address of the bitmap.
@@ -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)
{
That merits some kernel doc! :)
yap
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; }
All variables should be initialized before we add the new entry to the any list.
object is allocated with kzalloc: everything is zero. I thought we use kzalloc for this purpose: avoid initialisation of every field..no?
@@ -436,7 +437,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;
Do you think David will accept that ?
yes and no :) maybe I should use some more variables..
@@ -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
Same here. I suggest to calculate router->bat_iv.tq_avg - BATADV_BONDING_TQ_THRESHOLD in advance and store it in a variable.
ok
@@ -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);
Looks unrelated to the topic of this patch ?
yeah, simon spotted a couple of these things already. I think they were introduced before and then I failed to reverse all them back.
/**
- 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;
- spinlock_t lq_update_lock; /* protects tq_recv & tq_index */
+};
@refcount ?
:)
+/**
- 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;
- spinlock_t lq_update_lock; /* protects tq_recv & tq_index */
- 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;
};
@priv ??? Copy and paste bug I presume ?
of course!
Cheers, Marek
On Friday, August 16, 2013 14:27:29 Antonio Quartulli wrote:
On Fri, Aug 16, 2013 at 11:46:41AM +0800, Marek Lindner wrote:
On Tuesday, August 13, 2013 14:43:44 Antonio Quartulli wrote:
@@ -1011,13 +1011,13 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
/* 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;
}
You can't make assumptions about the size of bitmap because it is platform specific which is why the variable declaration is a macro in the first place.
where do I make such assumption? Variable bitmap is a just storing the address of the bitmap.
If it is just the address it probably is ok - you are right.
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;
}
All variables should be initialized before we add the new entry to the any list.
object is allocated with kzalloc: everything is zero. I thought we use kzalloc for this purpose: avoid initialisation of every field..no?
What is "INIT_LIST_HEAD(&neigh_node->bonding_list);" ? Looks like an initialization if I am not mistaken. Moreover, batadv_iv_ogm_neigh_new() inits even more after this function returns ...
Cheers, Marek
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 | 93 ++++++++++++++++++++++++++++++++++++++++++------------------ originator.c | 77 ++++++++++++++++++------------------------------- originator.h | 2 +- types.h | 30 +++++++++++++------- 4 files changed, 114 insertions(+), 88 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index a0b11b0..b8114d6 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -87,6 +87,42 @@ 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, @@ -659,20 +695,22 @@ batadv_iv_ogm_slide_own_bcast_window(struct batadv_hard_iface *hard_iface) uint32_t i; size_t word_index; uint8_t *w; + int if_num;
for (i = 0; i < hash->size; i++) { head = &hash->table[i];
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]; + if_num = hard_iface->if_num; + w = &orig_node->bat_iv.bcast_own_sum[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(); } @@ -764,7 +802,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;
@@ -814,16 +852,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; @@ -851,7 +889,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 */ @@ -889,10 +927,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) @@ -976,11 +1015,11 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, 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 BATADV_NO_DUP;
- 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. */ @@ -1029,7 +1068,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
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; } @@ -1125,8 +1164,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;
@@ -1140,15 +1179,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, @@ -1171,7 +1210,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;
@@ -1221,8 +1260,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; diff --git a/originator.c b/originator.c index 3837195..ccdb246 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,7 +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); @@ -356,37 +350,21 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv, */ 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); - for (i = 0; i < BATADV_FRAG_BUFFER_COUNT; i++) { INIT_HLIST_HEAD(&orig_node->fragments[i].head); spin_lock_init(&orig_node->fragments[i].lock); orig_node->fragments[i].size = 0; }
- if (!orig_node->bcast_own_sum) - goto free_bcast_own; - 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; @@ -610,18 +588,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; } @@ -644,9 +622,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; @@ -664,8 +642,8 @@ err: static int batadv_orig_node_del_if(struct batadv_orig_node *orig_node, int max_if_num, int del_if_num) { + int chunk_size, if_offset; void *data_ptr = NULL; - int chunk_size;
/* last interface was removed */ if (max_if_num == 0) @@ -677,16 +655,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; @@ -695,16 +673,17 @@ 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));
+ if_offset = ((del_if_num + 1) * 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 + if_offset, (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; } @@ -728,10 +707,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/types.h b/types.h index 0088a53..dcfc5b0 100644 --- a/types.h +++ b/types.h @@ -133,14 +133,28 @@ 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->bat_iv.real_bits & neigh_node->bat_iv.real_packet_count + */ +struct batadv_orig_bat_iv { + unsigned long *bcast_own; + uint8_t *bcast_own_sum; + /* ogm_cnt_lock protects: bcast_own, bcast_own_sum, + * neigh_node->bat_iv.real_bits & neigh_node->bat_iv.real_packet_count + */ + 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 @@ -166,8 +180,6 @@ struct batadv_orig_node_vlan { * @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 @@ -181,6 +193,7 @@ 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]; @@ -189,8 +202,6 @@ struct batadv_orig_node { #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; @@ -211,10 +222,6 @@ struct batadv_orig_node { spinlock_t neigh_list_lock; struct hlist_node hash_entry; struct batadv_priv *bat_priv; - /* 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; @@ -230,6 +237,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; };
/**
On Tuesday, August 13, 2013 14:43:45 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 | 93 ++++++++++++++++++++++++++++++++++++++++++------------------ originator.c | 77 ++++++++++++++++++------------------------------- originator.h | 2 +- types.h | 30 +++++++++++++------- 4 files changed, 114 insertions(+), 88 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index a0b11b0..b8114d6 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -87,6 +87,42 @@ 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;
+}
Kernel doc ?
@@ -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)
When you are writing the kernel doc don't forget that you changed the behavior of this function.
@@ -356,37 +350,21 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv, */ 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);
for (i = 0; i < BATADV_FRAG_BUFFER_COUNT; i++) { INIT_HLIST_HEAD(&orig_node->fragments[i].head); spin_lock_init(&orig_node->fragments[i].lock); orig_node->fragments[i].size = 0; }
if (!orig_node->bcast_own_sum)
goto free_bcast_own;
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;
- }
Why not go to free_orig_node ?
Cheers, Marek
On Fri, Aug 16, 2013 at 12:01:36PM +0800, Marek Lindner wrote:
On Tuesday, August 13, 2013 14:43:45 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 | 93 ++++++++++++++++++++++++++++++++++++++++++------------------ originator.c | 77 ++++++++++++++++++------------------------------- originator.h | 2 +- types.h | 30 +++++++++++++------- 4 files changed, 114 insertions(+), 88 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index a0b11b0..b8114d6 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -87,6 +87,42 @@ 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;
+}
Kernel doc ?
yap
@@ -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)
When you are writing the kernel doc don't forget that you changed the behavior of this function.
sure
@@ -356,37 +350,21 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv, */ 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);
for (i = 0; i < BATADV_FRAG_BUFFER_COUNT; i++) { INIT_HLIST_HEAD(&orig_node->fragments[i].head); spin_lock_init(&orig_node->fragments[i].lock); orig_node->fragments[i].size = 0; }
if (!orig_node->bcast_own_sum)
goto free_bcast_own;
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;
- }
Why not go to free_orig_node ?
dunno :) will do. thanks
Cheers, Marek
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 | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ originator.c | 66 +++++++++--------------------------------------------------- types.h | 3 +++ 3 files changed, 79 insertions(+), 56 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index b8114d6..61b74ca 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1392,6 +1392,71 @@ 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, @@ -1400,6 +1465,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 ccdb246..b3f6910 100644 --- a/originator.c +++ b/originator.c @@ -506,73 +506,27 @@ 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"); - - 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); - } + primary_if->net_dev->dev_addr, net_dev->name, + bat_priv->bat_algo_ops->name);
- seq_puts(seq, "\n"); - batman_count++; + batadv_hardif_free_ref(primary_if);
-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 dcfc5b0..0141261 100644 --- a/types.h +++ b/types.h @@ -991,6 +991,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; @@ -1001,6 +1002,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); };
/**
On Tuesday, August 13, 2013 14:43:46 Antonio Quartulli wrote:
@@ -991,6 +991,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; @@ -1001,6 +1002,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); };
How about adding the info somewhere that this callback is optional ?
Cheers, Marek
On Fri, Aug 16, 2013 at 12:05:49PM +0800, Marek Lindner wrote:
On Tuesday, August 13, 2013 14:43:46 Antonio Quartulli wrote:
@@ -991,6 +991,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; @@ -1001,6 +1002,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); };
How about adding the info somewhere that this callback is optional ?
Ok, will do it in the kernel doc.
Cheers, Marek
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 61b74ca..6865ccf 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1457,6 +1457,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, @@ -1465,6 +1473,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 1066889..975973f 100644 --- a/main.c +++ b/main.c @@ -502,7 +502,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 0141261..2b5b1c1 100644 --- a/types.h +++ b/types.h @@ -1002,6 +1002,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); };
On Tuesday, August 13, 2013 14:43:47 Antonio Quartulli wrote:
+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;
+}
Kernel doc ?
diff --git a/types.h b/types.h index 0141261..2b5b1c1 100644 --- a/types.h +++ b/types.h @@ -1002,6 +1002,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);
};
Kernel doc ?
Cheers, Marek
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 | 6 ++++++ main.c | 3 ++- main.h | 6 ++++++ types.h | 4 ++++ 4 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 6865ccf..12a9686 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1465,6 +1465,11 @@ 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_eob(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, @@ -1474,6 +1479,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_equiv_or_better = batadv_iv_ogm_metric_is_eob, .bat_orig_print = batadv_iv_ogm_orig_print, };
diff --git a/main.c b/main.c index 975973f..bf6ece1 100644 --- a/main.c +++ b/main.c @@ -503,7 +503,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_equiv_or_better) { 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 e860f1e..ebdfad8 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 2b5b1c1..7a12eef 100644 --- a/types.h +++ b/types.h @@ -991,6 +991,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_equiv_or_better: check if new_metric is good enough to be + * comparable with metric * @bat_orig_print: print the originator table */ struct batadv_algo_ops { @@ -1003,6 +1005,8 @@ 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_equiv_or_better)(uint32_t metric, + uint32_t new_metric); /* orig_node handling API */ void (*bat_orig_print)(struct batadv_priv *priv, struct seq_file *seq); };
On Tuesday, August 13, 2013 14:43:48 Antonio Quartulli wrote:
+static bool batadv_iv_ogm_metric_is_eob(uint32_t metric, uint32_t new_metric) +{
- return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
+}
Kernel doc ?
diff --git a/types.h b/types.h index 2b5b1c1..7a12eef 100644 --- a/types.h +++ b/types.h @@ -991,6 +991,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_equiv_or_better: check if new_metric is good enough to
be + * comparable with metric
- @bat_orig_print: print the originator table
*/
How about this description: Returns true if new_metric is equally good or better than metric.
Actually, you could call the parameters metricA and metricB or something along these lines.
Cheers, Marek
On Mon, Aug 26, 2013 at 09:49:42AM +0800, Marek Lindner wrote:
On Tuesday, August 13, 2013 14:43:48 Antonio Quartulli wrote:
+static bool batadv_iv_ogm_metric_is_eob(uint32_t metric, uint32_t new_metric) +{
- return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
+}
Kernel doc ?
diff --git a/types.h b/types.h index 2b5b1c1..7a12eef 100644 --- a/types.h +++ b/types.h @@ -991,6 +991,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_equiv_or_better: check if new_metric is good enough to
be + * comparable with metric
- @bat_orig_print: print the originator table
*/
How about this description: Returns true if new_metric is equally good or better than metric.
Actually, you could call the parameters metricA and metricB or something along these lines.
ok, thanks!
Cheers, Marek
From: Antonio Quartulli antonio@open-mesh.com
Signed-off-by: Antonio Quartulli antonio@open-mesh.com --- bat_iv_ogm.c | 2 +- routing.c | 25 +++++++++++++++++-------- routing.h | 3 ++- 3 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 12a9686..956d39b 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -834,7 +834,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 5b8f087..5672a2d 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_equiv_or_better(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 = UINT_MAX;
rcu_read_lock(); list_for_each_entry_rcu(tmp_neigh_node, &primary_orig->bond_list, @@ -502,8 +509,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->bat_iv.tq_avg <= router->bat_iv.tq_avg) + tmp_metric = bao->bat_metric_get(tmp_neigh_node); + if (router && (tmp_metric <= router_metric)) continue;
if (!atomic_inc_not_zero(&tmp_neigh_node->refcount)) @@ -515,6 +522,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 +645,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,
On Tuesday, August 13, 2013 14:43:49 Antonio Quartulli wrote:
-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)
{
Kernel doc ?
@@ -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_equiv_or_better(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)
{
Kernel doc ?
- 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 = UINT_MAX;
rcu_read_lock(); list_for_each_entry_rcu(tmp_neigh_node, &primary_orig->bond_list,
@@ -502,8 +509,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->bat_iv.tq_avg <= router->bat_iv.tq_avg)
tmp_metric = bao->bat_metric_get(tmp_neigh_node);
if (router && (tmp_metric <= router_metric)) continue;
This last comparison looks horribly wrong. Isn't that what we have the API for?
Besides, it seems to me the bat_metric_is_equiv_or_better() call has been misdesigned. Instead of passing the metric and having each function store + process the metric this call should accept 2 neighbor nodes as argument. This should save 2 out of 3 API calls (2 * bao->bat_metric_get()) and some code in all the functions needing the bat_metric_is_equiv_or_better() API. Moreover, it should reduce the impact on performance because each callback will cost us a little bit ..
Cheers, Marek
On Mon, Aug 26, 2013 at 11:11:37AM +0800, Marek Lindner wrote:
On Tuesday, August 13, 2013 14:43:49 Antonio Quartulli wrote:
list_for_each_entry_rcu(tmp_neigh_node, &primary_orig->bond_list, @@ -502,8 +509,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->bat_iv.tq_avg <= router->bat_iv.tq_avg)
tmp_metric = bao->bat_metric_get(tmp_neigh_node);
if (router && (tmp_metric <= router_metric)) continue;
This last comparison looks horribly wrong. Isn't that what we have the API for?
we don't have an API for this. In this case we should use the "bat_metric_compare" but we removed that API in this version since it was going to be implemented as a "<=" for all the algorithms we have on the plate now (bat_iv and bat_v). This is the reason why here we use the compare operation directly.
Besides, it seems to me the bat_metric_is_equiv_or_better() call has been misdesigned. Instead of passing the metric and having each function store + process the metric this call should accept 2 neighbor nodes as argument. This should save 2 out of 3 API calls (2 * bao->bat_metric_get()) and some code in all the functions needing the bat_metric_is_equiv_or_better() API. Moreover, it should reduce the impact on performance because each callback will cost us a little bit ..
we could do that, yeah, even if the tests[1] shown that there is not much to gain at the moment. What do you suggest?
Cheers,
[1] http://codepad.org/ZEtQASF3 - test performed using iperf between two clients connected via Ethernet to 2 OM2P-HS.
From: Antonio Quartulli antonio@open-mesh.com
Signed-off-by: Antonio Quartulli antonio@open-mesh.com --- originator.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/originator.c b/originator.c index b3f6910..3fb917b 100644 --- a/originator.c +++ b/originator.c @@ -380,6 +380,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 = UINT_MAX;
*best_neigh_node = NULL;
@@ -414,10 +416,14 @@ 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); + /* store the best_neighbour if this is the first + * iteration or if a better neighbor has been found + */ + if (!*best_neigh_node || (neigh_metric > best_metric)) { *best_neigh_node = neigh_node; + best_metric = bao->bat_metric_get(neigh_node); + } } }
On Tuesday, August 13, 2013 14:43:50 Antonio Quartulli wrote:
--- a/originator.c +++ b/originator.c @@ -380,6 +380,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 = UINT_MAX;
Albeit being not wrong I'd rather initialize best_metric with zero. It was confusing when reading the first time.
Cheers, Marek
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 | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ originator.c | 102 ++++++++------------------------------------------------- types.h | 5 +++ 3 files changed, 123 insertions(+), 88 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 956d39b..2af2017 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -87,6 +87,107 @@ 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) +{ + int chunk_size, ret = -ENOMEM, if_offset; + void *data_ptr = NULL; + + 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)); + + if_offset = ((del_if_num + 1) * sizeof(uint8_t)); + memcpy((char *)data_ptr + del_if_num * sizeof(uint8_t), + orig_node->bat_iv.bcast_own_sum + if_offset, + (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) { @@ -1481,6 +1582,9 @@ static struct batadv_algo_ops batadv_batman_iv __read_mostly = { .bat_metric_get = batadv_iv_ogm_metric_get, .bat_metric_is_equiv_or_better = batadv_iv_ogm_metric_is_eob, .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 3fb917b..78c2282 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); }
@@ -536,38 +537,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; @@ -582,10 +556,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; } @@ -599,55 +573,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) -{ - int chunk_size, if_offset; - void *data_ptr = NULL; - - /* 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)); - - if_offset = ((del_if_num + 1) * sizeof(uint8_t)); - memcpy((char *)data_ptr + del_if_num * sizeof(uint8_t), - orig_node->bat_iv.bcast_own_sum + if_offset, - (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) { @@ -656,6 +581,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;
@@ -667,11 +593,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 7a12eef..6333b31 100644 --- a/types.h +++ b/types.h @@ -1009,6 +1009,11 @@ struct batadv_algo_ops { 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); };
/**
On Tuesday, August 13, 2013 14:43:51 Antonio Quartulli wrote:
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
The whole patch lacks kernel documentation ..
Cheers, Marek
From: Antonio Quartulli antonio@open-mesh.com
Signed-off-by: Antonio Quartulli antonio@open-mesh.com --- translation-table.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/translation-table.c b/translation-table.c index 2dc214c..ff492c8 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1280,18 +1280,21 @@ out: }
/* batadv_transtable_best_orig - Get best originator list entry from tt entry + * @bat_priv: the bat priv with all the soft interface information * @tt_global_entry: global translation table entry to be analyzed * * This functon assumes the caller holds rcu_read_lock(). * Returns best originator list entry or NULL on errors. */ static struct batadv_tt_orig_list_entry * -batadv_transtable_best_orig(struct batadv_tt_global_entry *tt_global_entry) +batadv_transtable_best_orig(struct batadv_priv *bat_priv, + struct batadv_tt_global_entry *tt_global_entry) { - struct batadv_neigh_node *router = NULL; + struct batadv_neigh_node *router; + struct batadv_algo_ops *bao = bat_priv->bat_algo_ops; struct hlist_head *head; struct batadv_tt_orig_list_entry *orig_entry, *best_entry = NULL; - int best_tq = 0; + uint16_t metric, best_metric = 0;
head = &tt_global_entry->orig_list; hlist_for_each_entry_rcu(orig_entry, head, list) { @@ -1299,12 +1302,14 @@ batadv_transtable_best_orig(struct batadv_tt_global_entry *tt_global_entry) if (!router) continue;
- if (router->bat_iv.tq_avg > best_tq) { - best_entry = orig_entry; - best_tq = router->bat_iv.tq_avg; - } - + metric = bao->bat_metric_get(router); batadv_neigh_node_free_ref(router); + + if (metric <= best_metric) + continue; + + best_entry = orig_entry; + best_metric = metric; }
return best_entry; @@ -1312,13 +1317,15 @@ batadv_transtable_best_orig(struct batadv_tt_global_entry *tt_global_entry)
/* batadv_tt_global_print_entry - print all orig nodes who announce the address * for this global entry + * @bat_priv: the bat priv with all the soft interface information * @tt_global_entry: global translation table entry to be printed * @seq: debugfs table seq_file struct * * This functon assumes the caller holds rcu_read_lock(). */ static void -batadv_tt_global_print_entry(struct batadv_tt_global_entry *tt_global_entry, +batadv_tt_global_print_entry(struct batadv_priv *bat_priv, + struct batadv_tt_global_entry *tt_global_entry, struct seq_file *seq) { struct batadv_tt_orig_list_entry *orig_entry, *best_entry; @@ -1331,7 +1338,7 @@ batadv_tt_global_print_entry(struct batadv_tt_global_entry *tt_global_entry, tt_common_entry = &tt_global_entry->common; flags = tt_common_entry->flags;
- best_entry = batadv_transtable_best_orig(tt_global_entry); + best_entry = batadv_transtable_best_orig(bat_priv, tt_global_entry); if (best_entry) { vlan = batadv_orig_node_vlan_get(best_entry->orig_node, tt_common_entry->vid); @@ -1420,7 +1427,7 @@ int batadv_tt_global_seq_print_text(struct seq_file *seq, void *offset) tt_global = container_of(tt_common_entry, struct batadv_tt_global_entry, common); - batadv_tt_global_print_entry(tt_global, seq); + batadv_tt_global_print_entry(bat_priv, tt_global, seq); } rcu_read_unlock(); } @@ -1808,7 +1815,7 @@ struct batadv_orig_node *batadv_transtable_search(struct batadv_priv *bat_priv, goto out;
rcu_read_lock(); - best_entry = batadv_transtable_best_orig(tt_global_entry); + best_entry = batadv_transtable_best_orig(bat_priv, tt_global_entry); /* found anything? */ if (best_entry) orig_node = best_entry->orig_node;
b.a.t.m.a.n@lists.open-mesh.org