Hello,
as you may have seen in the last period, TT has been under heavily re-design/work in order to improve its robustness (and also readability). However, some more extensive tests and emulated scenarios revealed that it is possible to hit some situations in which TT behaviour is not optimal.
This set of patches aims to fix some glitches introduces with the last re-design and to fix some issues discovered by testing complex scenarios.
Cheers,
Antonio Quartulli (6): batman-adv: fix TT packet rerouting batman-adv: fix debug message batman-adv: remove useless goto batman-adv: send ROAMING_ADV once batman-adv: remove useless assignment in tt_local_add() batman-adv: fix local client recognition in is_my_client()
routing.c | 36 ++++++++++++++++++++++++++---------- translation-table.c | 22 +++++++++++----------- 2 files changed, 37 insertions(+), 21 deletions(-)
when the node checks for a packet to be re-routed or not, first it should consider whether the destination is a local client. Only after this check, the node can eventually search the global table.
Signed-off-by: Antonio Quartulli ordex@autistici.org --- routing.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/routing.c b/routing.c index 5da7724..c8d2fbf 100644 --- a/routing.c +++ b/routing.c @@ -890,22 +890,37 @@ batadv_reroute_unicast_packet(struct batadv_priv *bat_priv, struct batadv_unicast_packet *unicast_packet, uint8_t *dst_addr) { - struct batadv_orig_node *orig_node; + struct batadv_orig_node *orig_node = NULL; + struct batadv_hard_iface *primary_if = NULL; bool ret = false; + uint8_t *orig_addr, orig_ttvn;
- orig_node = batadv_transtable_search(bat_priv, NULL, dst_addr); - if (!orig_node) - goto out; + if (batadv_is_my_client(bat_priv, dst_addr)) { + primary_if = batadv_primary_if_get_selected(bat_priv); + if (!primary_if) + goto out; + orig_addr = primary_if->net_dev->dev_addr; + orig_ttvn = (uint8_t)atomic_read(&bat_priv->tt.vn); + } else { + orig_node = batadv_transtable_search(bat_priv, NULL, dst_addr); + if (!orig_node) + goto out;
- if (batadv_compare_eth(orig_node->orig, unicast_packet->dest)) - goto out; + if (batadv_compare_eth(orig_node->orig, unicast_packet->dest)) + goto out; + + orig_addr = orig_node->orig; + orig_ttvn = (uint8_t)atomic_read(&orig_node->last_ttvn); + }
/* update the packet header */ - memcpy(unicast_packet->dest, orig_node->orig, ETH_ALEN); - unicast_packet->ttvn = (uint8_t)atomic_read(&orig_node->last_ttvn); + memcpy(unicast_packet->dest, orig_addr, ETH_ALEN); + unicast_packet->ttvn = orig_ttvn;
ret = true; out: + if (primary_if) + batadv_hardif_free_ref(primary_if); if (orig_node) batadv_orig_node_free_ref(orig_node);
In case of re-routing, the old_ttvn must be saved before being overwritten, otherwise the debug message will obviously print the new one.
Signed-off-by: Antonio Quartulli ordex@autistici.org --- routing.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/routing.c b/routing.c index c8d2fbf..6a84f0d 100644 --- a/routing.c +++ b/routing.c @@ -929,7 +929,7 @@ out:
static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv, struct sk_buff *skb) { - uint8_t curr_ttvn; + uint8_t curr_ttvn, old_ttvn; struct batadv_orig_node *orig_node; struct ethhdr *ethhdr; struct batadv_hard_iface *primary_if; @@ -994,6 +994,7 @@ static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv, if (!is_old_ttvn) return 1;
+ old_ttvn = unicast_packet->ttvn; /* the packet was forged based on outdated network information. Its * destination can possibly be updated and forwarded towards the new * target host @@ -1003,7 +1004,7 @@ static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv, net_ratelimited_function(batadv_dbg, BATADV_DBG_TT, bat_priv, "Rerouting unicast packet to %pM (dst=%pM): TTVN mismatch old_ttvn=%u new_ttvn=%u\n", unicast_packet->dest, ethhdr->h_dest, - unicast_packet->ttvn, curr_ttvn); + old_ttvn, curr_ttvn); return 1; }
This was introduced by ("batman-adv: roaming handling mechanism redesign")
Signed-off-by: Antonio Quartulli ordex@autistici.org --- translation-table.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/translation-table.c b/translation-table.c index 6fecb94..59e64bc 100644 --- a/translation-table.c +++ b/translation-table.c @@ -292,7 +292,6 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, */ tt_local->common.flags &= ~BATADV_TT_CLIENT_ROAM; roamed_back = true; - goto check_roaming; } goto check_roaming; }
If a client roaming has already been advertised, the node should prevent it from doing the same more than once. To achieve this, the node has to check the ROAM flag on the global client: if this is set already, then the ROAMING_ADV for this client has already been sent.
This should be merged with changes done by ("batman-adv: roaming handling mechanism redesign")
Signed-off-by: Antonio Quartulli ordex@autistici.org --- translation-table.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/translation-table.c b/translation-table.c index 59e64bc..517324a 100644 --- a/translation-table.c +++ b/translation-table.c @@ -336,8 +336,10 @@ add_event: batadv_tt_local_event(bat_priv, addr, tt_local->common.flags);
check_roaming: - /* Check whether it is a roaming! */ - if (tt_global) { + /* Check whether it is a roaming, but don't do anything if the roaming + * process has already been handled + */ + if (tt_global && !(tt_global->common.flags & BATADV_TT_CLIENT_ROAM)) { /* These node are probably going to update their tt table */ head = &tt_global->orig_list; rcu_read_lock();
The flag field of the tt_local_entry->common structure in tt_local_add() is first assigned NO_FLAGS and then TT_CLIENT_NEW so nullifying the first operation. For this reason it is safe to remove the first assignment.
Signed-off-by: Antonio Quartulli ordex@autistici.org --- translation-table.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/translation-table.c b/translation-table.c index 517324a..0e18348 100644 --- a/translation-table.c +++ b/translation-table.c @@ -305,7 +305,11 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, (uint8_t)atomic_read(&bat_priv->tt.vn));
memcpy(tt_local->common.addr, addr, ETH_ALEN); - tt_local->common.flags = BATADV_NO_FLAGS; + /* The local entry has to be marked as NEW to avoid to send it in + * a full table response going out before the next ttvn increment + * (consistency check) + */ + tt_local->common.flags = BATADV_TT_CLIENT_NEW; if (batadv_is_wifi_iface(ifindex)) tt_local->common.flags |= BATADV_TT_CLIENT_WIFI; atomic_set(&tt_local->common.refcount, 2); @@ -316,12 +320,6 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr, if (batadv_compare_eth(addr, soft_iface->dev_addr)) tt_local->common.flags |= BATADV_TT_CLIENT_NOPURGE;
- /* The local entry has to be marked as NEW to avoid to send it in - * a full table response going out before the next ttvn increment - * (consistency check) - */ - tt_local->common.flags |= BATADV_TT_CLIENT_NEW; - hash_added = batadv_hash_add(bat_priv->tt.local_hash, batadv_compare_tt, batadv_choose_orig, &tt_local->common, &tt_local->common.hash_entry);
A tt_local_entry which ROAM flag is set cannot be considered a local client anymore. Having the ROAM flag set means that the client roamed away and that the nodes received a ROAMING_ADV for such event
Signed-off-by: Antonio Quartulli ordex@autistici.org --- translation-table.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/translation-table.c b/translation-table.c index 0e18348..78d02fb 100644 --- a/translation-table.c +++ b/translation-table.c @@ -2042,7 +2042,8 @@ bool batadv_is_my_client(struct batadv_priv *bat_priv, const uint8_t *addr) /* Check if the client has been logically deleted (but is kept for * consistency purpose) */ - if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) + if ((tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) || + (tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM)) goto out; ret = true; out:
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli