This patchset adds the network wide multi interface optimization as proposed in our wiki [1] for BATMAN IV. The main purpose is to do interface alternating and bonding by considering multi interface capabilities globally and not just for the (next) link, otherwise non-optimal links may be chosen.
This patchset needs to change a lot of core data structures and routing, please review it carefully. A local development branch exists on the public git repo [2].
Changes from RFCv1 in May are: * rebase on current master including routing abstraction * use routing abstraction and change some of the API calls for the new multi interface usage * check bonding candidates using the new API * changed wifi penalty to use the double hop penalty, default hop penalty changed to 15 to make no effective change in single station networks * fixed seqno protection window troubles (changed to protection window per interface for OGMs) * various smaller changes, including locking and NULL checking
I've tested the patchset in my VMs to confirm that bonding and alternating works as expected.
As always, any comments are appreciated!
Thanks, Simon
[1] http://www.open-mesh.org/projects/batman-adv/wiki/network-wide-multi-link-op... [2] http://git.open-mesh.org/batman-adv.git/shortlog/refs/heads/simon/network-wi...
Simon Wunderlich (6): batman-adv: remove bonding and interface alternating batman-adv: split tq information in neigh_node struct batman-adv: split out router from orig_node batman-adv: add WiFi penalty batman-adv: consider outgoing interface in OGM sending batman-adv: add bonding again
bat_iv_ogm.c | 732 +++++++++++++++++++++++++++++++---------------- distributed-arp-table.c | 3 +- gateway_client.c | 79 ++++- hard-interface.c | 2 +- hard-interface.h | 1 + icmp_socket.c | 2 +- main.c | 2 +- network-coding.c | 9 +- originator.c | 304 +++++++++++++++++--- originator.h | 14 +- routing.c | 432 +++++++++------------------- routing.h | 12 +- send.c | 13 +- soft-interface.c | 2 +- translation-table.c | 5 +- types.h | 110 ++++--- 16 files changed, 1073 insertions(+), 649 deletions(-)
From: Simon Wunderlich simon@open-mesh.com
Remove bonding and interface alternating code - it will be replaced by a new, network-wide multi interface optimization which enables both bonding and interface alternating in a better way.
Keep the sysfs and find router function though, this will be needed later.
Signed-off-by: Simon Wunderlich simon@open-mesh.com --- bat_iv_ogm.c | 5 - originator.c | 15 +-- routing.c | 300 ++-------------------------------------------------------- routing.h | 9 -- types.h | 10 +- 5 files changed, 12 insertions(+), 327 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index a2b480a..6cd9a0f 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -976,8 +976,6 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, neigh_node->last_ttl = batadv_ogm_packet->header.ttl; }
- batadv_bonding_candidate_add(bat_priv, orig_node, neigh_node); - /* if this neighbor already is our next hop there is nothing * to change */ @@ -1426,9 +1424,6 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, is_bidirect = batadv_iv_ogm_calc_tq(orig_node, orig_neigh_node, batadv_ogm_packet, if_incoming);
- batadv_bonding_save_primary(orig_node, orig_neigh_node, - batadv_ogm_packet); - /* update ranking if it is not a duplicate or has the same * seqno and similar ttl as the non-duplicate */ diff --git a/originator.c b/originator.c index 8ab1434..919fdc0 100644 --- a/originator.c +++ b/originator.c @@ -198,8 +198,6 @@ batadv_neigh_node_new(struct batadv_hard_iface *hard_iface, neigh_node->if_incoming = hard_iface; neigh_node->orig_node = orig_node;
- INIT_LIST_HEAD(&neigh_node->bonding_list); - /* extra reference for return */ atomic_set(&neigh_node->refcount, 2);
@@ -210,20 +208,13 @@ out: static void batadv_orig_node_free_rcu(struct rcu_head *rcu) { struct hlist_node *node_tmp; - struct batadv_neigh_node *neigh_node, *tmp_neigh_node; + struct batadv_neigh_node *neigh_node; struct batadv_orig_node *orig_node;
orig_node = container_of(rcu, struct batadv_orig_node, rcu);
spin_lock_bh(&orig_node->neigh_list_lock);
- /* for all bonding members ... */ - list_for_each_entry_safe(neigh_node, tmp_neigh_node, - &orig_node->bond_list, bonding_list) { - list_del_rcu(&neigh_node->bonding_list); - batadv_neigh_node_free_ref(neigh_node); - } - /* for all neighbors towards this originator ... */ hlist_for_each_entry_safe(neigh_node, node_tmp, &orig_node->neigh_list, list) { @@ -327,7 +318,6 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv, return NULL;
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->bcast_seqno_lock); spin_lock_init(&orig_node->neigh_list_lock); @@ -352,8 +342,6 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv, orig_node->bcast_seqno_reset = reset_time; orig_node->batman_seqno_reset = reset_time;
- atomic_set(&orig_node->bond_candidates, 0); - /* create a vlan object for the "untagged" LAN */ vlan = batadv_orig_node_vlan_new(orig_node, BATADV_NO_FLAGS); if (!vlan) @@ -418,7 +406,6 @@ batadv_purge_orig_neighbors(struct batadv_priv *bat_priv, neigh_purged = true;
hlist_del_rcu(&neigh_node->list); - batadv_bonding_candidate_del(orig_node, neigh_node); batadv_neigh_node_free_ref(neigh_node); } else { /* store the best_neighbour if this is the first diff --git a/routing.c b/routing.c index 71fba14..733ebbf 100644 --- a/routing.c +++ b/routing.c @@ -98,115 +98,6 @@ out: batadv_neigh_node_free_ref(router); }
-/* caller must hold the neigh_list_lock */ -void batadv_bonding_candidate_del(struct batadv_orig_node *orig_node, - struct batadv_neigh_node *neigh_node) -{ - /* this neighbor is not part of our candidate list */ - if (list_empty(&neigh_node->bonding_list)) - goto out; - - list_del_rcu(&neigh_node->bonding_list); - INIT_LIST_HEAD(&neigh_node->bonding_list); - batadv_neigh_node_free_ref(neigh_node); - atomic_dec(&orig_node->bond_candidates); - -out: - return; -} - -/** - * batadv_bonding_candidate_add - consider a new link for bonding mode towards - * the given originator - * @bat_priv: the bat priv with all the soft interface information - * @orig_node: the target node - * @neigh_node: the neighbor representing the new link to consider for bonding - * mode - */ -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; - - spin_lock_bh(&orig_node->neigh_list_lock); - - /* only consider if it has the same primary address ... */ - if (!batadv_compare_eth(orig_node->orig, - neigh_node->orig_node->primary_addr)) - goto candidate_del; - - router = batadv_orig_node_get_router(orig_node); - if (!router) - goto candidate_del; - - - /* ... and is good enough to be considered */ - if (bao->bat_neigh_is_equiv_or_better(neigh_node, router)) - goto candidate_del; - - /* check if we have another candidate with the same mac address or - * interface. If we do, we won't select this candidate because of - * possible interference. - */ - hlist_for_each_entry_rcu(tmp_neigh_node, - &orig_node->neigh_list, list) { - if (tmp_neigh_node == neigh_node) - continue; - - /* we only care if the other candidate is even - * considered as candidate. - */ - if (list_empty(&tmp_neigh_node->bonding_list)) - continue; - - if ((neigh_node->if_incoming == tmp_neigh_node->if_incoming) || - (batadv_compare_eth(neigh_node->addr, - tmp_neigh_node->addr))) { - interference_candidate = 1; - break; - } - } - - /* don't care further if it is an interference candidate */ - if (interference_candidate) - goto candidate_del; - - /* this neighbor already is part of our candidate list */ - if (!list_empty(&neigh_node->bonding_list)) - goto out; - - if (!atomic_inc_not_zero(&neigh_node->refcount)) - goto out; - - list_add_rcu(&neigh_node->bonding_list, &orig_node->bond_list); - atomic_inc(&orig_node->bond_candidates); - goto out; - -candidate_del: - batadv_bonding_candidate_del(orig_node, neigh_node); - -out: - spin_unlock_bh(&orig_node->neigh_list_lock); - - if (router) - batadv_neigh_node_free_ref(router); -} - -/* copy primary address for bonding */ -void -batadv_bonding_save_primary(const struct batadv_orig_node *orig_node, - struct batadv_orig_node *orig_neigh_node, - const struct batadv_ogm_packet *batman_ogm_packet) -{ - if (!(batman_ogm_packet->flags & BATADV_PRIMARIES_FIRST_HOP)) - return; - - memcpy(orig_neigh_node->primary_addr, orig_node->orig, ETH_ALEN); -} - /* checks whether the host restarted and is in the protection time. * returns: * 0 if the packet is to be accepted @@ -435,114 +326,6 @@ out: return ret; }
-/* In the bonding case, send the packets in a round - * robin fashion over the remaining interfaces. - * - * This method rotates the bonding list and increases the - * returned router's refcount. - */ -static struct batadv_neigh_node * -batadv_find_bond_router(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; - - rcu_read_lock(); - list_for_each_entry_rcu(tmp_neigh_node, &primary_orig->bond_list, - bonding_list) { - if (!first_candidate) - first_candidate = tmp_neigh_node; - - /* recv_if == NULL on the first node. */ - if (tmp_neigh_node->if_incoming == recv_if) - continue; - - if (!atomic_inc_not_zero(&tmp_neigh_node->refcount)) - continue; - - router = tmp_neigh_node; - break; - } - - /* use the first candidate if nothing was found. */ - if (!router && first_candidate && - atomic_inc_not_zero(&first_candidate->refcount)) - router = first_candidate; - - if (!router) - goto out; - - /* selected should point to the next element - * after the current router - */ - spin_lock_bh(&primary_orig->neigh_list_lock); - /* this is a list_move(), which unfortunately - * does not exist as rcu version - */ - list_del_rcu(&primary_orig->bond_list); - list_add_rcu(&primary_orig->bond_list, - &router->bonding_list); - spin_unlock_bh(&primary_orig->neigh_list_lock); - -out: - rcu_read_unlock(); - return router; -} - -/** - * batadv_find_ifalter_router - find the best of the remaining candidates which - * are not using this interface - * @bat_priv: the bat priv with all the soft interface information - * @primary_orig: the destination - * @recv_if: the interface that the router returned by this function has to not - * use - * - * Returns the best candidate towards primary_orig that is not using recv_if. - * Increases the returned neighbor's refcount - */ -static struct batadv_neigh_node * -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 *router = NULL, *first_candidate = NULL; - struct batadv_algo_ops *bao = bat_priv->bat_algo_ops; - struct batadv_neigh_node *tmp_neigh_node; - - rcu_read_lock(); - list_for_each_entry_rcu(tmp_neigh_node, &primary_orig->bond_list, - bonding_list) { - if (!first_candidate) - first_candidate = tmp_neigh_node; - - /* recv_if == NULL on the first node. */ - if (tmp_neigh_node->if_incoming == recv_if) - continue; - - if (router && bao->bat_neigh_cmp(tmp_neigh_node, router)) - continue; - - if (!atomic_inc_not_zero(&tmp_neigh_node->refcount)) - continue; - - /* decrement refcount of previously selected router */ - if (router) - batadv_neigh_node_free_ref(router); - - /* we found a better router (or at least one valid router) */ - router = tmp_neigh_node; - } - - /* use the first candidate if nothing was found. */ - if (!router && first_candidate && - atomic_inc_not_zero(&first_candidate->refcount)) - router = first_candidate; - - rcu_read_unlock(); - return router; -} - /** * batadv_check_unicast_packet - Check for malformed unicast packets * @bat_priv: the bat priv with all the soft interface information @@ -580,95 +363,30 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv, return 0; }
-/* find a suitable router for this originator, and use - * bonding if possible. increases the found neighbors - * refcount. +/** + * batadv_find_router - find a suitable router for this originator + * @bat_priv: the bat priv with all the soft interface information + * @orig_node: the destination node + * @recv_if: pointer to interface this packet was received on + * + * Returns the router which should be used for this orig_node on + * this interface, or NULL if not available. */ struct batadv_neigh_node * batadv_find_router(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, const struct batadv_hard_iface *recv_if) { - struct batadv_orig_node *primary_orig_node; - struct batadv_orig_node *router_orig; struct batadv_neigh_node *router; - static uint8_t zero_mac[ETH_ALEN] = {0, 0, 0, 0, 0, 0}; - int bonding_enabled; - uint8_t *primary_addr;
if (!orig_node) return NULL;
router = batadv_orig_node_get_router(orig_node); - if (!router) - goto err;
- /* without bonding, the first node should - * always choose the default router. - */ - bonding_enabled = atomic_read(&bat_priv->bonding); + /* TODO: fill this later with new bonding mechanism */
- rcu_read_lock(); - /* select default router to output */ - router_orig = router->orig_node; - if (!router_orig) - goto err_unlock; - - if ((!recv_if) && (!bonding_enabled)) - goto return_router; - - primary_addr = router_orig->primary_addr; - - /* if we have something in the primary_addr, we can search - * for a potential bonding candidate. - */ - if (batadv_compare_eth(primary_addr, zero_mac)) - goto return_router; - - /* find the orig_node which has the primary interface. might - * even be the same as our router_orig in many cases - */ - if (batadv_compare_eth(primary_addr, router_orig->orig)) { - primary_orig_node = router_orig; - } else { - primary_orig_node = batadv_orig_hash_find(bat_priv, - primary_addr); - if (!primary_orig_node) - goto return_router; - - batadv_orig_node_free_ref(primary_orig_node); - } - - /* with less than 2 candidates, we can't do any - * bonding and prefer the original router. - */ - if (atomic_read(&primary_orig_node->bond_candidates) < 2) - goto return_router; - - /* all nodes between should choose a candidate which - * is is not on the interface where the packet came - * in. - */ - batadv_neigh_node_free_ref(router); - - if (bonding_enabled) - router = batadv_find_bond_router(primary_orig_node, recv_if); - else - router = batadv_find_ifalter_router(bat_priv, primary_orig_node, - recv_if); - -return_router: - if (router && router->if_incoming->if_status != BATADV_IF_ACTIVE) - goto err_unlock; - - rcu_read_unlock(); return router; -err_unlock: - rcu_read_unlock(); -err: - if (router) - batadv_neigh_node_free_ref(router); - return NULL; }
static int batadv_route_unicast_packet(struct sk_buff *skb, diff --git a/routing.h b/routing.h index 19544dd..b8fed80 100644 --- a/routing.h +++ b/routing.h @@ -46,15 +46,6 @@ struct batadv_neigh_node * batadv_find_router(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, 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_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, - const struct batadv_ogm_packet - *batman_ogm_packet); int batadv_window_protected(struct batadv_priv *bat_priv, int32_t seq_num_diff, unsigned long *last_reset);
diff --git a/types.h b/types.h index f323822..ef08333 100644 --- a/types.h +++ b/types.h @@ -177,12 +177,10 @@ struct batadv_orig_bat_iv { * last_bcast_seqno) * @last_bcast_seqno: last broadcast sequence number received by this host * @neigh_list: list of potential next hop neighbor towards this orig node - * @neigh_list_lock: lock protecting neigh_list, router and bonding_list + * @neigh_list_lock: lock protecting neigh_list and router * @hash_entry: hlist node for batadv_priv::orig_hash * @bat_priv: pointer to soft_iface this orig node belongs to * @bcast_seqno_lock: lock protecting bcast_bits & last_bcast_seqno - * @bond_candidates: how many candidates are available - * @bond_list: list of bonding candidates * @refcount: number of contexts the object is used * @rcu: struct used for freeing in an RCU-safe manner * @in_coding_list: list of nodes this orig can hear @@ -218,14 +216,12 @@ struct batadv_orig_node { DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE); uint32_t last_bcast_seqno; struct hlist_head neigh_list; - /* neigh_list_lock protects: neigh_list, router & bonding_list */ + /* neigh_list_lock protects: neigh_list and router */ spinlock_t neigh_list_lock; struct hlist_node hash_entry; struct batadv_priv *bat_priv; /* bcast_seqno_lock protects: bcast_bits & last_bcast_seqno */ spinlock_t bcast_seqno_lock; - atomic_t bond_candidates; - struct list_head bond_list; atomic_t refcount; struct rcu_head rcu; #ifdef CONFIG_BATMAN_ADV_NC @@ -298,7 +294,6 @@ struct batadv_neigh_bat_iv { * @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 * @refcount: number of contexts the object is used * @rcu: struct used for freeing in an RCU-safe manner * @bat_iv: B.A.T.M.A.N. IV private structure @@ -310,7 +305,6 @@ struct batadv_neigh_node { struct batadv_hard_iface *if_incoming; unsigned long last_seen; uint8_t last_ttl; - struct list_head bonding_list; atomic_t refcount; struct rcu_head rcu; struct batadv_neigh_bat_iv bat_iv;
From: Simon Wunderlich simon@open-mesh.com
For the network wide multi interface optimization it is required to save metrics per outgoing interface in one neighbor. Therefore a new type is introduced to keep interface-specific information. This also requires some changes in access and list management.
Signed-off-by: Simon Wunderlich simon@open-mesh.com --- Changes to RFC: * rebase on latest master * change compare API to consider outgoing interface * change EOB API to use ifinfo - this will be required to compare routers for different ougoing interfaces in the bonding patch * fix missing spin_unlock_bh --- bat_iv_ogm.c | 194 ++++++++++++++++++++++++++++++++++++++------------- gateway_client.c | 69 +++++++++++++++--- main.c | 2 +- originator.c | 127 ++++++++++++++++++++++++++++----- originator.h | 3 + translation-table.c | 2 +- types.h | 62 ++++++++++------ 7 files changed, 359 insertions(+), 100 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 6cd9a0f..d46746f 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -275,6 +275,13 @@ batadv_iv_ogm_neigh_new(struct batadv_hard_iface *hard_iface, goto out;
spin_lock_init(&neigh_node->bat_iv.lq_update_lock); + if (!atomic_inc_not_zero(&hard_iface->refcount)) { + kfree(neigh_node); + goto out; + } + + neigh_node->orig_node = orig_neigh; + neigh_node->if_incoming = hard_iface;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Creating new neighbor %pM for orig_node %pM on interface %s\n", @@ -897,22 +904,39 @@ static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface) batadv_hardif_free_ref(primary_if); }
+/** + * batadv_iv_ogm_orig_update - use OGM to update corresponding data in an + * originator + * @bat_priv: the bat priv with all the soft interface information + * @orig_node: the orig node who originally emitted the ogm packet + * @orig_node_ifinfo: ifinfo for the according outgoing interface + * @ethhdr: Ethernet header of the OGM + * @@batadv_ogm_packet: the ogm packet + * @if_incoming: interface where the packet was received + * @if_outgoing: interface for which the retransmission should be considered + * @tt_buff: pointer to the tt buffer + * @dup_status: the duplicate status of this ogm packet. + */ static void batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, const struct ethhdr *ethhdr, const struct batadv_ogm_packet *batadv_ogm_packet, struct batadv_hard_iface *if_incoming, + struct batadv_hard_iface *if_outgoing, const unsigned char *tt_buff, enum batadv_dup_status dup_status) { struct batadv_neigh_node *neigh_node = NULL, *tmp_neigh_node = NULL; struct batadv_neigh_node *router = NULL; struct batadv_orig_node *orig_node_tmp; + struct batadv_neigh_node_ifinfo *neigh_ifinfo = NULL, + *tmp_neigh_ifinfo, + *router_ifinfo; int if_num; uint8_t sum_orig, sum_neigh; uint8_t *neigh_addr; - uint8_t tq_avg; + uint8_t tq_avg, *tq_recv, *tq_index;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "update_originator(): Searching and updating originator entry of received packet\n"); @@ -934,10 +958,19 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, continue;
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; + /* only update the entry for this outgoing interface */ + hlist_for_each_entry_rcu(tmp_neigh_ifinfo, + &tmp_neigh_node->ifinfo_list, + list) { + if (tmp_neigh_ifinfo->if_outgoing != if_outgoing) + continue; + + tq_recv = tmp_neigh_ifinfo->bat_iv.tq_recv; + tq_index = &tmp_neigh_ifinfo->bat_iv.tq_index; + batadv_ring_buffer_set(tq_recv, tq_index, 0); + tq_avg = batadv_ring_buffer_avg(tq_recv); + tmp_neigh_ifinfo->bat_iv.tq_avg = tq_avg; + } spin_unlock_bh(&tmp_neigh_node->bat_iv.lq_update_lock); }
@@ -946,7 +979,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
orig_tmp = batadv_iv_ogm_orig_get(bat_priv, ethhdr->h_source); if (!orig_tmp) - goto unlock; + goto out;
neigh_node = batadv_iv_ogm_neigh_new(if_incoming, ethhdr->h_source, @@ -954,26 +987,29 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
batadv_orig_node_free_ref(orig_tmp); if (!neigh_node) - goto unlock; + goto out; } else batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Updating existing last-hop neighbor of originator\n");
- rcu_read_unlock(); + neigh_ifinfo = batadv_neigh_node_get_ifinfo(neigh_node, + if_outgoing); + if (!neigh_ifinfo) + goto out;
neigh_node->last_seen = jiffies;
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_ring_buffer_set(neigh_ifinfo->bat_iv.tq_recv, + &neigh_ifinfo->bat_iv.tq_index, batadv_ogm_packet->tq); - tq_avg = batadv_ring_buffer_avg(neigh_node->bat_iv.tq_recv); - neigh_node->bat_iv.tq_avg = tq_avg; + tq_avg = batadv_ring_buffer_avg(neigh_ifinfo->bat_iv.tq_recv); + neigh_ifinfo->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; - neigh_node->last_ttl = batadv_ogm_packet->header.ttl; + neigh_ifinfo->last_ttl = batadv_ogm_packet->header.ttl; }
/* if this neighbor already is our next hop there is nothing @@ -983,14 +1019,17 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, if (router == neigh_node) goto out;
+ router_ifinfo = batadv_neigh_node_get_ifinfo(router, if_outgoing); /* if this neighbor does not offer a better TQ we won't consider it */ - if (router && (router->bat_iv.tq_avg > neigh_node->bat_iv.tq_avg)) + if (router_ifinfo && + (router_ifinfo->bat_iv.tq_avg > neigh_ifinfo->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->bat_iv.tq_avg == router->bat_iv.tq_avg)) { + if (router_ifinfo && + (neigh_ifinfo->bat_iv.tq_avg == router_ifinfo->bat_iv.tq_avg)) { orig_node_tmp = router->orig_node; spin_lock_bh(&orig_node_tmp->bat_iv.ogm_cnt_lock); if_num = router->if_incoming->if_num; @@ -1007,25 +1046,37 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, goto out; }
+ /* TODO: pass if_outgoing later */ batadv_update_route(bat_priv, orig_node, neigh_node); goto out;
-unlock: - rcu_read_unlock(); out: + rcu_read_unlock(); if (neigh_node) batadv_neigh_node_free_ref(neigh_node); if (router) batadv_neigh_node_free_ref(router); }
+/** + * batadv_iv_ogm_calc_tq - calculate tq for current received ogm packet + * @orig_node: the orig node who originally emitted the ogm packet + * @orig_neigh_node: the orig node struct of the neighbor who sent the packet + * @batadv_ogm_packet: the ogm packet + * @if_incoming: interface where the packet was received + * @if_outgoing: interface for which the retransmission should be considered + * + * Returns 1 if the link can be considered bidirectional, 0 otherwise + */ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, struct batadv_orig_node *orig_neigh_node, struct batadv_ogm_packet *batadv_ogm_packet, - struct batadv_hard_iface *if_incoming) + struct batadv_hard_iface *if_incoming, + struct batadv_hard_iface *if_outgoing) { struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface); struct batadv_neigh_node *neigh_node = NULL, *tmp_neigh_node; + struct batadv_neigh_node_ifinfo *neigh_node_ifinfo; 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; @@ -1070,7 +1121,15 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, 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; + rcu_read_lock(); + neigh_node_ifinfo = batadv_neigh_node_get_ifinfo(neigh_node, + if_outgoing); + if (!neigh_node_ifinfo) + neigh_rq_count = 0; + else + neigh_rq_count = neigh_node_ifinfo->bat_iv.real_packet_count; + + rcu_read_unlock(); spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock);
/* pay attention to not get a value bigger than 100 % */ @@ -1134,17 +1193,20 @@ out: * @ethhdr: ethernet header of the packet * @batadv_ogm_packet: OGM packet to be considered * @if_incoming: interface on which the OGM packet was received + * @if_outgoing: interface for which the retransmission should be considered * * Returns duplicate status as enum batadv_dup_status */ static enum batadv_dup_status batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, const struct batadv_ogm_packet *batadv_ogm_packet, - const struct batadv_hard_iface *if_incoming) + const struct batadv_hard_iface *if_incoming, + struct batadv_hard_iface *if_outgoing) { struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface); struct batadv_orig_node *orig_node; - struct batadv_neigh_node *tmp_neigh_node; + struct batadv_neigh_node *neigh_node; + struct batadv_neigh_node_ifinfo *neigh_ifinfo; int is_dup; int32_t seq_diff; int need_update = 0; @@ -1171,15 +1233,20 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
rcu_read_lock(); - hlist_for_each_entry_rcu(tmp_neigh_node, + hlist_for_each_entry_rcu(neigh_node, &orig_node->neigh_list, list) { - neigh_addr = tmp_neigh_node->addr; - is_dup = batadv_test_bit(tmp_neigh_node->bat_iv.real_bits, + neigh_ifinfo = batadv_neigh_node_get_ifinfo(neigh_node, + if_outgoing); + if (WARN_ON(!neigh_ifinfo)) + continue; + + neigh_addr = neigh_node->addr; + is_dup = batadv_test_bit(neigh_ifinfo->bat_iv.real_bits, orig_node->last_real_seqno, seqno);
if (batadv_compare_eth(neigh_addr, ethhdr->h_source) && - tmp_neigh_node->if_incoming == if_incoming) { + neigh_node->if_incoming == if_incoming) { set_mark = 1; if (is_dup) ret = BATADV_NEIGH_DUP; @@ -1190,15 +1257,14 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
/* if the window moved, set the update flag. */ - bitmap = tmp_neigh_node->bat_iv.real_bits; + bitmap = neigh_ifinfo->bat_iv.real_bits; need_update |= batadv_bit_get_packet(bat_priv, bitmap, seq_diff, set_mark);
- packet_count = bitmap_weight(tmp_neigh_node->bat_iv.real_bits, + packet_count = bitmap_weight(bitmap, BATADV_TQ_LOCAL_WINDOW_SIZE); - tmp_neigh_node->bat_iv.real_packet_count = packet_count; + neigh_ifinfo->bat_iv.real_packet_count = packet_count; } - rcu_read_unlock();
if (need_update) { batadv_dbg(BATADV_DBG_BATMAN, bat_priv, @@ -1206,6 +1272,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, orig_node->last_real_seqno, seqno); orig_node->last_real_seqno = seqno; } + rcu_read_unlock();
out: spin_unlock_bh(&orig_node->bat_iv.ogm_cnt_lock); @@ -1223,6 +1290,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, 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; + struct batadv_neigh_node_ifinfo *router_ifinfo = NULL; int has_directlink_flag; int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0; int is_bidirect; @@ -1355,7 +1423,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, return;
dup_status = batadv_iv_ogm_update_seqnos(ethhdr, batadv_ogm_packet, - if_incoming); + if_incoming, NULL);
if (dup_status == BATADV_PROTECTED) { batadv_dbg(BATADV_DBG_BATMAN, bat_priv, @@ -1376,7 +1444,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, router_router = batadv_orig_node_get_router(orig_node_tmp); }
- if ((router && router->bat_iv.tq_avg != 0) && + if ((router && router_ifinfo->bat_iv.tq_avg != 0) && (batadv_compare_eth(router->addr, ethhdr->h_source))) is_from_best_next_hop = true;
@@ -1422,7 +1490,8 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, }
is_bidirect = batadv_iv_ogm_calc_tq(orig_node, orig_neigh_node, - batadv_ogm_packet, if_incoming); + batadv_ogm_packet, if_incoming, + NULL);
/* update ranking if it is not a duplicate or has the same * seqno and similar ttl as the non-duplicate @@ -1433,7 +1502,7 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, (sameseq && similar_ttl))) batadv_iv_ogm_orig_update(bat_priv, orig_node, ethhdr, batadv_ogm_packet, if_incoming, - tt_buff, dup_status); + NULL, tt_buff, dup_status);
/* is single hop (direct) neighbor */ if (is_single_hop_neigh) { @@ -1541,6 +1610,7 @@ static void batadv_iv_ogm_orig_print(struct batadv_priv *bat_priv, struct batadv_hashtable *hash = bat_priv->orig_hash; int last_seen_msecs, last_seen_secs; struct batadv_orig_node *orig_node; + struct batadv_neigh_node_ifinfo *neigh_ifinfo; unsigned long last_seen_jiffies; struct hlist_head *head; int batman_count = 0; @@ -1559,7 +1629,12 @@ static void batadv_iv_ogm_orig_print(struct batadv_priv *bat_priv, if (!neigh_node) continue;
- if (neigh_node->bat_iv.tq_avg == 0) + neigh_ifinfo = batadv_neigh_node_get_ifinfo(neigh_node, + NULL); + if (!neigh_ifinfo) + goto next; + + if (neigh_ifinfo->bat_iv.tq_avg == 0) goto next;
last_seen_jiffies = jiffies - orig_node->last_seen; @@ -1569,15 +1644,20 @@ static void batadv_iv_ogm_orig_print(struct batadv_priv *bat_priv,
seq_printf(seq, "%pM %4i.%03is (%3i) %pM [%10s]:", orig_node->orig, last_seen_secs, - last_seen_msecs, neigh_node->bat_iv.tq_avg, + last_seen_msecs, neigh_ifinfo->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) { + neigh_ifinfo = batadv_neigh_node_get_ifinfo( + neigh_node, NULL); + if (!neigh_ifinfo) + continue; + seq_printf(seq, " %pM (%3i)", neigh_node_tmp->addr, - neigh_node_tmp->bat_iv.tq_avg); + neigh_ifinfo->bat_iv.tq_avg); }
seq_puts(seq, "\n"); @@ -1597,34 +1677,50 @@ next: * batadv_iv_ogm_neigh_cmp - compare the metrics of two neighbors * @neigh1: the first neighbor object of the comparison * @neigh2: the second neighbor object of the comparison + * @if_outgoing: outgoing interface for the comparison * * Returns a value less, equal to or greater than 0 if the metric via neigh1 is * lower, the same as or higher than the metric via neigh2 */ static int batadv_iv_ogm_neigh_cmp(struct batadv_neigh_node *neigh1, - struct batadv_neigh_node *neigh2) + struct batadv_neigh_node *neigh2, + struct batadv_hard_iface *if_outgoing) { + struct batadv_neigh_node_ifinfo *neigh1_ifinfo, *neigh2_ifinfo; uint8_t tq1, tq2;
- tq1 = neigh1->bat_iv.tq_avg; - tq2 = neigh2->bat_iv.tq_avg; + neigh1_ifinfo = batadv_neigh_node_get_ifinfo(neigh1, if_outgoing); + neigh2_ifinfo = batadv_neigh_node_get_ifinfo(neigh2, if_outgoing); + + if (!neigh1_ifinfo || !neigh2_ifinfo) + return 0; + + tq1 = neigh1_ifinfo->bat_iv.tq_avg; + tq2 = neigh2_ifinfo->bat_iv.tq_avg;
return tq1 - tq2; }
/** - * batadv_iv_ogm_neigh_is_eob - check if neigh1 is equally good or better than - * neigh2 from the metric prospective - * @neigh1: the first neighbor object of the comparison - * @neigh2: the second neighbor object of the comparison + * batadv_iv_ogm_neigh_is_eob - check if neigh1_ifinfo is equally good or\ + * better than neigh2_ifinfo from the metric prospective + * @neigh1_ifinfo: the first neighbor ifinfo object of the comparison + * @neigh2_ifinfo: the second neighbor ifinfo object of the comparison * - * Returns true if the metric via neigh1 is equally good or better than the - * metric via neigh2, false otherwise. + * Returns true if the metric via neigh1_ifinfo is equally good or better than + * the metric via neigh2_ifinfo, false otherwise. */ -static bool batadv_iv_ogm_neigh_is_eob(struct batadv_neigh_node *neigh1, - struct batadv_neigh_node *neigh2) +static bool +batadv_iv_ogm_neigh_is_eob(struct batadv_neigh_node_ifinfo *neigh1_ifinfo, + struct batadv_neigh_node_ifinfo *neigh2_ifinfo) { - int diff = batadv_iv_ogm_neigh_cmp(neigh1, neigh2); + uint8_t tq1, tq2; + int diff; + + tq1 = neigh1_ifinfo->bat_iv.tq_avg; + tq2 = neigh2_ifinfo->bat_iv.tq_avg; + + diff = tq1 - tq2;
return diff > -BATADV_TQ_SIMILARITY_THRESHOLD; } @@ -1638,7 +1734,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_neigh_cmp = batadv_iv_ogm_neigh_cmp, - .bat_neigh_is_equiv_or_better = batadv_iv_ogm_neigh_is_eob, + .bat_neigh_ifinfo_is_equiv_or_better = batadv_iv_ogm_neigh_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, diff --git a/gateway_client.c b/gateway_client.c index 7299936..b845ee6 100644 --- a/gateway_client.c +++ b/gateway_client.c @@ -114,6 +114,7 @@ static struct batadv_gw_node * batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv) { struct batadv_neigh_node *router; + struct batadv_neigh_node_ifinfo *router_ifinfo; struct batadv_gw_node *gw_node, *curr_gw = NULL; uint32_t max_gw_factor = 0, tmp_gw_factor = 0; uint32_t gw_divisor; @@ -134,10 +135,14 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv) if (!router) continue;
+ router_ifinfo = batadv_neigh_node_get_ifinfo(router, NULL); + if (!router_ifinfo) + goto next; + if (!atomic_inc_not_zero(&gw_node->refcount)) goto next;
- tq_avg = router->bat_iv.tq_avg; + tq_avg = router_ifinfo->bat_iv.tq_avg;
switch (atomic_read(&bat_priv->gw_sel_class)) { case 1: /* fast connection */ @@ -219,8 +224,10 @@ void batadv_gw_election(struct batadv_priv *bat_priv) { struct batadv_gw_node *curr_gw = NULL, *next_gw = NULL; struct batadv_neigh_node *router = NULL; + struct batadv_neigh_node_ifinfo *router_ifinfo = NULL; char gw_addr[18] = { '\0' };
+ rcu_read_lock(); if (atomic_read(&bat_priv->gw_mode) != BATADV_GW_MODE_CLIENT) goto out;
@@ -242,6 +249,12 @@ void batadv_gw_election(struct batadv_priv *bat_priv) batadv_gw_deselect(bat_priv); goto out; } + + router_ifinfo = batadv_neigh_node_get_ifinfo(router, NULL); + if (!router_ifinfo) { + batadv_gw_deselect(bat_priv); + goto out; + } }
if ((curr_gw) && (!next_gw)) { @@ -256,7 +269,8 @@ 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->bat_iv.tq_avg); + next_gw->bandwidth_up % 10, + router_ifinfo->bat_iv.tq_avg); batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_ADD, gw_addr); } else { @@ -266,7 +280,8 @@ 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->bat_iv.tq_avg); + next_gw->bandwidth_up % 10, + router_ifinfo->bat_iv.tq_avg); batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_CHANGE, gw_addr); } @@ -274,6 +289,7 @@ void batadv_gw_election(struct batadv_priv *bat_priv) batadv_gw_select(bat_priv, next_gw);
out: + rcu_read_unlock(); if (curr_gw) batadv_gw_node_free_ref(curr_gw); if (next_gw) @@ -287,8 +303,11 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv, { struct batadv_orig_node *curr_gw_orig; struct batadv_neigh_node *router_gw = NULL, *router_orig = NULL; + struct batadv_neigh_node_ifinfo *router_gw_tq = NULL, + *router_orig_tq = NULL; uint8_t gw_tq_avg, orig_tq_avg;
+ rcu_read_lock(); curr_gw_orig = batadv_gw_get_selected_orig(bat_priv); if (!curr_gw_orig) goto deselect; @@ -297,6 +316,10 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv, if (!router_gw) goto deselect;
+ router_gw_tq = batadv_neigh_node_get_ifinfo(router_gw, NULL); + if (!router_gw_tq) + goto deselect; + /* this node already is the gateway */ if (curr_gw_orig == orig_node) goto out; @@ -305,8 +328,15 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv, if (!router_orig) goto out;
- gw_tq_avg = router_gw->bat_iv.tq_avg; - orig_tq_avg = router_orig->bat_iv.tq_avg; + router_orig_tq = batadv_neigh_node_get_ifinfo(router_orig, NULL); + if (!router_orig_tq) { + batadv_gw_deselect(bat_priv); + goto out; + } + + + gw_tq_avg = router_gw_tq->bat_iv.tq_avg; + orig_tq_avg = router_orig_tq->bat_iv.tq_avg;
/* the TQ value has to be better */ if (orig_tq_avg < gw_tq_avg) @@ -324,6 +354,7 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv, gw_tq_avg, orig_tq_avg);
deselect: + rcu_read_unlock(); batadv_gw_deselect(bat_priv); out: if (curr_gw_orig) @@ -517,18 +548,24 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv, { struct batadv_gw_node *curr_gw; struct batadv_neigh_node *router; + struct batadv_neigh_node_ifinfo *router_ifinfo; int ret = -1;
+ rcu_read_lock(); router = batadv_orig_node_get_router(gw_node->orig_node); if (!router) goto out;
+ router_ifinfo = batadv_neigh_node_get_ifinfo(router, NULL); + if (!router_ifinfo) + goto out; + curr_gw = batadv_gw_get_selected_gw_node(bat_priv);
ret = seq_printf(seq, "%s %pM (%3i) %pM [%10s]: %u.%u/%u.%u MBit\n", (curr_gw == gw_node ? "=>" : " "), gw_node->orig_node->orig, - router->bat_iv.tq_avg, router->addr, + router_ifinfo->bat_iv.tq_avg, router->addr, router->if_incoming->net_dev->name, gw_node->bandwidth_down / 10, gw_node->bandwidth_down % 10, @@ -539,6 +576,7 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv, if (curr_gw) batadv_gw_node_free_ref(curr_gw); out: + rcu_read_unlock(); return ret; }
@@ -741,14 +779,16 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, struct batadv_neigh_node *neigh_curr = NULL, *neigh_old = NULL; struct batadv_orig_node *orig_dst_node = NULL; struct batadv_gw_node *gw_node = NULL, *curr_gw = NULL; + struct batadv_neigh_node_ifinfo *curr_ifinfo, *old_ifinfo; struct ethhdr *ethhdr; bool ret, out_of_range = false; unsigned int header_len = 0; - uint8_t curr_tq_avg; + uint8_t curr_if_avg; unsigned short vid;
vid = batadv_get_vid(skb, 0);
+ rcu_read_lock(); ret = batadv_gw_is_dhcp_target(skb, &header_len); if (!ret) goto out; @@ -772,7 +812,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, /* If we are a GW then we are our best GW. We can artificially * set the tq towards ourself as the maximum value */ - curr_tq_avg = BATADV_TQ_MAX_VALUE; + curr_if_avg = BATADV_TQ_MAX_VALUE; break; case BATADV_GW_MODE_CLIENT: curr_gw = batadv_gw_get_selected_gw_node(bat_priv); @@ -792,7 +832,11 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, if (!neigh_curr) goto out;
- curr_tq_avg = neigh_curr->bat_iv.tq_avg; + curr_ifinfo = batadv_neigh_node_get_ifinfo(neigh_curr, NULL); + if (!curr_ifinfo) + goto out; + + curr_if_avg = curr_ifinfo->bat_iv.tq_avg; break; case BATADV_GW_MODE_OFF: default: @@ -803,10 +847,15 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, if (!neigh_old) goto out;
- if (curr_tq_avg - neigh_old->bat_iv.tq_avg > BATADV_GW_THRESHOLD) + old_ifinfo = batadv_neigh_node_get_ifinfo(neigh_old, NULL); + if (!old_ifinfo) + goto out; + + if ((curr_if_avg - old_ifinfo->bat_iv.tq_avg) > BATADV_GW_THRESHOLD) out_of_range = true;
out: + rcu_read_unlock(); if (orig_dst_node) batadv_orig_node_free_ref(orig_dst_node); if (curr_gw) diff --git a/main.c b/main.c index 5c5e64b..cd6fac4 100644 --- a/main.c +++ b/main.c @@ -504,7 +504,7 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops) !bat_algo_ops->bat_ogm_schedule || !bat_algo_ops->bat_ogm_emit || !bat_algo_ops->bat_neigh_cmp || - !bat_algo_ops->bat_neigh_is_equiv_or_better) { + !bat_algo_ops->bat_neigh_ifinfo_is_equiv_or_better) { pr_info("Routing algo '%s' does not implement required ops\n", bat_algo_ops->name); ret = -EINVAL; diff --git a/originator.c b/originator.c index 919fdc0..52cd618 100644 --- a/originator.c +++ b/originator.c @@ -150,10 +150,28 @@ err: return -ENOMEM; }
+static void batadv_neigh_node_free_rcu(struct rcu_head *rcu) +{ + struct hlist_node *node_tmp; + struct batadv_neigh_node *neigh_node; + struct batadv_neigh_node_ifinfo *neigh_node_ifinfo; + + neigh_node = container_of(rcu, struct batadv_neigh_node, rcu); + + hlist_for_each_entry_safe(neigh_node_ifinfo, node_tmp, + &neigh_node->ifinfo_list, list) { + if (neigh_node_ifinfo->if_outgoing) + batadv_hardif_free_ref(neigh_node_ifinfo->if_outgoing); + kfree(neigh_node_ifinfo); + } + + kfree(neigh_node); +} + void batadv_neigh_node_free_ref(struct batadv_neigh_node *neigh_node) { if (atomic_dec_and_test(&neigh_node->refcount)) - kfree_rcu(neigh_node, rcu); + call_rcu(&neigh_node->rcu, batadv_neigh_node_free_rcu); }
/* increases the refcounter of a found router */ @@ -173,6 +191,55 @@ batadv_orig_node_get_router(struct batadv_orig_node *orig_node) }
/** + * batadv_neigh_node_get_ifinfo - gets the ifinfo from an neigh_node + * @neigh_node: the neigh node to be queried + * @if_outgoing: the interface for which the ifinfo should be acquired + * + * Note: this function must be called under rcu lock + * + * Returns the requested neigh_node_ifinfo or NULL if not found + */ +struct batadv_neigh_node_ifinfo * +batadv_neigh_node_get_ifinfo(struct batadv_neigh_node *neigh, + struct batadv_hard_iface *if_outgoing) +{ + struct batadv_neigh_node_ifinfo *neigh_ifinfo = NULL, + *tmp_neigh_ifinfo; + + rcu_read_lock(); + hlist_for_each_entry_rcu(tmp_neigh_ifinfo, &neigh->ifinfo_list, + list) { + if (tmp_neigh_ifinfo->if_outgoing != if_outgoing) + continue; + + neigh_ifinfo = tmp_neigh_ifinfo; + } + if (neigh_ifinfo) + goto out; + + neigh_ifinfo = kzalloc(sizeof(*neigh_ifinfo), GFP_ATOMIC); + if (!neigh_ifinfo) + goto out; + + if (if_outgoing && !atomic_inc_not_zero(&if_outgoing->refcount)) { + kfree(neigh_ifinfo); + neigh_ifinfo = NULL; + goto out; + } + + INIT_HLIST_NODE(&neigh_ifinfo->list); + neigh_ifinfo->if_outgoing = if_outgoing; + + spin_lock_bh(&neigh->bat_iv.lq_update_lock); + hlist_add_head_rcu(&neigh_ifinfo->list, &neigh->ifinfo_list); + spin_unlock_bh(&neigh->bat_iv.lq_update_lock); +out: + rcu_read_unlock(); + + return neigh_ifinfo; +} + +/** * batadv_neigh_node_new - create and init a new neigh_node object * @hard_iface: the interface where the neighbour is connected to * @neigh_addr: the mac address of the neighbour interface @@ -181,6 +248,7 @@ batadv_orig_node_get_router(struct batadv_orig_node *orig_node) * Allocates a new neigh_node object and initialises all the generic fields. * Returns the new object or NULL on failure. */ + struct batadv_neigh_node * batadv_neigh_node_new(struct batadv_hard_iface *hard_iface, const uint8_t *neigh_addr, @@ -193,6 +261,7 @@ batadv_neigh_node_new(struct batadv_hard_iface *hard_iface, goto out;
INIT_HLIST_NODE(&neigh_node->list); + INIT_HLIST_HEAD(&neigh_node->ifinfo_list);
memcpy(neigh_node->addr, neigh_addr, ETH_ALEN); neigh_node->if_incoming = hard_iface; @@ -364,20 +433,23 @@ free_orig_node: return NULL; }
+/** + * batadv_purge_orig_neighbors - purges neighbors from originator + * @bat_priv: the bat priv with all the soft interface information + * @orig_node: orig node which is to be checked + * + * Returns true if any neighbor was purged, false otherwise + */ static bool batadv_purge_orig_neighbors(struct batadv_priv *bat_priv, - struct batadv_orig_node *orig_node, - struct batadv_neigh_node **best_neigh) + struct batadv_orig_node *orig_node) { - struct batadv_algo_ops *bao = bat_priv->bat_algo_ops; struct hlist_node *node_tmp; struct batadv_neigh_node *neigh_node; bool neigh_purged = false; unsigned long last_seen; struct batadv_hard_iface *if_incoming;
- *best_neigh = NULL; - spin_lock_bh(&orig_node->neigh_list_lock);
/* for all neighbors towards this originator ... */ @@ -407,13 +479,6 @@ batadv_purge_orig_neighbors(struct batadv_priv *bat_priv,
hlist_del_rcu(&neigh_node->list); batadv_neigh_node_free_ref(neigh_node); - } else { - /* store the best_neighbour if this is the first - * iteration or if a better neighbor has been found - */ - if (!*best_neigh || - bao->bat_neigh_cmp(neigh_node, *best_neigh) > 0) - *best_neigh = neigh_node; } }
@@ -421,6 +486,32 @@ batadv_purge_orig_neighbors(struct batadv_priv *bat_priv, return neigh_purged; }
+/** + * batadv_find_best_neighbor - finds the best neighbor after purging + * @bat_priv: the bat priv with all the soft interface information + * @orig_node: orig node which is to be checked + * @if_outgoing: the interface for which the metric should be compared + * + * Returns the current best neighbor + */ +static struct batadv_neigh_node * +batadv_find_best_neighbor(struct batadv_priv *bat_priv, + struct batadv_orig_node *orig_node, + struct batadv_hard_iface *if_outgoing) +{ + struct batadv_neigh_node *best = NULL, *neigh; + struct batadv_algo_ops *bao = bat_priv->bat_algo_ops; + + rcu_read_lock(); + hlist_for_each_entry_rcu(neigh, &orig_node->neigh_list, list) + if (!best || + (bao->bat_neigh_cmp(neigh, best, if_outgoing) <= 0)) + best = neigh; + rcu_read_unlock(); + + return best; +} + static bool batadv_purge_orig_node(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node) { @@ -433,12 +524,12 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv, orig_node->orig, jiffies_to_msecs(orig_node->last_seen)); return true; - } else { - if (batadv_purge_orig_neighbors(bat_priv, orig_node, - &best_neigh_node)) - batadv_update_route(bat_priv, orig_node, - best_neigh_node); } + if (!batadv_purge_orig_neighbors(bat_priv, orig_node)) + return false; + + best_neigh_node = batadv_find_best_neighbor(bat_priv, orig_node, NULL); + batadv_update_route(bat_priv, orig_node, best_neigh_node);
return false; } diff --git a/originator.h b/originator.h index 6f77d80..161c1e3 100644 --- a/originator.h +++ b/originator.h @@ -37,6 +37,9 @@ batadv_neigh_node_new(struct batadv_hard_iface *hard_iface, 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); +struct batadv_neigh_node_ifinfo * +batadv_neigh_node_get_ifinfo(struct batadv_neigh_node *neigh, + struct batadv_hard_iface *if_outgoing); int batadv_orig_seq_print_text(struct seq_file *seq, void *offset); int batadv_orig_hash_add_if(struct batadv_hard_iface *hard_iface, int max_if_num); diff --git a/translation-table.c b/translation-table.c index 1198aea..ea4c65d 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1361,7 +1361,7 @@ batadv_transtable_best_orig(struct batadv_priv *bat_priv, continue;
if (best_router && - bao->bat_neigh_cmp(router, best_router) <= 0) { + bao->bat_neigh_cmp(router, best_router, NULL) <= 0) { batadv_neigh_node_free_ref(router); continue; } diff --git a/types.h b/types.h index ef08333..e830e20 100644 --- a/types.h +++ b/types.h @@ -269,32 +269,20 @@ struct batadv_gw_node { /** * struct batadv_neigh_bat_iv - B.A.T.M.A.N. IV specific 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) - * @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 - * @lq_update_lock: lock protecting tq_recv & tq_index + * @lq_update_lock: lock protecting tq_recv & tq_index for ifinfo members */ struct batadv_neigh_bat_iv { - uint8_t tq_recv[BATADV_TQ_GLOBAL_WINDOW_SIZE]; - uint8_t tq_index; - uint8_t tq_avg; - 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 + * @ifinfo_list: list for routing metrics per outgoing 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 - * @refcount: number of contexts the object is used * @rcu: struct used for freeing in an RCU-safe manner * @bat_iv: B.A.T.M.A.N. IV private structure */ @@ -302,14 +290,43 @@ struct batadv_neigh_node { struct hlist_node list; struct batadv_orig_node *orig_node; uint8_t addr[ETH_ALEN]; + struct hlist_head ifinfo_list; struct batadv_hard_iface *if_incoming; unsigned long last_seen; - uint8_t last_ttl; atomic_t refcount; struct rcu_head rcu; struct batadv_neigh_bat_iv bat_iv; };
+/* struct batadv_neigh_node_ifinfo - neighbor information per outgoing interface + * for BATMAN IV + * @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) + * @real_bits: bitfield containing the number of OGMs received from this neigh + */ +struct batadv_neigh_ifinfo_bat_iv { + uint8_t tq_recv[BATADV_TQ_GLOBAL_WINDOW_SIZE]; + uint8_t tq_index; + uint8_t tq_avg; + DECLARE_BITMAP(real_bits, BATADV_TQ_LOCAL_WINDOW_SIZE); + uint8_t real_packet_count; +}; + +/* struct batadv_neigh_node_ifinfo - neighbor information per outgoing interface + * @list: list node for batadv_orig_node::neigh_list + * @if_outgoing: pointer to outgoing hard interface + * @last_ttl: last received ttl from this neigh node + * node (relative to orig_node->last_real_seqno) + * @rcu: struct used for freeing in an RCU-safe manner + */ +struct batadv_neigh_node_ifinfo { + struct hlist_node list; + struct batadv_hard_iface *if_outgoing; + struct batadv_neigh_ifinfo_bat_iv bat_iv; + uint8_t last_ttl; +}; + /** * struct batadv_bcast_duplist_entry - structure for LAN broadcast suppression * @orig[ETH_ALEN]: mac address of orig node orginating the broadcast @@ -989,9 +1006,10 @@ 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_neigh_cmp: compare the metrics of two neighbors - * @bat_neigh_is_equiv_or_better: check if neigh1 is equally good or - * better than neigh2 from the metric prospective + * @bat_neigh_cmp: compare the metrics of two neighbors for an outgoing + * interface + * @bat_neigh_is_equiv_or_better: check if neigh1_ifinfo is equally good or + * better than neigh2_ifinfo from the metric prospective * @bat_orig_print: print the originator table (optional) * @bat_orig_free: free the resources allocated by the routing algorithm for an * orig_node object @@ -1010,9 +1028,11 @@ struct batadv_algo_ops { void (*bat_ogm_schedule)(struct batadv_hard_iface *hard_iface); void (*bat_ogm_emit)(struct batadv_forw_packet *forw_packet); int (*bat_neigh_cmp)(struct batadv_neigh_node *neigh1, - struct batadv_neigh_node *neigh2); - bool (*bat_neigh_is_equiv_or_better)(struct batadv_neigh_node *neigh1, - struct batadv_neigh_node *neigh2); + struct batadv_neigh_node *neigh2, + struct batadv_hard_iface *if_outgoing); + bool (*bat_neigh_ifinfo_is_equiv_or_better) + (struct batadv_neigh_node_ifinfo *neigh1_ifinfo, + struct batadv_neigh_node_ifinfo *neigh2_ifinfo); /* 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);
On Wed, Sep 04, 2013 at 06:27:36PM +0200, Simon Wunderlich wrote:
@@ -897,22 +904,39 @@ static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface) batadv_hardif_free_ref(primary_if); }
+/**
- batadv_iv_ogm_orig_update - use OGM to update corresponding data in an
- originator
- @bat_priv: the bat priv with all the soft interface information
- @orig_node: the orig node who originally emitted the ogm packet
- @orig_node_ifinfo: ifinfo for the according outgoing interface
where is this argument in the function declaration?
- @ethhdr: Ethernet header of the OGM
- @@batadv_ogm_packet: the ogm packet
too many '@' :)
- @if_incoming: interface where the packet was received
- @if_outgoing: interface for which the retransmission should be considered
- @tt_buff: pointer to the tt buffer
- @dup_status: the duplicate status of this ogm packet.
- */
static void batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, const struct ethhdr *ethhdr, const struct batadv_ogm_packet *batadv_ogm_packet, struct batadv_hard_iface *if_incoming,
struct batadv_hard_iface *if_outgoing, const unsigned char *tt_buff, enum batadv_dup_status dup_status)
{ struct batadv_neigh_node *neigh_node = NULL, *tmp_neigh_node = NULL; struct batadv_neigh_node *router = NULL; struct batadv_orig_node *orig_node_tmp;
- struct batadv_neigh_node_ifinfo *neigh_ifinfo = NULL,
*tmp_neigh_ifinfo,
*router_ifinfo;
we have never used this kind of style for declaration. I'd prefer you to re-write the variable type on each line, please.
int if_num; uint8_t sum_orig, sum_neigh; uint8_t *neigh_addr;
- uint8_t tq_avg;
uint8_t tq_avg, *tq_recv, *tq_index;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "update_originator(): Searching and updating originator entry of received packet\n");
[...]
@@ -1134,17 +1193,20 @@ out:
- @ethhdr: ethernet header of the packet
- @batadv_ogm_packet: OGM packet to be considered
- @if_incoming: interface on which the OGM packet was received
*/
- @if_outgoing: interface for which the retransmission should be considered
- Returns duplicate status as enum batadv_dup_status
static enum batadv_dup_status batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, const struct batadv_ogm_packet *batadv_ogm_packet,
const struct batadv_hard_iface *if_incoming)
const struct batadv_hard_iface *if_incoming,
struct batadv_hard_iface *if_outgoing)
why isn't this const as well? It is not going to be changed or what, so keeping the const qualifier is good imho.
{ struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface); struct batadv_orig_node *orig_node;
- struct batadv_neigh_node *tmp_neigh_node;
- struct batadv_neigh_node *neigh_node;
- struct batadv_neigh_node_ifinfo *neigh_ifinfo; int is_dup; int32_t seq_diff; int need_update = 0;
@@ -1171,15 +1233,20 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, }
rcu_read_lock();
- hlist_for_each_entry_rcu(tmp_neigh_node,
- hlist_for_each_entry_rcu(neigh_node, &orig_node->neigh_list, list) {
maybe this line can be rearranged now that the first parameter is shorter than before?
neigh_addr = tmp_neigh_node->addr;
is_dup = batadv_test_bit(tmp_neigh_node->bat_iv.real_bits,
neigh_ifinfo = batadv_neigh_node_get_ifinfo(neigh_node,
if_outgoing);
if (WARN_ON(!neigh_ifinfo))
continue;
neigh_addr = neigh_node->addr;
is_dup = batadv_test_bit(neigh_ifinfo->bat_iv.real_bits, orig_node->last_real_seqno, seqno);
if (batadv_compare_eth(neigh_addr, ethhdr->h_source) &&
tmp_neigh_node->if_incoming == if_incoming) {
neigh_node->if_incoming == if_incoming) { set_mark = 1; if (is_dup) ret = BATADV_NEIGH_DUP;
[...]
@@ -1597,34 +1677,50 @@ next:
- batadv_iv_ogm_neigh_cmp - compare the metrics of two neighbors
- @neigh1: the first neighbor object of the comparison
- @neigh2: the second neighbor object of the comparison
*/
- @if_outgoing: outgoing interface for the comparison
- Returns a value less, equal to or greater than 0 if the metric via neigh1 is
- lower, the same as or higher than the metric via neigh2
static int batadv_iv_ogm_neigh_cmp(struct batadv_neigh_node *neigh1,
struct batadv_neigh_node *neigh2)
struct batadv_neigh_node *neigh2,
struct batadv_hard_iface *if_outgoing)
To make this API really generic, wouldn't it better to specify an if_outgoing1 and an if_outgoing2 (one per neigh)? The use cases we have now would invoke this function passing the same iface as both arguments, but if we want to make this API generic enough and avoid changing it in the future I'd suggest this change.
Theoretically we could use this function by passing the same neigh and two different interfaces...no? In this way we decide which is the best outgoing interface (what I described does not sound like the best approach...I think I will understand how you do this in the next patches...)
[...]
/**
- batadv_iv_ogm_neigh_is_eob - check if neigh1 is equally good or better than
- neigh2 from the metric prospective
- @neigh1: the first neighbor object of the comparison
- @neigh2: the second neighbor object of the comparison
- batadv_iv_ogm_neigh_is_eob - check if neigh1_ifinfo is equally good or\
- better than neigh2_ifinfo from the metric prospective
- @neigh1_ifinfo: the first neighbor ifinfo object of the comparison
- @neigh2_ifinfo: the second neighbor ifinfo object of the comparison
- Returns true if the metric via neigh1 is equally good or better than the
- metric via neigh2, false otherwise.
- Returns true if the metric via neigh1_ifinfo is equally good or better than
*/
- the metric via neigh2_ifinfo, false otherwise.
-static bool batadv_iv_ogm_neigh_is_eob(struct batadv_neigh_node *neigh1,
struct batadv_neigh_node *neigh2)
+static bool +batadv_iv_ogm_neigh_is_eob(struct batadv_neigh_node_ifinfo *neigh1_ifinfo,
struct batadv_neigh_node_ifinfo *neigh2_ifinfo)
We could do the same as the previous function (..neigh1, iface1, neigh2, iface2). In this way we would not need to get the ifinfo objects outside but we can leave the duty to this function (and reduce code).
[...]
@@ -287,8 +303,11 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv, { struct batadv_orig_node *curr_gw_orig; struct batadv_neigh_node *router_gw = NULL, *router_orig = NULL;
- struct batadv_neigh_node_ifinfo *router_gw_tq = NULL,
*router_orig_tq = NULL;
as before.
[...]
@@ -173,6 +191,55 @@ batadv_orig_node_get_router(struct batadv_orig_node *orig_node) }
/**
- batadv_neigh_node_get_ifinfo - gets the ifinfo from an neigh_node
- @neigh_node: the neigh node to be queried
- @if_outgoing: the interface for which the ifinfo should be acquired
- Note: this function must be called under rcu lock
- Returns the requested neigh_node_ifinfo or NULL if not found
- */
+struct batadv_neigh_node_ifinfo * +batadv_neigh_node_get_ifinfo(struct batadv_neigh_node *neigh,
struct batadv_hard_iface *if_outgoing)
+{
- struct batadv_neigh_node_ifinfo *neigh_ifinfo = NULL,
*tmp_neigh_ifinfo;
- rcu_read_lock();
- hlist_for_each_entry_rcu(tmp_neigh_ifinfo, &neigh->ifinfo_list,
list) {
if (tmp_neigh_ifinfo->if_outgoing != if_outgoing)
continue;
neigh_ifinfo = tmp_neigh_ifinfo;
- }
- if (neigh_ifinfo)
goto out;
- neigh_ifinfo = kzalloc(sizeof(*neigh_ifinfo), GFP_ATOMIC);
- if (!neigh_ifinfo)
goto out;
- if (if_outgoing && !atomic_inc_not_zero(&if_outgoing->refcount)) {
kfree(neigh_ifinfo);
neigh_ifinfo = NULL;
goto out;
- }
- INIT_HLIST_NODE(&neigh_ifinfo->list);
- neigh_ifinfo->if_outgoing = if_outgoing;
- spin_lock_bh(&neigh->bat_iv.lq_update_lock);
- hlist_add_head_rcu(&neigh_ifinfo->list, &neigh->ifinfo_list);
- spin_unlock_bh(&neigh->bat_iv.lq_update_lock);
These batman iv attributes should be initialised in a batman-iv-private function, otherwise we are back with to code (something that has been partly solved by my previous patchset)...
[..]
- bool (*bat_neigh_ifinfo_is_equiv_or_better)
(struct batadv_neigh_node_ifinfo *neigh1_ifinfo,
struct batadv_neigh_node_ifinfo *neigh2_ifinfo);
really ugly! but I don't know how we should re-arrange this... :(
Cheers,
Hey Antonio,
thanks a lot for the review. I'll make changes according to your suggestions, and will comment on stuff which I don't change or which require more discussion:
On Fri, Sep 13, 2013 at 03:23:54PM +0200, Antonio Quartulli wrote:
On Wed, Sep 04, 2013 at 06:27:36PM +0200, Simon Wunderlich wrote:
@@ -1134,17 +1193,20 @@ out:
- @ethhdr: ethernet header of the packet
- @batadv_ogm_packet: OGM packet to be considered
- @if_incoming: interface on which the OGM packet was received
*/
- @if_outgoing: interface for which the retransmission should be considered
- Returns duplicate status as enum batadv_dup_status
static enum batadv_dup_status batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, const struct batadv_ogm_packet *batadv_ogm_packet,
const struct batadv_hard_iface *if_incoming)
const struct batadv_hard_iface *if_incoming,
struct batadv_hard_iface *if_outgoing)
why isn't this const as well? It is not going to be changed or what, so keeping the const qualifier is good imho.
This can't be const - batadv_neigh_node_get_ifinfo() is called within this function, and this might change the refcounter in the if_outgoing interface.
@@ -1597,34 +1677,50 @@ next:
- batadv_iv_ogm_neigh_cmp - compare the metrics of two neighbors
- @neigh1: the first neighbor object of the comparison
- @neigh2: the second neighbor object of the comparison
*/
- @if_outgoing: outgoing interface for the comparison
- Returns a value less, equal to or greater than 0 if the metric via neigh1 is
- lower, the same as or higher than the metric via neigh2
static int batadv_iv_ogm_neigh_cmp(struct batadv_neigh_node *neigh1,
struct batadv_neigh_node *neigh2)
struct batadv_neigh_node *neigh2,
struct batadv_hard_iface *if_outgoing)
To make this API really generic, wouldn't it better to specify an if_outgoing1 and an if_outgoing2 (one per neigh)? The use cases we have now would invoke this function passing the same iface as both arguments, but if we want to make this API generic enough and avoid changing it in the future I'd suggest this change.
Theoretically we could use this function by passing the same neigh and two different interfaces...no? In this way we decide which is the best outgoing interface (what I described does not sound like the best approach...I think I will understand how you do this in the next patches...)
I agree - so far we only can use it for the bonding case later, but maybe there are other use cases. In most cases we will use the same outgoint interface, but it doesn't really hurt having two parameters.
[...]
/**
- batadv_iv_ogm_neigh_is_eob - check if neigh1 is equally good or better than
- neigh2 from the metric prospective
- @neigh1: the first neighbor object of the comparison
- @neigh2: the second neighbor object of the comparison
- batadv_iv_ogm_neigh_is_eob - check if neigh1_ifinfo is equally good or\
- better than neigh2_ifinfo from the metric prospective
- @neigh1_ifinfo: the first neighbor ifinfo object of the comparison
- @neigh2_ifinfo: the second neighbor ifinfo object of the comparison
- Returns true if the metric via neigh1 is equally good or better than the
- metric via neigh2, false otherwise.
- Returns true if the metric via neigh1_ifinfo is equally good or better than
*/
- the metric via neigh2_ifinfo, false otherwise.
-static bool batadv_iv_ogm_neigh_is_eob(struct batadv_neigh_node *neigh1,
struct batadv_neigh_node *neigh2)
+static bool +batadv_iv_ogm_neigh_is_eob(struct batadv_neigh_node_ifinfo *neigh1_ifinfo,
struct batadv_neigh_node_ifinfo *neigh2_ifinfo)
We could do the same as the previous function (..neigh1, iface1, neigh2, iface2). In this way we would not need to get the ifinfo objects outside but we can leave the duty to this function (and reduce code).
Yup.
@@ -173,6 +191,55 @@ batadv_orig_node_get_router(struct batadv_orig_node *orig_node) }
/**
- batadv_neigh_node_get_ifinfo - gets the ifinfo from an neigh_node
- @neigh_node: the neigh node to be queried
- @if_outgoing: the interface for which the ifinfo should be acquired
- Note: this function must be called under rcu lock
- Returns the requested neigh_node_ifinfo or NULL if not found
- */
+struct batadv_neigh_node_ifinfo * +batadv_neigh_node_get_ifinfo(struct batadv_neigh_node *neigh,
struct batadv_hard_iface *if_outgoing)
+{
- struct batadv_neigh_node_ifinfo *neigh_ifinfo = NULL,
*tmp_neigh_ifinfo;
- rcu_read_lock();
- hlist_for_each_entry_rcu(tmp_neigh_ifinfo, &neigh->ifinfo_list,
list) {
if (tmp_neigh_ifinfo->if_outgoing != if_outgoing)
continue;
neigh_ifinfo = tmp_neigh_ifinfo;
- }
- if (neigh_ifinfo)
goto out;
- neigh_ifinfo = kzalloc(sizeof(*neigh_ifinfo), GFP_ATOMIC);
- if (!neigh_ifinfo)
goto out;
- if (if_outgoing && !atomic_inc_not_zero(&if_outgoing->refcount)) {
kfree(neigh_ifinfo);
neigh_ifinfo = NULL;
goto out;
- }
- INIT_HLIST_NODE(&neigh_ifinfo->list);
- neigh_ifinfo->if_outgoing = if_outgoing;
- spin_lock_bh(&neigh->bat_iv.lq_update_lock);
- hlist_add_head_rcu(&neigh_ifinfo->list, &neigh->ifinfo_list);
- spin_unlock_bh(&neigh->bat_iv.lq_update_lock);
These batman iv attributes should be initialised in a batman-iv-private function, otherwise we are back with to code (something that has been partly solved by my previous patchset)...
Yes, you are right ... I've renamed this to neigh->ifinfo_lock because this lock is now used for ifinfo_list and its members.
[..]
- bool (*bat_neigh_ifinfo_is_equiv_or_better)
(struct batadv_neigh_node_ifinfo *neigh1_ifinfo,
struct batadv_neigh_node_ifinfo *neigh2_ifinfo);
really ugly! but I don't know how we should re-arrange this... :(
Yep, but I don't know how to fix that. This function is still way too long, maybe rename it to eob instead of equiv_or_better?
Cheers,
-- Antonio Quartulli
..each of us alone is worth nothing.. Ernesto "Che" Guevara
From: Simon Wunderlich simon@open-mesh.com
For the network wide multi interface optimization there are different routers for each outgoing interface (outgoing from the OGM perspective, incoming for payload traffic). To reflect this, change the router and associated data to a list of routers.
Signed-off-by: Simon Wunderlich simon@open-mesh.com --- Changes to RFC: * rebase on current master * remove useless goto * split out batman_seqno_reset as well to avoid false seqno window protections --- bat_iv_ogm.c | 395 +++++++++++++++++++++++++++++------------------ distributed-arp-table.c | 3 +- gateway_client.c | 10 +- icmp_socket.c | 2 +- network-coding.c | 9 +- originator.c | 166 +++++++++++++++++++- originator.h | 11 +- routing.c | 42 ++++- routing.h | 1 + translation-table.c | 3 +- types.h | 29 +++- 11 files changed, 488 insertions(+), 183 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index d46746f..d32b2f2 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -920,6 +920,7 @@ static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface) static void batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, + struct batadv_orig_node_ifinfo *orig_node_ifinfo, const struct ethhdr *ethhdr, const struct batadv_ogm_packet *batadv_ogm_packet, struct batadv_hard_iface *if_incoming, @@ -1008,22 +1009,28 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, 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; + orig_node_ifinfo->last_ttl = batadv_ogm_packet->header.ttl; neigh_ifinfo->last_ttl = batadv_ogm_packet->header.ttl; }
/* if this neighbor already is our next hop there is nothing * to change */ - router = batadv_orig_node_get_router(orig_node); + router = batadv_orig_node_get_router(orig_node, if_outgoing); if (router == neigh_node) goto out;
- router_ifinfo = batadv_neigh_node_get_ifinfo(router, if_outgoing); - /* if this neighbor does not offer a better TQ we won't consider it */ - if (router_ifinfo && - (router_ifinfo->bat_iv.tq_avg > neigh_ifinfo->bat_iv.tq_avg)) - goto out; + if (router) { + router_ifinfo = batadv_neigh_node_get_ifinfo(router, + if_outgoing); + /* if this neighbor does not offer a better TQ we won't + * consider it + */ + if (router_ifinfo->bat_iv.tq_avg > neigh_ifinfo->bat_iv.tq_avg) + goto out; + } else { + router_ifinfo = NULL; + }
/* if the TQ is the same and the link not more symmetric we * won't consider it either @@ -1046,10 +1053,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv, goto out; }
- /* TODO: pass if_outgoing later */ - batadv_update_route(bat_priv, orig_node, neigh_node); - goto out; - + batadv_update_route(bat_priv, orig_node, if_outgoing, neigh_node); out: rcu_read_unlock(); if (neigh_node) @@ -1205,6 +1209,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, { struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface); struct batadv_orig_node *orig_node; + struct batadv_orig_node_ifinfo *orig_node_ifinfo; struct batadv_neigh_node *neigh_node; struct batadv_neigh_node_ifinfo *neigh_ifinfo; int is_dup; @@ -1221,13 +1226,19 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr, if (!orig_node) return BATADV_NO_DUP;
+ orig_node_ifinfo = batadv_orig_node_get_ifinfo(orig_node, if_outgoing); + if (WARN_ON(!orig_node_ifinfo)) { + batadv_orig_node_free_ref(orig_node); + return 0; + } + spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock); - seq_diff = seqno - orig_node->last_real_seqno; + seq_diff = seqno - orig_node_ifinfo->last_real_seqno;
/* signalize caller that the packet is to be dropped. */ if (!hlist_empty(&orig_node->neigh_list) && batadv_window_protected(bat_priv, seq_diff, - &orig_node->batman_seqno_reset)) { + &orig_node_ifinfo->batman_seqno_reset)) { ret = BATADV_PROTECTED; goto out; } @@ -1242,7 +1253,7 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
neigh_addr = neigh_node->addr; is_dup = batadv_test_bit(neigh_ifinfo->bat_iv.real_bits, - orig_node->last_real_seqno, + orig_node_ifinfo->last_real_seqno, seqno);
if (batadv_compare_eth(neigh_addr, ethhdr->h_source) && @@ -1268,9 +1279,10 @@ batadv_iv_ogm_update_seqnos(const struct ethhdr *ethhdr,
if (need_update) { batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "updating last_seqno: old %u, new %u\n", - orig_node->last_real_seqno, seqno); - orig_node->last_real_seqno = seqno; + "%s updating last_seqno: old %u, new %u\n", + if_outgoing ? if_outgoing->net_dev->name : "NULL", + orig_node_ifinfo->last_real_seqno, seqno); + orig_node_ifinfo->last_real_seqno = seqno; } rcu_read_unlock();
@@ -1280,26 +1292,214 @@ out: return ret; }
-static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, - struct batadv_ogm_packet *batadv_ogm_packet, - const unsigned char *tt_buff, - struct batadv_hard_iface *if_incoming) + +/** + * batadv_iv_ogm_process_per_outif - process a batman iv OGM for an outgoing if + * @ethhdr: the original ethernet header of the sender + * @orig_node: the orig node for the originator of this packet + * @batadv_ogm_packet: pointer to the ogm packet + * @tt_buff: pointer to the tt buffer + * @if_incoming: the interface where this packet was receved + * @if_outgoing: the interface for which the packet should be considered + */ +static void +batadv_iv_ogm_process_per_outif(const struct ethhdr *ethhdr, + struct batadv_orig_node *orig_node, + struct batadv_ogm_packet *batadv_ogm_packet, + const unsigned char *tt_buff, + struct batadv_hard_iface *if_incoming, + struct batadv_hard_iface *if_outgoing) { 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, *orig_node_tmp; struct batadv_neigh_node *router = NULL, *router_router = NULL; struct batadv_neigh_node *orig_neigh_router = NULL; struct batadv_neigh_node_ifinfo *router_ifinfo = NULL; - int has_directlink_flag; - int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0; - int is_bidirect; - bool is_single_hop_neigh = false; - bool is_from_best_next_hop = false; - int sameseq, similar_ttl; + struct batadv_orig_node *orig_neigh_node, *orig_node_tmp; + struct batadv_orig_node_ifinfo *orig_node_ifinfo; + int is_bidirect, sameseq, similar_ttl; enum batadv_dup_status dup_status; - uint32_t if_incoming_seqno; uint8_t *prev_sender; + struct batadv_ogm_packet ogm_packet_backup; + bool is_from_best_next_hop = false; + bool is_single_hop_neigh = false; + + /* some functions change tq value and/or flags. backup the ogm packet + * and restore it at the end to allow other interfaces to access the + * original data. + */ + + memcpy(&ogm_packet_backup, batadv_ogm_packet, + sizeof(ogm_packet_backup)); + + dup_status = batadv_iv_ogm_update_seqnos(ethhdr, batadv_ogm_packet, + if_incoming, if_outgoing); + if (batadv_compare_eth(ethhdr->h_source, batadv_ogm_packet->orig)) + is_single_hop_neigh = true; + + if (dup_status == BATADV_PROTECTED) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: packet within seqno protection time (sender: %pM)\n", + ethhdr->h_source); + goto out; + } + + if (batadv_ogm_packet->tq == 0) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: originator packet with tq equal 0\n"); + goto out; + } + + rcu_read_lock(); + router = batadv_orig_node_get_router(orig_node, if_outgoing); + if (router) { + orig_node_tmp = orig_node_tmp; + router_router = batadv_orig_node_get_router(router->orig_node, + if_outgoing); + router_ifinfo = batadv_neigh_node_get_ifinfo(router, + if_outgoing); + } + + if ((router_ifinfo && router_ifinfo->bat_iv.tq_avg != 0) && + (batadv_compare_eth(router->addr, ethhdr->h_source))) + is_from_best_next_hop = true; + rcu_read_unlock(); + + prev_sender = batadv_ogm_packet->prev_sender; + /* avoid temporary routing loops */ + if (router && router_router && + (batadv_compare_eth(router->addr, prev_sender)) && + !(batadv_compare_eth(batadv_ogm_packet->orig, prev_sender)) && + (batadv_compare_eth(router->addr, router_router->addr))) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: ignoring all rebroadcast packets that may make me loop (sender: %pM)\n", + ethhdr->h_source); + goto out; + } + + /* if sender is a direct neighbor the sender mac equals + * originator mac + */ + if (is_single_hop_neigh) + orig_neigh_node = orig_node; + else + orig_neigh_node = batadv_iv_ogm_orig_get(bat_priv, + ethhdr->h_source); + + if (!orig_neigh_node) + goto out; + + /* Update nc_nodes of the originator */ + batadv_nc_update_nc_node(bat_priv, orig_node, orig_neigh_node, + batadv_ogm_packet, is_single_hop_neigh); + + orig_neigh_router = batadv_orig_node_get_router(orig_neigh_node, + if_outgoing); + + /* drop packet if sender is not a direct neighbor and if we + * don't route towards it + */ + if (!is_single_hop_neigh && (!orig_neigh_router)) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: OGM via unknown neighbor!\n"); + goto out_neigh; + } + + is_bidirect = batadv_iv_ogm_calc_tq(orig_node, orig_neigh_node, + batadv_ogm_packet, if_incoming, + if_outgoing); + + /* update ranking if it is not a duplicate or has the same + * seqno and similar ttl as the non-duplicate + */ + rcu_read_lock(); + orig_node_ifinfo = batadv_orig_node_get_ifinfo(orig_node, if_outgoing); + if (!orig_node_ifinfo) { + rcu_read_unlock(); + goto out_neigh; + } + + sameseq = (orig_node_ifinfo->last_real_seqno == + ntohl(batadv_ogm_packet->seqno)); + similar_ttl = (orig_node_ifinfo->last_ttl - 3) <= + batadv_ogm_packet->header.ttl; + if (is_bidirect && ((dup_status == BATADV_NO_DUP) || + (sameseq && similar_ttl))) + batadv_iv_ogm_orig_update(bat_priv, orig_node, + orig_node_ifinfo, ethhdr, + batadv_ogm_packet, if_incoming, + if_outgoing, tt_buff, dup_status); + rcu_read_unlock(); + + /* is single hop (direct) neighbor */ + if (is_single_hop_neigh) { + if ((batadv_ogm_packet->header.ttl <= 2) && + (if_incoming != if_outgoing)) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: OGM from secondary interface and wrong outgoing interface\n"); + goto out_neigh; + } + /* mark direct link on incoming interface */ + batadv_iv_ogm_forward(orig_node, ethhdr, batadv_ogm_packet, + is_single_hop_neigh, + is_from_best_next_hop, if_incoming); + + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Forwarding packet: rebroadcast neighbor packet with direct link flag\n"); + goto out_neigh; + } + + /* multihop originator */ + if (!is_bidirect) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: not received via bidirectional link\n"); + goto out_neigh; + } + + if (dup_status == BATADV_NEIGH_DUP) { + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Drop packet: duplicate packet received\n"); + goto out_neigh; + } + + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "Forwarding packet: rebroadcast originator packet\n"); + batadv_iv_ogm_forward(orig_node, ethhdr, batadv_ogm_packet, + is_single_hop_neigh, is_from_best_next_hop, + if_incoming); + +out_neigh: + if ((orig_neigh_node) && (!is_single_hop_neigh)) + batadv_orig_node_free_ref(orig_neigh_node); +out: + if (router) + batadv_neigh_node_free_ref(router); + if (router_router) + batadv_neigh_node_free_ref(router_router); + if (orig_neigh_router) + batadv_neigh_node_free_ref(orig_neigh_router); + + memcpy(batadv_ogm_packet, &ogm_packet_backup, + sizeof(ogm_packet_backup)); +} + +/** + * batadv_iv_ogm_process - process an incoming batman iv OGM + * @ethhdr: the original ethernet header of the sender + * @batadv_ogm_packet: pointer to the ogm packet + * @tt_buff: pointer to the tt buffer + * @if_incoming: the interface where this packet was receved + */ +static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, + struct batadv_ogm_packet *batadv_ogm_packet, + const unsigned char *tt_buff, + struct batadv_hard_iface *if_incoming) +{ + 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; + uint32_t if_incoming_seqno; + int has_directlink_flag; + int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0;
/* Silently drop when the batman packet is actually not a * correct packet. @@ -1324,9 +1524,6 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, else has_directlink_flag = 0;
- if (batadv_compare_eth(ethhdr->h_source, batadv_ogm_packet->orig)) - is_single_hop_neigh = true; - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Received BATMAN packet via NB: %pM, IF: %s [%pM] (from OG: %pM, via prev OG: %pM, seqno %u, tq %d, TTL %d, V %d, IDF %d)\n", ethhdr->h_source, if_incoming->net_dev->name, @@ -1422,129 +1619,24 @@ static void batadv_iv_ogm_process(const struct ethhdr *ethhdr, if (!orig_node) return;
- dup_status = batadv_iv_ogm_update_seqnos(ethhdr, batadv_ogm_packet, - if_incoming, NULL); - - if (dup_status == BATADV_PROTECTED) { - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "Drop packet: packet within seqno protection time (sender: %pM)\n", - ethhdr->h_source); - goto out; - } - - if (batadv_ogm_packet->tq == 0) { - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "Drop packet: originator packet with tq equal 0\n"); - goto out; - } - - router = batadv_orig_node_get_router(orig_node); - if (router) { - orig_node_tmp = router->orig_node; - router_router = batadv_orig_node_get_router(orig_node_tmp); - } - - if ((router && router_ifinfo->bat_iv.tq_avg != 0) && - (batadv_compare_eth(router->addr, ethhdr->h_source))) - is_from_best_next_hop = true; - - prev_sender = batadv_ogm_packet->prev_sender; - /* avoid temporary routing loops */ - if (router && router_router && - (batadv_compare_eth(router->addr, prev_sender)) && - !(batadv_compare_eth(batadv_ogm_packet->orig, prev_sender)) && - (batadv_compare_eth(router->addr, router_router->addr))) { - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "Drop packet: ignoring all rebroadcast packets that may make me loop (sender: %pM)\n", - ethhdr->h_source); - goto out; - } - batadv_tvlv_ogm_receive(bat_priv, batadv_ogm_packet, orig_node);
- /* if sender is a direct neighbor the sender mac equals - * originator mac - */ - if (is_single_hop_neigh) - orig_neigh_node = orig_node; - else - orig_neigh_node = batadv_iv_ogm_orig_get(bat_priv, - ethhdr->h_source); + batadv_iv_ogm_process_per_outif(ethhdr, orig_node, batadv_ogm_packet, + tt_buff, if_incoming, NULL);
- if (!orig_neigh_node) - goto out; + rcu_read_lock(); + list_for_each_entry_rcu(hard_iface, &batadv_hardif_list, list) { + if (hard_iface->if_status != BATADV_IF_ACTIVE) + continue;
- /* Update nc_nodes of the originator */ - batadv_nc_update_nc_node(bat_priv, orig_node, orig_neigh_node, - batadv_ogm_packet, is_single_hop_neigh); + if (hard_iface->soft_iface != bat_priv->soft_iface) + continue;
- orig_neigh_router = batadv_orig_node_get_router(orig_neigh_node); - - /* drop packet if sender is not a direct neighbor and if we - * don't route towards it - */ - if (!is_single_hop_neigh && (!orig_neigh_router)) { - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "Drop packet: OGM via unknown neighbor!\n"); - goto out_neigh; - } - - is_bidirect = batadv_iv_ogm_calc_tq(orig_node, orig_neigh_node, - batadv_ogm_packet, if_incoming, - NULL); - - /* update ranking if it is not a duplicate or has the same - * seqno and similar ttl as the non-duplicate - */ - sameseq = orig_node->last_real_seqno == ntohl(batadv_ogm_packet->seqno); - similar_ttl = orig_node->last_ttl - 3 <= batadv_ogm_packet->header.ttl; - if (is_bidirect && ((dup_status == BATADV_NO_DUP) || - (sameseq && similar_ttl))) - batadv_iv_ogm_orig_update(bat_priv, orig_node, ethhdr, - batadv_ogm_packet, if_incoming, - NULL, tt_buff, dup_status); - - /* is single hop (direct) neighbor */ - if (is_single_hop_neigh) { - /* mark direct link on incoming interface */ - batadv_iv_ogm_forward(orig_node, ethhdr, batadv_ogm_packet, - is_single_hop_neigh, - is_from_best_next_hop, if_incoming); - - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "Forwarding packet: rebroadcast neighbor packet with direct link flag\n"); - goto out_neigh; + batadv_iv_ogm_process_per_outif(ethhdr, orig_node, + batadv_ogm_packet, tt_buff, + if_incoming, hard_iface); } - - /* multihop originator */ - if (!is_bidirect) { - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "Drop packet: not received via bidirectional link\n"); - goto out_neigh; - } - - if (dup_status == BATADV_NEIGH_DUP) { - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "Drop packet: duplicate packet received\n"); - goto out_neigh; - } - - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "Forwarding packet: rebroadcast originator packet\n"); - batadv_iv_ogm_forward(orig_node, ethhdr, batadv_ogm_packet, - is_single_hop_neigh, is_from_best_next_hop, - if_incoming); - -out_neigh: - if ((orig_neigh_node) && (!is_single_hop_neigh)) - batadv_orig_node_free_ref(orig_neigh_node); -out: - if (router) - batadv_neigh_node_free_ref(router); - if (router_router) - batadv_neigh_node_free_ref(router_router); - if (orig_neigh_router) - batadv_neigh_node_free_ref(orig_neigh_router); + rcu_read_unlock();
batadv_orig_node_free_ref(orig_node); } @@ -1625,7 +1717,8 @@ static void batadv_iv_ogm_orig_print(struct batadv_priv *bat_priv,
rcu_read_lock(); hlist_for_each_entry_rcu(orig_node, head, hash_entry) { - neigh_node = batadv_orig_node_get_router(orig_node); + neigh_node = batadv_orig_node_get_router(orig_node, + NULL); if (!neigh_node) continue;
diff --git a/distributed-arp-table.c b/distributed-arp-table.c index 6c8c393..411189b 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -591,7 +591,8 @@ static bool batadv_dat_send_data(struct batadv_priv *bat_priv, if (cand[i].type == BATADV_DAT_CANDIDATE_NOT_FOUND) continue;
- neigh_node = batadv_orig_node_get_router(cand[i].orig_node); + neigh_node = batadv_orig_node_get_router(cand[i].orig_node, + NULL); if (!neigh_node) goto free_orig;
diff --git a/gateway_client.c b/gateway_client.c index b845ee6..53bf325 100644 --- a/gateway_client.c +++ b/gateway_client.c @@ -131,7 +131,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv) continue;
orig_node = gw_node->orig_node; - router = batadv_orig_node_get_router(orig_node); + router = batadv_orig_node_get_router(orig_node, NULL); if (!router) continue;
@@ -244,7 +244,7 @@ void batadv_gw_election(struct batadv_priv *bat_priv) if (next_gw) { sprintf(gw_addr, "%pM", next_gw->orig_node->orig);
- router = batadv_orig_node_get_router(next_gw->orig_node); + router = batadv_orig_node_get_router(next_gw->orig_node, NULL); if (!router) { batadv_gw_deselect(bat_priv); goto out; @@ -312,7 +312,7 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv, if (!curr_gw_orig) goto deselect;
- router_gw = batadv_orig_node_get_router(curr_gw_orig); + router_gw = batadv_orig_node_get_router(curr_gw_orig, NULL); if (!router_gw) goto deselect;
@@ -324,7 +324,7 @@ void batadv_gw_check_election(struct batadv_priv *bat_priv, if (curr_gw_orig == orig_node) goto out;
- router_orig = batadv_orig_node_get_router(orig_node); + router_orig = batadv_orig_node_get_router(orig_node, NULL); if (!router_orig) goto out;
@@ -552,7 +552,7 @@ static int batadv_write_buffer_text(struct batadv_priv *bat_priv, int ret = -1;
rcu_read_lock(); - router = batadv_orig_node_get_router(gw_node->orig_node); + router = batadv_orig_node_get_router(gw_node->orig_node, NULL); if (!router) goto out;
diff --git a/icmp_socket.c b/icmp_socket.c index 82ac647..efb51f6 100644 --- a/icmp_socket.c +++ b/icmp_socket.c @@ -223,7 +223,7 @@ static ssize_t batadv_socket_write(struct file *file, const char __user *buff, if (!orig_node) goto dst_unreach;
- neigh_node = batadv_orig_node_get_router(orig_node); + neigh_node = batadv_orig_node_get_router(orig_node, NULL); if (!neigh_node) goto dst_unreach;
diff --git a/network-coding.c b/network-coding.c index 173a96e..6d1b659 100644 --- a/network-coding.c +++ b/network-coding.c @@ -1010,12 +1010,17 @@ static bool batadv_nc_code_packets(struct batadv_priv *bat_priv, int coded_size = sizeof(*coded_packet); int header_add = coded_size - unicast_size;
- router_neigh = batadv_orig_node_get_router(neigh_node->orig_node); + /* TODO: do we need to consider the outgoing interface for + * coded packets? + */ + router_neigh = batadv_orig_node_get_router(neigh_node->orig_node, + NULL); if (!router_neigh) goto out;
neigh_tmp = nc_packet->neigh_node; - router_coding = batadv_orig_node_get_router(neigh_tmp->orig_node); + router_coding = batadv_orig_node_get_router(neigh_tmp->orig_node, + NULL); if (!router_coding) goto out;
diff --git a/originator.c b/originator.c index 52cd618..8ed5993 100644 --- a/originator.c +++ b/originator.c @@ -174,14 +174,28 @@ void batadv_neigh_node_free_ref(struct batadv_neigh_node *neigh_node) call_rcu(&neigh_node->rcu, batadv_neigh_node_free_rcu); }
-/* increases the refcounter of a found router */ +/** + * batadv_orig_node_get_router - router to the originator depending on iface + * @orig_node: the orig node for the router + * @if_received: the interface where the packet to be transmitted was received + * Returns: the neighbor which should be router for this orig_node/iface + */ struct batadv_neigh_node * -batadv_orig_node_get_router(struct batadv_orig_node *orig_node) +batadv_orig_node_get_router(struct batadv_orig_node *orig_node, + const struct batadv_hard_iface *if_received) { - struct batadv_neigh_node *router; + struct batadv_orig_node_ifinfo *orig_node_ifinfo; + struct batadv_neigh_node *router = NULL;
rcu_read_lock(); - router = rcu_dereference(orig_node->router); + hlist_for_each_entry_rcu(orig_node_ifinfo, + &orig_node->ifinfo_list, list) { + if (orig_node_ifinfo->if_outgoing != if_received) + continue; + + router = rcu_dereference(orig_node_ifinfo->router); + break; + }
if (router && !atomic_inc_not_zero(&router->refcount)) router = NULL; @@ -191,6 +205,54 @@ batadv_orig_node_get_router(struct batadv_orig_node *orig_node) }
/** + * batadv_orig_node_get_ifinfo - gets the ifinfo from an orig_node + * @orig_node: the orig node to be queried + * @if_received: the interface for which the ifinfo should be acquired + * Returns: the requested orig_node_ifinfo or NULL if not found + * + * Note: this function must be called under rcu lock + */ +struct batadv_orig_node_ifinfo * +batadv_orig_node_get_ifinfo(struct batadv_orig_node *orig_node, + struct batadv_hard_iface *if_received) +{ + struct batadv_orig_node_ifinfo *tmp, *orig_ifinfo = NULL; + unsigned long reset_time; + + hlist_for_each_entry_rcu(tmp, &orig_node->ifinfo_list, + list) { + if (tmp->if_outgoing != if_received) + continue; + orig_ifinfo = tmp; + break; + } + + spin_lock_bh(&orig_node->neigh_list_lock); + if (!orig_ifinfo) { + orig_ifinfo = kzalloc(sizeof(*orig_ifinfo), GFP_ATOMIC); + if (!orig_ifinfo) + goto out; + + if (if_received && + !atomic_inc_not_zero(&if_received->refcount)) { + kfree(orig_ifinfo); + orig_ifinfo = NULL; + goto out; + } + reset_time = jiffies - 1; + reset_time -= msecs_to_jiffies(BATADV_RESET_PROTECTION_MS); + orig_ifinfo->batman_seqno_reset = reset_time; + orig_ifinfo->if_outgoing = if_received; + INIT_HLIST_NODE(&orig_ifinfo->list); + hlist_add_head_rcu(&orig_ifinfo->list, + &orig_node->ifinfo_list); + } +out: + spin_unlock_bh(&orig_node->neigh_list_lock); + return orig_ifinfo; +} + +/** * batadv_neigh_node_get_ifinfo - gets the ifinfo from an neigh_node * @neigh_node: the neigh node to be queried * @if_outgoing: the interface for which the ifinfo should be acquired @@ -274,11 +336,24 @@ out: return neigh_node; }
+static void batadv_orig_node_ifinfo_free_rcu(struct rcu_head *rcu) +{ + struct batadv_orig_node_ifinfo *orig_node_ifinfo; + + orig_node_ifinfo = container_of(rcu, struct batadv_orig_node_ifinfo, + rcu); + if (orig_node_ifinfo->if_outgoing) + batadv_hardif_free_ref(orig_node_ifinfo->if_outgoing); + + kfree(orig_node_ifinfo); +} + static void batadv_orig_node_free_rcu(struct rcu_head *rcu) { struct hlist_node *node_tmp; struct batadv_neigh_node *neigh_node; struct batadv_orig_node *orig_node; + struct batadv_orig_node_ifinfo *orig_node_ifinfo;
orig_node = container_of(rcu, struct batadv_orig_node, rcu);
@@ -291,6 +366,12 @@ static void batadv_orig_node_free_rcu(struct rcu_head *rcu) batadv_neigh_node_free_ref(neigh_node); }
+ hlist_for_each_entry_safe(orig_node_ifinfo, node_tmp, + &orig_node->ifinfo_list, list) { + hlist_del_rcu(&orig_node_ifinfo->list); + call_rcu(&orig_node_ifinfo->rcu, + batadv_orig_node_ifinfo_free_rcu); + } spin_unlock_bh(&orig_node->neigh_list_lock);
/* Free nc_nodes */ @@ -388,6 +469,7 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv,
INIT_HLIST_HEAD(&orig_node->neigh_list); INIT_LIST_HEAD(&orig_node->vlan_list); + INIT_HLIST_HEAD(&orig_node->ifinfo_list); spin_lock_init(&orig_node->bcast_seqno_lock); spin_lock_init(&orig_node->neigh_list_lock); spin_lock_init(&orig_node->tt_buff_lock); @@ -403,13 +485,11 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv, orig_node->bat_priv = bat_priv; memcpy(orig_node->orig, addr, ETH_ALEN); batadv_dat_init_orig_node_addr(orig_node); - orig_node->router = NULL; atomic_set(&orig_node->last_ttvn, 0); orig_node->tt_buff = NULL; orig_node->tt_buff_len = 0; reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS); orig_node->bcast_seqno_reset = reset_time; - orig_node->batman_seqno_reset = reset_time;
/* create a vlan object for the "untagged" LAN */ vlan = batadv_orig_node_vlan_new(orig_node, BATADV_NO_FLAGS); @@ -434,6 +514,53 @@ free_orig_node: }
/** + * batadv_purge_orig_ifinfo - purge ifinfo entries from originator + * @bat_priv: the bat priv with all the soft interface information + * @orig_node: orig node which is to be checked + * Returns: true if any ifinfo entry was purged, false otherwise + */ +static bool +batadv_purge_orig_ifinfo(struct batadv_priv *bat_priv, + struct batadv_orig_node *orig_node) +{ + struct hlist_node *node_tmp; + struct batadv_orig_node_ifinfo *orig_node_ifinfo; + bool ifinfo_purged = false; + struct batadv_hard_iface *if_outgoing; + + rcu_read_lock(); + spin_lock_bh(&orig_node->neigh_list_lock); + + /* for all neighbors towards this originator ... */ + hlist_for_each_entry_safe(orig_node_ifinfo, node_tmp, + &orig_node->ifinfo_list, list) { + if_outgoing = orig_node_ifinfo->if_outgoing; + if (!if_outgoing) + continue; + + if ((if_outgoing->if_status != BATADV_IF_INACTIVE) && + (if_outgoing->if_status != BATADV_IF_NOT_IN_USE) && + (if_outgoing->if_status != BATADV_IF_TO_BE_REMOVED)) + continue; + + batadv_dbg(BATADV_DBG_BATMAN, bat_priv, + "router/ifinfo purge: originator %pM, iface: %s\n", + orig_node->orig, if_outgoing->net_dev->name); + + ifinfo_purged = true; + + hlist_del_rcu(&orig_node_ifinfo->list); + call_rcu(&orig_node_ifinfo->rcu, + batadv_orig_node_ifinfo_free_rcu); + } + + spin_unlock_bh(&orig_node->neigh_list_lock); + rcu_read_unlock(); + return ifinfo_purged; +} + + +/** * batadv_purge_orig_neighbors - purges neighbors from originator * @bat_priv: the bat priv with all the soft interface information * @orig_node: orig node which is to be checked @@ -516,6 +643,8 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node) { struct batadv_neigh_node *best_neigh_node; + struct batadv_hard_iface *hard_iface; + bool changed = false;
if (batadv_has_timed_out(orig_node->last_seen, 2 * BATADV_PURGE_TIMEOUT)) { @@ -525,11 +654,32 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv, jiffies_to_msecs(orig_node->last_seen)); return true; } - if (!batadv_purge_orig_neighbors(bat_priv, orig_node)) + changed = changed || batadv_purge_orig_ifinfo(bat_priv, orig_node); + changed = changed || batadv_purge_orig_neighbors(bat_priv, orig_node); + + if (!changed) return false;
+ /* first for NULL ... */ best_neigh_node = batadv_find_best_neighbor(bat_priv, orig_node, NULL); - batadv_update_route(bat_priv, orig_node, best_neigh_node); + batadv_update_route(bat_priv, orig_node, NULL, best_neigh_node); + + /* ... then for all other interfaces. */ + rcu_read_lock(); + list_for_each_entry_rcu(hard_iface, &batadv_hardif_list, list) { + if (hard_iface->if_status != BATADV_IF_ACTIVE) + continue; + + if (hard_iface->soft_iface != bat_priv->soft_iface) + continue; + + best_neigh_node = batadv_find_best_neighbor(bat_priv, + orig_node, + hard_iface); + batadv_update_route(bat_priv, orig_node, hard_iface, + best_neigh_node); + } + rcu_read_unlock();
return false; } diff --git a/originator.h b/originator.h index 161c1e3..1dce5b4 100644 --- a/originator.h +++ b/originator.h @@ -36,7 +36,16 @@ batadv_neigh_node_new(struct batadv_hard_iface *hard_iface, 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); +batadv_orig_node_get_router(struct batadv_orig_node *orig_node, + const struct batadv_hard_iface *if_received); +void +batadv_orig_node_set_router(struct batadv_orig_node *orig_node, + struct batadv_hard_iface *if_received, + struct batadv_neigh_node *router); +struct batadv_orig_node_ifinfo * +batadv_orig_node_get_ifinfo(struct batadv_orig_node *orig_node, + struct batadv_hard_iface *if_received); + struct batadv_neigh_node_ifinfo * batadv_neigh_node_get_ifinfo(struct batadv_neigh_node *neigh, struct batadv_hard_iface *if_outgoing); diff --git a/routing.c b/routing.c index 733ebbf..304e44b 100644 --- a/routing.c +++ b/routing.c @@ -35,13 +35,35 @@ static int batadv_route_unicast_packet(struct sk_buff *skb, struct batadv_hard_iface *recv_if);
+/* _batadv_update_route - set the router for this originator + * @bat_priv: the bat priv with all the soft interface information + * @orig_node: orig node which is to be configured + * @recv_if: the receive interface for which this route is set + * @neigh_node: neighbor which should be the next router + * + * This function does not perform any error checks + */ static void _batadv_update_route(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, + struct batadv_hard_iface *recv_if, struct batadv_neigh_node *neigh_node) { + struct batadv_orig_node_ifinfo *orig_node_ifinfo; struct batadv_neigh_node *curr_router;
- curr_router = batadv_orig_node_get_router(orig_node); + /* use batadv_orig_node_get_ifinfo() instead of + * batadv_orig_node_get_router() to re-use the ifinfo below. + */ + rcu_read_lock(); + orig_node_ifinfo = batadv_orig_node_get_ifinfo(orig_node, recv_if); + if (orig_node_ifinfo) { + curr_router = rcu_dereference(orig_node_ifinfo->router); + if (curr_router && !atomic_inc_not_zero(&curr_router->refcount)) + curr_router = NULL; + } else { + rcu_read_unlock(); + return; + }
/* route deleted */ if ((curr_router) && (!neigh_node)) { @@ -71,16 +93,26 @@ static void _batadv_update_route(struct batadv_priv *bat_priv, neigh_node = NULL;
spin_lock_bh(&orig_node->neigh_list_lock); - rcu_assign_pointer(orig_node->router, neigh_node); + rcu_assign_pointer(orig_node_ifinfo->router, neigh_node); spin_unlock_bh(&orig_node->neigh_list_lock);
+ rcu_read_unlock(); + /* decrease refcount of previous best neighbor */ if (curr_router) batadv_neigh_node_free_ref(curr_router); }
+/** + * batadv_update_route - set the router for this originator + * @bat_priv: the bat priv with all the soft interface information + * @orig_node: orig node which is to be configured + * @recv_if: the receive interface for which this route is set + * @neigh_node: neighbor which should be the next router + */ void batadv_update_route(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, + struct batadv_hard_iface *recv_if, struct batadv_neigh_node *neigh_node) { struct batadv_neigh_node *router = NULL; @@ -88,10 +120,10 @@ void batadv_update_route(struct batadv_priv *bat_priv, if (!orig_node) goto out;
- router = batadv_orig_node_get_router(orig_node); + router = batadv_orig_node_get_router(orig_node, recv_if);
if (router != neigh_node) - _batadv_update_route(bat_priv, orig_node, neigh_node); + _batadv_update_route(bat_priv, orig_node, recv_if, neigh_node);
out: if (router) @@ -382,7 +414,7 @@ batadv_find_router(struct batadv_priv *bat_priv, if (!orig_node) return NULL;
- router = batadv_orig_node_get_router(orig_node); + router = batadv_orig_node_get_router(orig_node, recv_if);
/* TODO: fill this later with new bonding mechanism */
diff --git a/routing.h b/routing.h index b8fed80..7a7c6e9 100644 --- a/routing.h +++ b/routing.h @@ -25,6 +25,7 @@ bool batadv_check_management_packet(struct sk_buff *skb, int header_len); void batadv_update_route(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, + struct batadv_hard_iface *recv_if, struct batadv_neigh_node *neigh_node); int batadv_recv_icmp_packet(struct sk_buff *skb, struct batadv_hard_iface *recv_if); diff --git a/translation-table.c b/translation-table.c index ea4c65d..e718da5 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1356,7 +1356,8 @@ batadv_transtable_best_orig(struct batadv_priv *bat_priv,
head = &tt_global_entry->orig_list; hlist_for_each_entry_rcu(orig_entry, head, list) { - router = batadv_orig_node_get_router(orig_entry->orig_node); + router = batadv_orig_node_get_router(orig_entry->orig_node, + NULL); if (!router) continue;
diff --git a/types.h b/types.h index e830e20..9d221a8 100644 --- a/types.h +++ b/types.h @@ -78,6 +78,25 @@ struct batadv_hard_iface { struct work_struct cleanup_work; };
+/* struct batadv_orig_node_ifinfo - originator info per outgoing interface + * @list: list node for orig_node::ifinfo_list + * @if_outgoing: pointer to outgoing hard interface + * @router: router that should be used to reach this originator + * @last_real_seqno: last and best known sequence number + * @last_ttl: ttl of last received packet + * @batman_seqno_reset: time when the batman seqno window was reset + * @rcu: struct used for freeing in an RCU-safe manner + */ +struct batadv_orig_node_ifinfo { + struct hlist_node list; + struct batadv_hard_iface *if_outgoing; + struct batadv_neigh_node __rcu *router; /* rcu protected pointer */ + uint32_t last_real_seqno; + uint8_t last_ttl; + unsigned long batman_seqno_reset; + struct rcu_head rcu; +}; + /** * struct batadv_frag_table_entry - head in the fragment buffer table * @head: head of list with fragments @@ -153,11 +172,10 @@ struct batadv_orig_bat_iv { * 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 + * @ifinfo_list: list for routers per outgoing interface * @batadv_dat_addr_t: address of the orig node in the distributed hash * @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 * @capabilities: announced capabilities of this originator * @last_ttvn: last seen translation table version number * @tt_buff: last tt changeset this node received from the orig node @@ -170,8 +188,6 @@ struct batadv_orig_bat_iv { * made up by two operations (data structure update and metdata -CRC/TTVN- * recalculation) and they have to be executed atomically in order to avoid * another thread to read the table/metadata between those. - * @last_real_seqno: last and best known sequence number - * @last_ttl: ttl of last received packet * @bcast_bits: bitfield containing the info which payload broadcast originated * from this orig node this host already has seen (relative to * last_bcast_seqno) @@ -196,13 +212,12 @@ struct batadv_orig_bat_iv { struct batadv_orig_node { uint8_t orig[ETH_ALEN]; uint8_t primary_addr[ETH_ALEN]; - struct batadv_neigh_node __rcu *router; /* rcu protected pointer */ + struct hlist_head ifinfo_list; #ifdef CONFIG_BATMAN_ADV_DAT batadv_dat_addr_t dat_addr; #endif unsigned long last_seen; unsigned long bcast_seqno_reset; - unsigned long batman_seqno_reset; uint8_t capabilities; atomic_t last_ttvn; unsigned char *tt_buff; @@ -211,8 +226,6 @@ struct batadv_orig_node { bool tt_initialised; /* prevents from changing the table while reading it */ spinlock_t tt_lock; - uint32_t last_real_seqno; - uint8_t last_ttl; DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE); uint32_t last_bcast_seqno; struct hlist_head neigh_list;
On Wed, Sep 04, 2013 at 06:27:37PM +0200, Simon Wunderlich wrote:
From: Simon Wunderlich simon@open-mesh.com
For the network wide multi interface optimization there are different routers for each outgoing interface (outgoing from the OGM perspective, incoming for payload traffic). To reflect this, change the router and associated data to a list of routers.
Signed-off-by: Simon Wunderlich simon@open-mesh.com
[...]
+static void +batadv_iv_ogm_process_per_outif(const struct ethhdr *ethhdr,
struct batadv_orig_node *orig_node,
struct batadv_ogm_packet *batadv_ogm_packet,
const unsigned char *tt_buff,
struct batadv_hard_iface *if_incoming,
struct batadv_hard_iface *if_outgoing)
{ 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, *orig_node_tmp; struct batadv_neigh_node *router = NULL, *router_router = NULL; struct batadv_neigh_node *orig_neigh_router = NULL; struct batadv_neigh_node_ifinfo *router_ifinfo = NULL;
- int has_directlink_flag;
- int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0;
- int is_bidirect;
- bool is_single_hop_neigh = false;
- bool is_from_best_next_hop = false;
- int sameseq, similar_ttl;
- struct batadv_orig_node *orig_neigh_node, *orig_node_tmp;
- struct batadv_orig_node_ifinfo *orig_node_ifinfo;
- int is_bidirect, sameseq, similar_ttl; enum batadv_dup_status dup_status;
- uint32_t if_incoming_seqno; uint8_t *prev_sender;
- struct batadv_ogm_packet ogm_packet_backup;
- bool is_from_best_next_hop = false;
- bool is_single_hop_neigh = false;
Since you have the opportunity, please sort these new declarations by line length (like you have done 6 lines above).
[...]
- sameseq = orig_node_ifinfo->last_real_seqno ==
ntohl(batadv_ogm_packet->seqno));
- similar_ttl = (orig_node_ifinfo->last_ttl - 3) <=
batadv_ogm_packet->header.ttl;
you know this is very ugly, right? :-)
you could try to remove sameseq and similar_ttl and use temporary variables for last_real_seqno, seqno, last_ttl and header.ttl.....
- if (is_bidirect && ((dup_status == BATADV_NO_DUP) ||
(sameseq && similar_ttl)))
...then you can try to make them fit this 'if' directly.
batadv_iv_ogm_orig_update(bat_priv, orig_node,
orig_node_ifinfo, ethhdr,
batadv_ogm_packet, if_incoming,
if_outgoing, tt_buff, dup_status);
- rcu_read_unlock();
[..]
+static void batadv_iv_ogm_process(const struct ethhdr *ethhdr,
struct batadv_ogm_packet *batadv_ogm_packet,
const unsigned char *tt_buff,
struct batadv_hard_iface *if_incoming)
+{
- 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;
- uint32_t if_incoming_seqno;
- int has_directlink_flag;
- int is_my_addr = 0, is_my_orig = 0, is_my_oldorig = 0;
Please sort the declarations by line length.
[...]
diff --git a/network-coding.c b/network-coding.c index 173a96e..6d1b659 100644 --- a/network-coding.c +++ b/network-coding.c @@ -1010,12 +1010,17 @@ static bool batadv_nc_code_packets(struct batadv_priv *bat_priv, int coded_size = sizeof(*coded_packet); int header_add = coded_size - unicast_size;
- router_neigh = batadv_orig_node_get_router(neigh_node->orig_node);
- /* TODO: do we need to consider the outgoing interface for
* coded packets?
*/
- router_neigh = batadv_orig_node_get_router(neigh_node->orig_node,
NULL);
Perhaps we can consider them like "locally-generated". I think we should ask Martin to give us his feedback :-)
[...]
/**
- batadv_orig_node_get_ifinfo - gets the ifinfo from an orig_node
- @orig_node: the orig node to be queried
- @if_received: the interface for which the ifinfo should be acquired
Put a blank line here.
- Returns: the requested orig_node_ifinfo or NULL if not found
we never use the ':' after Returns (and a . is missing at the end of the sentence). Please make sure that all the kernel doc you added respect those guidelines.
[...]
+static void batadv_orig_node_ifinfo_free_rcu(struct rcu_head *rcu)
kernel doc?
+{
- struct batadv_orig_node_ifinfo *orig_node_ifinfo;
- orig_node_ifinfo = container_of(rcu, struct batadv_orig_node_ifinfo,
rcu);
- if (orig_node_ifinfo->if_outgoing)
batadv_hardif_free_ref(orig_node_ifinfo->if_outgoing);
Please, be sure this is not creating any race condition during the cleanup phase. We had problems int the past with a RCU callback scheduling another callback too.
The cleanup code has an rcu_barrier() to ensure everything is done and then safely free the various structures. What you do here may violate this assumption and the scheduled callback may generate a General Protection Fault.
[...]
@@ -291,6 +366,12 @@ static void batadv_orig_node_free_rcu(struct rcu_head *rcu) batadv_neigh_node_free_ref(neigh_node); }
- hlist_for_each_entry_safe(orig_node_ifinfo, node_tmp,
&orig_node->ifinfo_list, list) {
hlist_del_rcu(&orig_node_ifinfo->list);
call_rcu(&orig_node_ifinfo->rcu,
batadv_orig_node_ifinfo_free_rcu);
Same here. Please ensure everything is ok when scheduling a callback from another callback.
[...]
/**
- batadv_purge_orig_ifinfo - purge ifinfo entries from originator
- @bat_priv: the bat priv with all the soft interface information
- @orig_node: orig node which is to be checked
- Returns: true if any ifinfo entry was purged, false otherwise
- */
+static bool +batadv_purge_orig_ifinfo(struct batadv_priv *bat_priv,
struct batadv_orig_node *orig_node)
+{
- struct hlist_node *node_tmp;
- struct batadv_orig_node_ifinfo *orig_node_ifinfo;
- bool ifinfo_purged = false;
- struct batadv_hard_iface *if_outgoing;
sort by line length please.
- rcu_read_lock();
- spin_lock_bh(&orig_node->neigh_list_lock);
why do you need both the rcu and the neigh_list_lock at the same time? You acquire and release the two in the same moment (maybe I am overlooking something?)
[...]
+/**
- batadv_purge_orig_neighbors - purges neighbors from originator
- @bat_priv: the bat priv with all the soft interface information
- @orig_node: orig node which is to be checked
@@ -516,6 +643,8 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node) { struct batadv_neigh_node *best_neigh_node;
- struct batadv_hard_iface *hard_iface;
- bool changed = false;
this is useless and then.....
if (batadv_has_timed_out(orig_node->last_seen, 2 * BATADV_PURGE_TIMEOUT)) { @@ -525,11 +654,32 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv, jiffies_to_msecs(orig_node->last_seen)); return true; }
- if (!batadv_purge_orig_neighbors(bat_priv, orig_node))
- changed = changed || batadv_purge_orig_ifinfo(bat_priv, orig_node);
....just do:
changed = batadv_purge_orig_ifinfo(...);
[...]
diff --git a/originator.h b/originator.h index 161c1e3..1dce5b4 100644 --- a/originator.h +++ b/originator.h @@ -36,7 +36,16 @@ batadv_neigh_node_new(struct batadv_hard_iface *hard_iface, 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); +batadv_orig_node_get_router(struct batadv_orig_node *orig_node,
const struct batadv_hard_iface *if_received);
+void
why void on another line? It should fit in the next one.
+batadv_orig_node_set_router(struct batadv_orig_node *orig_node,
struct batadv_hard_iface *if_received,
struct batadv_neigh_node *router);
Cheers,
From: Simon Wunderlich simon@open-mesh.com
If the same interface is used for sending and receiving, there might be throughput degradation on half-duplex interfaces such as WiFi. Add a penalty if the same interface is used to reflect this problem in the metric. At the same time, change the hop penalty from 30 to 15 so there will be no change for single wifi mesh network. the effective hop penalty will stay at 30 due to the new wifi penalty for these networks.
Signed-off-by: Simon Wunderlich simon@open-mesh.com --- Changes to RFCv1: * use hop penalty for wifi penalty, and use half of the original hop penalty. --- bat_iv_ogm.c | 27 +++++++++++++++++++++++---- hard-interface.c | 2 +- hard-interface.h | 1 + soft-interface.c | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index d32b2f2..efddadf 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1085,6 +1085,7 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, 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, if_num, ret = 0; + int tq_iface_penalty; unsigned int combined_tq;
/* find corresponding one hop neighbor */ @@ -1169,15 +1170,33 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, inv_asym_penalty /= neigh_rq_max_cube; tq_asym_penalty = BATADV_TQ_MAX_VALUE - inv_asym_penalty;
- combined_tq = batadv_ogm_packet->tq * tq_own * tq_asym_penalty; - combined_tq /= BATADV_TQ_MAX_VALUE * BATADV_TQ_MAX_VALUE; + /* penalize if the OGM is forwarded on the same interface. WiFi + * interfaces and other half duplex devices suffer from throughput + * drops as they can't send and receive at the same time. + */ + tq_iface_penalty = BATADV_TQ_MAX_VALUE; + if (if_outgoing && (if_incoming == if_outgoing) && + batadv_is_wifi_netdev(if_outgoing->net_dev)) + tq_iface_penalty = batadv_hop_penalty(BATADV_TQ_MAX_VALUE, + bat_priv); + + combined_tq = batadv_ogm_packet->tq * + tq_own * + tq_asym_penalty * + tq_iface_penalty; + combined_tq /= BATADV_TQ_MAX_VALUE * + BATADV_TQ_MAX_VALUE * + BATADV_TQ_MAX_VALUE; batadv_ogm_packet->tq = combined_tq;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "bidirectional: orig = %-15pM neigh = %-15pM => own_bcast = %2i, real recv = %2i, local tq: %3i, asym_penalty: %3i, total tq: %3i\n", + "bidirectional: orig = %-15pM neigh = %-15pM => own_bcast = %2i, real recv = %2i, local tq: %3i, asym_penalty: %3i, iface_penalty: %3i, total tq: %3i, if_incoming = %s, if_outgoing = %s\n", orig_node->orig, orig_neigh_node->orig, total_count, neigh_rq_count, tq_own, - tq_asym_penalty, batadv_ogm_packet->tq); + tq_asym_penalty, tq_iface_penalty, + batadv_ogm_packet->tq, + if_incoming ? if_incoming->net_dev->name : "NULL", + if_outgoing ? if_outgoing->net_dev->name : "NULL");
/* if link has the minimum required transmission quality * consider it bidirectional diff --git a/hard-interface.c b/hard-interface.c index c60d3ed..2a4b202 100644 --- a/hard-interface.c +++ b/hard-interface.c @@ -124,7 +124,7 @@ static int batadv_is_valid_iface(const struct net_device *net_dev) * * Returns true if the net device is a 802.11 wireless device, false otherwise. */ -static bool batadv_is_wifi_netdev(struct net_device *net_device) +bool batadv_is_wifi_netdev(struct net_device *net_device) { #ifdef CONFIG_WIRELESS_EXT /* pre-cfg80211 drivers have to implement WEXT, so it is possible to diff --git a/hard-interface.h b/hard-interface.h index 4989288..1a90ea5 100644 --- a/hard-interface.h +++ b/hard-interface.h @@ -52,6 +52,7 @@ int batadv_hardif_min_mtu(struct net_device *soft_iface); void batadv_update_min_mtu(struct net_device *soft_iface); void batadv_hardif_free_rcu(struct rcu_head *rcu); bool batadv_is_wifi_iface(int ifindex); +bool batadv_is_wifi_netdev(struct net_device *net_device);
static inline void batadv_hardif_free_ref(struct batadv_hard_iface *hard_iface) diff --git a/soft-interface.c b/soft-interface.c index a1f00e8..81c608d 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -672,7 +672,7 @@ static int batadv_softif_init_late(struct net_device *dev) atomic_set(&bat_priv->gw.bandwidth_down, 100); atomic_set(&bat_priv->gw.bandwidth_up, 20); atomic_set(&bat_priv->orig_interval, 1000); - atomic_set(&bat_priv->hop_penalty, 30); + atomic_set(&bat_priv->hop_penalty, 15); #ifdef CONFIG_BATMAN_ADV_DEBUG atomic_set(&bat_priv->log_level, 0); #endif
On Wed, Sep 04, 2013 at 06:27:38PM +0200, Simon Wunderlich wrote:
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index d32b2f2..efddadf 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -1085,6 +1085,7 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, 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, if_num, ret = 0;
- int tq_iface_penalty; unsigned int combined_tq;
please, add the new variable at the end (line length sorting).
/* find corresponding one hop neighbor */ @@ -1169,15 +1170,33 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node, inv_asym_penalty /= neigh_rq_max_cube; tq_asym_penalty = BATADV_TQ_MAX_VALUE - inv_asym_penalty;
- combined_tq = batadv_ogm_packet->tq * tq_own * tq_asym_penalty;
- combined_tq /= BATADV_TQ_MAX_VALUE * BATADV_TQ_MAX_VALUE;
- /* penalize if the OGM is forwarded on the same interface. WiFi
* interfaces and other half duplex devices suffer from throughput
* drops as they can't send and receive at the same time.
*/
- tq_iface_penalty = BATADV_TQ_MAX_VALUE;
- if (if_outgoing && (if_incoming == if_outgoing) &&
batadv_is_wifi_netdev(if_outgoing->net_dev))
tq_iface_penalty = batadv_hop_penalty(BATADV_TQ_MAX_VALUE,
bat_priv);
- combined_tq = batadv_ogm_packet->tq *
tq_own *
tq_asym_penalty *
tq_iface_penalty;
- combined_tq /= BATADV_TQ_MAX_VALUE *
BATADV_TQ_MAX_VALUE *
BATADV_TQ_MAX_VALUE;
Mh..I am not sure about the style of these assignments...but we can live with those for now.
batadv_ogm_packet->tq = combined_tq;
batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
"bidirectional: orig = %-15pM neigh = %-15pM => own_bcast = %2i, real recv = %2i, local tq: %3i, asym_penalty: %3i, total tq: %3i\n",
orig_node->orig, orig_neigh_node->orig, total_count, neigh_rq_count, tq_own,"bidirectional: orig = %-15pM neigh = %-15pM => own_bcast = %2i, real recv = %2i, local tq: %3i, asym_penalty: %3i, iface_penalty: %3i, total tq: %3i, if_incoming = %s, if_outgoing = %s\n",
tq_asym_penalty, batadv_ogm_packet->tq);
tq_asym_penalty, tq_iface_penalty,
batadv_ogm_packet->tq,
this one can go on the line above..
if_incoming ? if_incoming->net_dev->name : "NULL",
if_outgoing ? if_outgoing->net_dev->name : "NULL");
/* if link has the minimum required transmission quality
- consider it bidirectional
Cheers,
From: Simon Wunderlich simon@open-mesh.com
The current OGM sending an aggregation functionality decides on which interfaces a packet should be sent when it parses the forward packet struct. However, with the network wide multi interface optimization the outgoing interface is decided by the OGM processing function.
This is reflected by moving the decision in the OGM processing function and add the outgoing interface in the forwarding packet struct. This practically implies that an OGM may be added multiple times (once per outgoing interface), and this also affects aggregation which needs to consider the outgoing interface as well.
Signed-off-by: Simon Wunderlich simon@open-mesh.com --- bat_iv_ogm.c | 133 +++++++++++++++++++++++++++++++++++++++------------------- send.c | 13 ++++-- types.h | 7 +++- 3 files changed, 104 insertions(+), 49 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index efddadf..349344f 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -468,7 +468,6 @@ static void batadv_iv_ogm_send_to_if(struct batadv_forw_packet *forw_packet, /* send a batman ogm packet */ static void batadv_iv_ogm_emit(struct batadv_forw_packet *forw_packet) { - struct batadv_hard_iface *hard_iface; struct net_device *soft_iface; struct batadv_priv *bat_priv; struct batadv_hard_iface *primary_if = NULL; @@ -488,6 +487,12 @@ static void batadv_iv_ogm_emit(struct batadv_forw_packet *forw_packet) soft_iface = forw_packet->if_incoming->soft_iface; bat_priv = netdev_priv(soft_iface);
+ if (WARN_ON(!forw_packet->if_outgoing)) + goto out; + + if (WARN_ON(forw_packet->if_outgoing->soft_iface != soft_iface)) + goto out; + if (forw_packet->if_incoming->if_status != BATADV_IF_ACTIVE) goto out;
@@ -495,52 +500,35 @@ static void batadv_iv_ogm_emit(struct batadv_forw_packet *forw_packet) if (!primary_if) goto out;
- /* multihomed peer assumed - * non-primary OGMs are only broadcasted on their interface - */ - if ((directlink && (batadv_ogm_packet->header.ttl == 1)) || - (forw_packet->own && (forw_packet->if_incoming != primary_if))) { - /* FIXME: what about aggregated packets ? */ - batadv_dbg(BATADV_DBG_BATMAN, bat_priv, - "%s packet (originator %pM, seqno %u, TTL %d) on interface %s [%pM]\n", - (forw_packet->own ? "Sending own" : "Forwarding"), - batadv_ogm_packet->orig, - ntohl(batadv_ogm_packet->seqno), - batadv_ogm_packet->header.ttl, - forw_packet->if_incoming->net_dev->name, - forw_packet->if_incoming->net_dev->dev_addr); - - /* skb is only used once and than forw_packet is free'd */ - batadv_send_skb_packet(forw_packet->skb, - forw_packet->if_incoming, - batadv_broadcast_addr); - forw_packet->skb = NULL; - - goto out; - } - - /* broadcast on every interface */ - rcu_read_lock(); - list_for_each_entry_rcu(hard_iface, &batadv_hardif_list, list) { - if (hard_iface->soft_iface != soft_iface) - continue; - - batadv_iv_ogm_send_to_if(forw_packet, hard_iface); - } - rcu_read_unlock(); + /* only for one specific outgoing interface */ + batadv_iv_ogm_send_to_if(forw_packet, forw_packet->if_outgoing);
out: if (primary_if) batadv_hardif_free_ref(primary_if); }
-/* return true if new_packet can be aggregated with forw_packet */ +/** + * batadv_iv_ogm_can_aggregate - find out if an OGM can be aggregated on an + * existing forward packet + * @new_bat_ogm_packet: OGM packet to be aggregated + * @bat_priv: the bat priv with all the soft interface information + * @packet_len: (total) length of the OGM + * @send_time: timestamp (jiffies) when the packet is to be sent + * @direktlink: true if this is a direct link packet + * @if_incoming: interface where the packet was received + * @if_outgoing: interface for which the retransmission should be considered + * @forw_packet: the forwarded packet which should be checked + * + * Returns true if new_packet can be aggregated with forw_packet + */ static bool batadv_iv_ogm_can_aggregate(const struct batadv_ogm_packet *new_bat_ogm_packet, struct batadv_priv *bat_priv, int packet_len, unsigned long send_time, bool directlink, const struct batadv_hard_iface *if_incoming, + const struct batadv_hard_iface *if_outgoing, const struct batadv_forw_packet *forw_packet) { struct batadv_ogm_packet *batadv_ogm_packet; @@ -574,6 +562,13 @@ batadv_iv_ogm_can_aggregate(const struct batadv_ogm_packet *new_bat_ogm_packet, if (!primary_if) goto out;
+ /* packet is not leaving on the same interface. + * TODO: some other parts here could be reworked as the + * outgoing interface is specified now. + */ + if (forw_packet->if_outgoing != if_outgoing) + goto out; + /* packets without direct link flag and high TTL * are flooded through the net */ @@ -620,6 +615,7 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, int packet_len, unsigned long send_time, bool direct_link, struct batadv_hard_iface *if_incoming, + struct batadv_hard_iface *if_outgoing, int own_packet) { struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface); @@ -630,6 +626,9 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, if (!atomic_inc_not_zero(&if_incoming->refcount)) return;
+ if (!atomic_inc_not_zero(&if_outgoing->refcount)) + goto out_free_incoming; + /* own packet should always be scheduled */ if (!own_packet) { if (!batadv_atomic_dec_not_zero(&bat_priv->batman_queue_left)) { @@ -670,6 +669,7 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff,
forw_packet_aggr->own = own_packet; forw_packet_aggr->if_incoming = if_incoming; + forw_packet_aggr->if_outgoing = if_outgoing; forw_packet_aggr->num_packets = 0; forw_packet_aggr->direct_link_flags = BATADV_NO_FLAGS; forw_packet_aggr->send_time = send_time; @@ -692,6 +692,8 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff,
return; out: + batadv_hardif_free_ref(if_outgoing); +out_free_incoming: batadv_hardif_free_ref(if_incoming); }
@@ -715,10 +717,21 @@ static void batadv_iv_ogm_aggregate(struct batadv_forw_packet *forw_packet_aggr, } }
+/** + * batadv_iv_ogm_queue_add - queue up an OGM for transmission + * @bat_priv: the bat priv with all the soft interface information + * @packet_buff: pointer to the OGM + * @packet_len: (total) length of the OGM + * @if_incoming: interface where the packet was received + * @if_outgoing: interface for which the retransmission should be considered + * @own_packet: true if it is a self-generated ogm + * @send_time: timestamp (jiffies) when the packet is to be sent + */ static void batadv_iv_ogm_queue_add(struct batadv_priv *bat_priv, unsigned char *packet_buff, int packet_len, struct batadv_hard_iface *if_incoming, + struct batadv_hard_iface *if_outgoing, int own_packet, unsigned long send_time) { /* _aggr -> pointer to the packet we want to aggregate with @@ -744,6 +757,7 @@ static void batadv_iv_ogm_queue_add(struct batadv_priv *bat_priv, bat_priv, packet_len, send_time, direct_link, if_incoming, + if_outgoing, forw_packet_pos)) { forw_packet_aggr = forw_packet_pos; break; @@ -767,7 +781,8 @@ static void batadv_iv_ogm_queue_add(struct batadv_priv *bat_priv,
batadv_iv_ogm_aggregate_new(packet_buff, packet_len, send_time, direct_link, - if_incoming, own_packet); + if_incoming, if_outgoing, + own_packet); } else { batadv_iv_ogm_aggregate(forw_packet_aggr, packet_buff, packet_len, direct_link); @@ -780,7 +795,8 @@ static void batadv_iv_ogm_forward(struct batadv_orig_node *orig_node, struct batadv_ogm_packet *batadv_ogm_packet, bool is_single_hop_neigh, bool is_from_best_next_hop, - struct batadv_hard_iface *if_incoming) + struct batadv_hard_iface *if_incoming, + struct batadv_hard_iface *if_outgoing) { struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface); uint16_t tvlv_len; @@ -825,7 +841,8 @@ static void batadv_iv_ogm_forward(struct batadv_orig_node *orig_node,
batadv_iv_ogm_queue_add(bat_priv, (unsigned char *)batadv_ogm_packet, BATADV_OGM_HLEN + tvlv_len, - if_incoming, 0, batadv_iv_ogm_fwd_send_time()); + if_incoming, if_outgoing, 0, + batadv_iv_ogm_fwd_send_time()); }
/** @@ -870,10 +887,11 @@ static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface) struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface); unsigned char **ogm_buff = &hard_iface->bat_iv.ogm_buff; struct batadv_ogm_packet *batadv_ogm_packet; - struct batadv_hard_iface *primary_if; + struct batadv_hard_iface *primary_if, *tmp_hard_iface; int *ogm_buff_len = &hard_iface->bat_iv.ogm_buff_len; uint32_t seqno; uint16_t tvlv_len = 0; + unsigned long send_time;
primary_if = batadv_primary_if_get_selected(bat_priv);
@@ -896,9 +914,31 @@ static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface) atomic_inc(&hard_iface->bat_iv.ogm_seqno);
batadv_iv_ogm_slide_own_bcast_window(hard_iface); - batadv_iv_ogm_queue_add(bat_priv, hard_iface->bat_iv.ogm_buff, - hard_iface->bat_iv.ogm_buff_len, hard_iface, 1, - batadv_iv_ogm_emit_send_time(bat_priv)); + + send_time = batadv_iv_ogm_emit_send_time(bat_priv); + + if (hard_iface == primary_if) { + /* OGMs from primary interfaces are scheduled on all + * interfaces. + */ + rcu_read_lock(); + list_for_each_entry_rcu(tmp_hard_iface, &batadv_hardif_list, + list) { + if (tmp_hard_iface->soft_iface != + hard_iface->soft_iface) + continue; + batadv_iv_ogm_queue_add(bat_priv, *ogm_buff, + *ogm_buff_len, hard_iface, + tmp_hard_iface, 1, send_time); + } + rcu_read_unlock(); + } else { + /* OGMs from secondary interfaces are only scheduled on their + * respective interfaces. + */ + batadv_iv_ogm_queue_add(bat_priv, *ogm_buff, *ogm_buff_len, + hard_iface, hard_iface, 1, send_time); + }
if (primary_if) batadv_hardif_free_ref(primary_if); @@ -1449,6 +1489,10 @@ batadv_iv_ogm_process_per_outif(const struct ethhdr *ethhdr, if_outgoing, tt_buff, dup_status); rcu_read_unlock();
+ /* don't forward packet if no outgoing interface was specified */ + if (!if_outgoing) + goto out_neigh; + /* is single hop (direct) neighbor */ if (is_single_hop_neigh) { if ((batadv_ogm_packet->header.ttl <= 2) && @@ -1460,7 +1504,8 @@ batadv_iv_ogm_process_per_outif(const struct ethhdr *ethhdr, /* mark direct link on incoming interface */ batadv_iv_ogm_forward(orig_node, ethhdr, batadv_ogm_packet, is_single_hop_neigh, - is_from_best_next_hop, if_incoming); + is_from_best_next_hop, if_incoming, + if_outgoing);
batadv_dbg(BATADV_DBG_BATMAN, bat_priv, "Forwarding packet: rebroadcast neighbor packet with direct link flag\n"); @@ -1484,7 +1529,7 @@ batadv_iv_ogm_process_per_outif(const struct ethhdr *ethhdr, "Forwarding packet: rebroadcast originator packet\n"); batadv_iv_ogm_forward(orig_node, ethhdr, batadv_ogm_packet, is_single_hop_neigh, is_from_best_next_hop, - if_incoming); + if_incoming, if_outgoing);
out_neigh: if ((orig_neigh_node) && (!is_single_hop_neigh)) diff --git a/send.c b/send.c index c83be5e..da256df 100644 --- a/send.c +++ b/send.c @@ -379,6 +379,8 @@ static void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet) kfree_skb(forw_packet->skb); if (forw_packet->if_incoming) batadv_hardif_free_ref(forw_packet->if_incoming); + if (forw_packet->if_outgoing) + batadv_hardif_free_ref(forw_packet->if_outgoing); kfree(forw_packet); }
@@ -442,6 +444,7 @@ int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
forw_packet->skb = newskb; forw_packet->if_incoming = primary_if; + forw_packet->if_outgoing = NULL;
/* how often did we send the bcast packet ? */ forw_packet->num_packets = 0; @@ -539,9 +542,12 @@ void batadv_send_outstanding_bat_ogm_packet(struct work_struct *work)
/* we have to have at least one packet in the queue * to determine the queues wake up time unless we are - * shutting down + * shutting down. + * + * only re-schedule if this is the "original" copy. */ - if (forw_packet->own) + if (forw_packet->own && + forw_packet->if_incoming == forw_packet->if_outgoing) batadv_schedule_bat_ogm(forw_packet->if_incoming);
out: @@ -602,7 +608,8 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv, * we delete only packets belonging to the given interface */ if ((hard_iface) && - (forw_packet->if_incoming != hard_iface)) + (forw_packet->if_incoming != hard_iface) && + (forw_packet->if_outgoing != hard_iface)) continue;
spin_unlock_bh(&bat_priv->forw_bat_list_lock); diff --git a/types.h b/types.h index 9d221a8..7255500 100644 --- a/types.h +++ b/types.h @@ -993,8 +993,10 @@ struct batadv_skb_cb { * @direct_link_flags: direct link flags for aggregated OGM packets * @num_packets: counter for bcast packet retransmission * @delayed_work: work queue callback item for packet sending - * @if_incoming: pointer incoming hard-iface or primary iface if locally - * generated packet + * @if_incoming: pointer to incoming hard-iface or primary iface if + * locally generated packet + * @if_outgoing: packet where the packet should be sent to, or NULL if + * unspecified */ struct batadv_forw_packet { struct hlist_node list; @@ -1006,6 +1008,7 @@ struct batadv_forw_packet { uint8_t num_packets; struct delayed_work delayed_work; struct batadv_hard_iface *if_incoming; + struct batadv_hard_iface *if_outgoing; };
/**
On Wed, Sep 04, 2013 at 06:27:39PM +0200, Simon Wunderlich wrote:
@@ -574,6 +562,13 @@ batadv_iv_ogm_can_aggregate(const struct batadv_ogm_packet *new_bat_ogm_packet, if (!primary_if) goto out;
/* packet is not leaving on the same interface.
* TODO: some other parts here could be reworked as the
* outgoing interface is specified now.
*/
What does this TODO exactly mean? What could be reworked?
if (forw_packet->if_outgoing != if_outgoing)
goto out;
- /* packets without direct link flag and high TTL
*/
- are flooded through the net
@@ -620,6 +615,7 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, int packet_len, unsigned long send_time, bool direct_link, struct batadv_hard_iface *if_incoming,
struct batadv_hard_iface *if_outgoing, int own_packet)
ehm, kerneldoc?
[...]
@@ -1449,6 +1489,10 @@ batadv_iv_ogm_process_per_outif(const struct ethhdr *ethhdr, if_outgoing, tt_buff, dup_status); rcu_read_unlock();
- /* don't forward packet if no outgoing interface was specified */
- if (!if_outgoing)
goto out_neigh;
can this really happen? or does it mean we have a BUG?
- /* is single hop (direct) neighbor */ if (is_single_hop_neigh) { if ((batadv_ogm_packet->header.ttl <= 2) &&
[...]
@@ -539,9 +542,12 @@ void batadv_send_outstanding_bat_ogm_packet(struct work_struct *work)
/* we have to have at least one packet in the queue * to determine the queues wake up time unless we are
* shutting down
* shutting down.
*
*/* only re-schedule if this is the "original" copy.
- if (forw_packet->own)
- if (forw_packet->own &&
batadv_schedule_bat_ogm(forw_packet->if_incoming);forw_packet->if_incoming == forw_packet->if_outgoing)
What do you mean with "original" copy? Can an "own" packet have if_incoming != if_outgoing?
Cheers,
On Tue, Oct 01, 2013 at 11:00:09AM +0200, Antonio Quartulli wrote:
On Wed, Sep 04, 2013 at 06:27:39PM +0200, Simon Wunderlich wrote:
@@ -574,6 +562,13 @@ batadv_iv_ogm_can_aggregate(const struct batadv_ogm_packet *new_bat_ogm_packet, if (!primary_if) goto out;
/* packet is not leaving on the same interface.
* TODO: some other parts here could be reworked as the
* outgoing interface is specified now.
*/
What does this TODO exactly mean? What could be reworked?
There might be some more places where we can optimize/save a few cycles. The forw_packet was scheduled for all interfaces instead of individual interfaces before. Therefore direct link flag handling might be simplified. TBH I'm not sure and the aggregation code is not too easy to understand, so I preferred to not break it and put that TODO. :P
@@ -1449,6 +1489,10 @@ batadv_iv_ogm_process_per_outif(const struct ethhdr *ethhdr, if_outgoing, tt_buff, dup_status); rcu_read_unlock();
- /* don't forward packet if no outgoing interface was specified */
- if (!if_outgoing)
goto out_neigh;
can this really happen? or does it mean we have a BUG?
No, this is one of the main cases actually. batadv_iv_ogm_process_per_outif() is called:
* first for the out interface NULL * then for all interfaces
However the OGM is not forwarded for the NULL interface, only for the hard interfaces according to the respective rules. The if_outgoing = NULL case is only for locally generated packets.
@@ -539,9 +542,12 @@ void batadv_send_outstanding_bat_ogm_packet(struct work_struct *work)
/* we have to have at least one packet in the queue * to determine the queues wake up time unless we are
* shutting down
* shutting down.
*
*/* only re-schedule if this is the "original" copy.
- if (forw_packet->own)
- if (forw_packet->own &&
batadv_schedule_bat_ogm(forw_packet->if_incoming);forw_packet->if_incoming == forw_packet->if_outgoing)
What do you mean with "original" copy? Can an "own" packet have if_incoming != if_outgoing?
Yes, for the forw_packet of the primary interface we have multiple copies now for each outgoing interface (secondary interfaces). However we want to reschedule the OGM only once per period. This is why the check is neccesary. I'll rework the comment to make this more clear ...
Cheers,
-- Antonio Quartulli
From: Simon Wunderlich simon@open-mesh.com
With the new interface alternating, the first hop may send packets in a round robin fashion to it's neighbors because it has multiple valid routes built by the multi interface optimization. This patch enables the feature if bonding is selected. Note that unlike the bonding implemented before, this version is much simpler and may even enable multi path routing to a certain degree.
Signed-off-by: Simon Wunderlich simon@open-mesh.com --- routing.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- routing.h | 2 +- types.h | 2 ++ 3 files changed, 104 insertions(+), 4 deletions(-)
diff --git a/routing.c b/routing.c index 304e44b..013958f 100644 --- a/routing.c +++ b/routing.c @@ -407,16 +407,114 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv, struct batadv_neigh_node * batadv_find_router(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, - const struct batadv_hard_iface *recv_if) + struct batadv_hard_iface *recv_if) { - struct batadv_neigh_node *router; + struct batadv_algo_ops *bao = bat_priv->bat_algo_ops; + struct batadv_neigh_node *router, *cand_router, + *first_candidate_router = NULL, + *next_candidate_router; + struct batadv_neigh_node_ifinfo *router_ifinfo, *cand_ifinfo; + struct batadv_orig_node_ifinfo *cand, *first_candidate = NULL, + *next_candidate = NULL; + bool last_candidate_found = false;
if (!orig_node) return NULL;
router = batadv_orig_node_get_router(orig_node, recv_if);
- /* TODO: fill this later with new bonding mechanism */ + /* only consider bonding for recv_if == NULL (first hop) and if + * activated. + */ + if (recv_if || !atomic_read(&bat_priv->bonding) || !router) + return router; + + /* bonding: loop through the list of possible routers found + * for the various outgoing interfaces and find a candidate after + * the last chosen bonding candidate (next_candidate). If no such + * router is found, use the first candidate found (the last chosen + * bonding candidate might have been the last one in the list). + * If this can't be found either, return the previously choosen + * router - obviously there are no other candidates. + */ + rcu_read_lock(); + router_ifinfo = batadv_neigh_node_get_ifinfo(router, recv_if); + hlist_for_each_entry_rcu(cand, &orig_node->ifinfo_list, list) { + cand_router = rcu_dereference(cand->router); + if (!cand_router) + continue; + + if (!atomic_inc_not_zero(&cand_router->refcount)) + continue; + + cand_ifinfo = batadv_neigh_node_get_ifinfo(cand_router, + cand->if_outgoing); + if (!cand_ifinfo) + goto next; + + /* alternative candidate should be good enough to be + * considered + */ + if (!bao->bat_neigh_ifinfo_is_equiv_or_better(cand_ifinfo, + router_ifinfo)) + goto next; + + /* don't use the same router twice */ + if (orig_node->last_bonding_candidate && + (orig_node->last_bonding_candidate->router == + cand_router)) + goto next; + + /* mark the first possible candidate */ + if (!first_candidate) { + atomic_inc(&cand_router->refcount); + first_candidate = cand; + first_candidate_router = cand_router; + } + + /* check if already passed the next candidate ... this function + * should get the next candidate AFTER the last used bonding + * candidate. + */ + if (orig_node->last_bonding_candidate) { + if (orig_node->last_bonding_candidate == cand) { + last_candidate_found = true; + goto next; + } + + if (!last_candidate_found) + goto next; + } + + next_candidate = cand; + next_candidate_router = cand_router; + break; +next: + batadv_neigh_node_free_ref(cand_router); + } + + if (next_candidate) { + /* found a possible candidate after the last chosen bonding + * candidate, return it. + */ + batadv_neigh_node_free_ref(router); + if (first_candidate) + batadv_neigh_node_free_ref(first_candidate_router); + router = next_candidate_router; + orig_node->last_bonding_candidate = next_candidate; + } else if (first_candidate) { + /* found no possible candidate after the last candidate, return + * the first candidate if available, or the already selected + * router otherwise. + */ + batadv_neigh_node_free_ref(router); + router = first_candidate_router; + orig_node->last_bonding_candidate = first_candidate; + } else { + orig_node->last_bonding_candidate = NULL; + } + + rcu_read_unlock();
return router; } diff --git a/routing.h b/routing.h index 7a7c6e9..2c80b6a 100644 --- a/routing.h +++ b/routing.h @@ -46,7 +46,7 @@ int batadv_recv_unhandled_unicast_packet(struct sk_buff *skb, struct batadv_neigh_node * batadv_find_router(struct batadv_priv *bat_priv, struct batadv_orig_node *orig_node, - const struct batadv_hard_iface *recv_if); + struct batadv_hard_iface *recv_if); int batadv_window_protected(struct batadv_priv *bat_priv, int32_t seq_num_diff, unsigned long *last_reset);
diff --git a/types.h b/types.h index 7255500..fa58a14 100644 --- a/types.h +++ b/types.h @@ -173,6 +173,7 @@ struct batadv_orig_bat_iv { * @orig: originator ethernet address * @primary_addr: hosts primary interface address * @ifinfo_list: list for routers per outgoing interface + * @last_bonding_candidate: pointer to last ifinfo of last used router * @batadv_dat_addr_t: address of the orig node in the distributed hash * @last_seen: time when last packet from this node was received * @bcast_seqno_reset: time when the broadcast seqno window was reset @@ -213,6 +214,7 @@ struct batadv_orig_node { uint8_t orig[ETH_ALEN]; uint8_t primary_addr[ETH_ALEN]; struct hlist_head ifinfo_list; + struct batadv_orig_node_ifinfo *last_bonding_candidate; #ifdef CONFIG_BATMAN_ADV_DAT batadv_dat_addr_t dat_addr; #endif
On Wed, Sep 04, 2013 at 06:27:40PM +0200, Simon Wunderlich wrote:
From: Simon Wunderlich simon@open-mesh.com
With the new interface alternating, the first hop may send packets in a round robin fashion to it's neighbors because it has multiple valid routes built by the multi interface optimization. This patch enables the feature if bonding is selected. Note that unlike the bonding implemented before, this version is much simpler and may even enable multi path routing to a certain degree.
y0! :)
Signed-off-by: Simon Wunderlich simon@open-mesh.com
routing.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- routing.h | 2 +- types.h | 2 ++ 3 files changed, 104 insertions(+), 4 deletions(-)
diff --git a/routing.c b/routing.c index 304e44b..013958f 100644 --- a/routing.c +++ b/routing.c @@ -407,16 +407,114 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv,
[..]
{
- struct batadv_neigh_node *router;
- struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
- struct batadv_neigh_node *router, *cand_router,
*first_candidate_router = NULL,
*next_candidate_router;
Can you avoid this strange declaration style? :) One variable (with its type) per line.
- struct batadv_neigh_node_ifinfo *router_ifinfo, *cand_ifinfo;
- struct batadv_orig_node_ifinfo *cand, *first_candidate = NULL,
*next_candidate = NULL;
here too.
[...]
- /* bonding: loop through the list of possible routers found
* for the various outgoing interfaces and find a candidate after
* the last chosen bonding candidate (next_candidate). If no such
* router is found, use the first candidate found (the last chosen
* bonding candidate might have been the last one in the list).
* If this can't be found either, return the previously choosen
* router - obviously there are no other candidates.
*/
- rcu_read_lock();
- router_ifinfo = batadv_neigh_node_get_ifinfo(router, recv_if);
- hlist_for_each_entry_rcu(cand, &orig_node->ifinfo_list, list) {
cand_router = rcu_dereference(cand->router);
if (!cand_router)
continue;
if (!atomic_inc_not_zero(&cand_router->refcount))
continue;
cand_ifinfo = batadv_neigh_node_get_ifinfo(cand_router,
cand->if_outgoing);
if (!cand_ifinfo)
goto next;
/* alternative candidate should be good enough to be
* considered
*/
if (!bao->bat_neigh_ifinfo_is_equiv_or_better(cand_ifinfo,
router_ifinfo))
goto next;
/* don't use the same router twice */
if (orig_node->last_bonding_candidate &&
(orig_node->last_bonding_candidate->router ==
cand_router))
goto next;
/* mark the first possible candidate */
if (!first_candidate) {
atomic_inc(&cand_router->refcount);
first_candidate = cand;
first_candidate_router = cand_router;
}
What if you change this:
/* check if already passed the next candidate ... this function
* should get the next candidate AFTER the last used bonding
* candidate.
*/
if (orig_node->last_bonding_candidate) {
if (orig_node->last_bonding_candidate == cand) {
last_candidate_found = true;
goto next;
}
if (!last_candidate_found)
goto next;
}
next_candidate = cand;
next_candidate_router = cand_router;
break;
+next:
batadv_neigh_node_free_ref(cand_router);
to this:
if (!orig_node->last_bonding_candidate || last_candidate_found) { next_candidate = cand; next_candidate_router = cand_router; break; }
if (orig_node->last_bonding_candidate == cand) last_candidate_found = true;
batadv_neigh_node_free_ref(cand_router);
? (I hope the two are equivalent..I took some time to fully get the flow of your if/else block)
Removing the "goto next" makes me look at it in a more linear way (and we also reduce the max indentation by one tab).
- }
- if (next_candidate) {
/* found a possible candidate after the last chosen bonding
* candidate, return it.
*/
batadv_neigh_node_free_ref(router);
if (first_candidate)
batadv_neigh_node_free_ref(first_candidate_router);
router = next_candidate_router;
orig_node->last_bonding_candidate = next_candidate;
- } else if (first_candidate) {
/* found no possible candidate after the last candidate, return
* the first candidate if available, or the already selected
* router otherwise.
*/
batadv_neigh_node_free_ref(router);
router = first_candidate_router;
orig_node->last_bonding_candidate = first_candidate;
- } else {
orig_node->last_bonding_candidate = NULL;
- }
Also this if/else block could be simplified a little bit? But still it is readable and understandable :)
rcu_read_unlock();
return router;
}
[..]
diff --git a/types.h b/types.h index 7255500..fa58a14 100644 --- a/types.h +++ b/types.h @@ -173,6 +173,7 @@ struct batadv_orig_bat_iv {
- @orig: originator ethernet address
- @primary_addr: hosts primary interface address
- @ifinfo_list: list for routers per outgoing interface
- @last_bonding_candidate: pointer to last ifinfo of last used router
- @batadv_dat_addr_t: address of the orig node in the distributed hash
- @last_seen: time when last packet from this node was received
- @bcast_seqno_reset: time when the broadcast seqno window was reset
@@ -213,6 +214,7 @@ struct batadv_orig_node { uint8_t orig[ETH_ALEN]; uint8_t primary_addr[ETH_ALEN]; struct hlist_head ifinfo_list;
- struct batadv_orig_node_ifinfo *last_bonding_candidate;
Do you think it is not needed to set this field to NULL when disabling bonding mode? Can an old value trigger some strange behaviour when bonding is turned on again? Maybe not..just wondering..
Cheers,
On Tue, Oct 01, 2013 at 11:24:19AM +0200, Antonio Quartulli wrote:
- /* bonding: loop through the list of possible routers found
* for the various outgoing interfaces and find a candidate after
* the last chosen bonding candidate (next_candidate). If no such
* router is found, use the first candidate found (the last chosen
* bonding candidate might have been the last one in the list).
* If this can't be found either, return the previously choosen
* router - obviously there are no other candidates.
*/
- rcu_read_lock();
- router_ifinfo = batadv_neigh_node_get_ifinfo(router, recv_if);
- hlist_for_each_entry_rcu(cand, &orig_node->ifinfo_list, list) {
cand_router = rcu_dereference(cand->router);
if (!cand_router)
continue;
if (!atomic_inc_not_zero(&cand_router->refcount))
continue;
cand_ifinfo = batadv_neigh_node_get_ifinfo(cand_router,
cand->if_outgoing);
if (!cand_ifinfo)
goto next;
/* alternative candidate should be good enough to be
* considered
*/
if (!bao->bat_neigh_ifinfo_is_equiv_or_better(cand_ifinfo,
router_ifinfo))
goto next;
/* don't use the same router twice */
if (orig_node->last_bonding_candidate &&
(orig_node->last_bonding_candidate->router ==
cand_router))
goto next;
/* mark the first possible candidate */
if (!first_candidate) {
atomic_inc(&cand_router->refcount);
first_candidate = cand;
first_candidate_router = cand_router;
}
What if you change this:
/* check if already passed the next candidate ... this function
* should get the next candidate AFTER the last used bonding
* candidate.
*/
if (orig_node->last_bonding_candidate) {
if (orig_node->last_bonding_candidate == cand) {
last_candidate_found = true;
goto next;
}
if (!last_candidate_found)
goto next;
}
next_candidate = cand;
next_candidate_router = cand_router;
break;
+next:
batadv_neigh_node_free_ref(cand_router);
to this:
if (!orig_node->last_bonding_candidate || last_candidate_found) { next_candidate = cand; next_candidate_router = cand_router; break; }
if (orig_node->last_bonding_candidate == cand) last_candidate_found = true;
batadv_neigh_node_free_ref(cand_router);
? (I hope the two are equivalent..I took some time to fully get the flow of your if/else block)
Removing the "goto next" makes me look at it in a more linear way (and we also reduce the max indentation by one tab).
Yup, that should work too, good suggestion. We need to keep the next label though. I'll change it according to your suggestion.
diff --git a/types.h b/types.h index 7255500..fa58a14 100644 --- a/types.h +++ b/types.h @@ -173,6 +173,7 @@ struct batadv_orig_bat_iv {
- @orig: originator ethernet address
- @primary_addr: hosts primary interface address
- @ifinfo_list: list for routers per outgoing interface
- @last_bonding_candidate: pointer to last ifinfo of last used router
- @batadv_dat_addr_t: address of the orig node in the distributed hash
- @last_seen: time when last packet from this node was received
- @bcast_seqno_reset: time when the broadcast seqno window was reset
@@ -213,6 +214,7 @@ struct batadv_orig_node { uint8_t orig[ETH_ALEN]; uint8_t primary_addr[ETH_ALEN]; struct hlist_head ifinfo_list;
- struct batadv_orig_node_ifinfo *last_bonding_candidate;
Do you think it is not needed to set this field to NULL when disabling bonding mode? Can an old value trigger some strange behaviour when bonding is turned on again? Maybe not..just wondering..
Hmm, maybe, can't say now but will look into that.
Thanks for all your suggestions! Simon
b.a.t.m.a.n@lists.open-mesh.org