On Tuesday 26 June 2012 22:00:21 Antonio Quartulli wrote: > With the current TT mechanism a new client joining the network is not > immediately able to communicate with other hosts because its MAC address has > not been announced yet. This situation holds until the first OGM containing > its joining event will be spread over the mesh network. > > This behaviour can be acceptable in networks where the originator interval > is a small value (e.g. 1sec) but if that value is set to an higher time > (e.g. 5secs) the client could suffer from several malfunctions like DHCP > client timeouts, etc. > > This patch adds an early detection mechanism that makes nodes in the network > able to recognise "not yet announced clients" by means of the broadcast > packets they emitted on connection (e.g. ARP or DHCP request). The added > client will then be confirmed upon receiving the OGM claiming it or purged > if such OGM is not received within a fixed amount of time. > > Signed-off-by: Antonio Quartulli <[email protected]> > --- > main.h | 2 + > packet.h | 1 + > translation-table.c | 152 > ++++++++++++++++++++++++++++++++++++++------------- translation-table.h | > 4 +- > types.h | 1 + > 5 files changed, 121 insertions(+), 39 deletions(-) > > diff --git a/main.h b/main.h > index 6dca9c4..00c9c34 100644 > --- a/main.h > +++ b/main.h > @@ -43,6 +43,8 @@ > #define BATADV_PURGE_TIMEOUT 200000 /* 200 seconds */ > #define BATADV_TT_LOCAL_TIMEOUT 3600000 /* in miliseconds */ > #define BATADV_TT_CLIENT_ROAM_TIMEOUT 600000 /* in miliseconds */ > +#define BATADV_TT_CLIENT_TEMP_TIMEOUT_FACT 10
At least make a comment what this value says (fact * orig_interval) or
something like that. And you may have to make it UL to really return unsigned
long in batadv_tt_client_temp_timeout.
> -static void batadv_tt_global_roam_purge(struct batadv_priv *bat_priv)
> +static void batadv_tt_global_purge(struct batadv_priv *bat_priv)
> {
> struct batadv_hashtable *hash = bat_priv->tt_global_hash;
> struct hlist_head *head;
> + struct hlist_node *node, *node_tmp;
> spinlock_t *list_lock; /* protects write access to the hash lists */
> uint32_t i;
> + bool purge;
> + char *msg = NULL;
> + struct batadv_tt_common_entry *tt_common_entry;
> + struct batadv_tt_global_entry *tt_global;
> + unsigned long temp_timeout = batadv_tt_client_temp_timeout(bat_priv);
> + unsigned long roam_timeout = BATADV_TT_CLIENT_ROAM_TIMEOUT;
>
> for (i = 0; i < hash->size; i++) {
> head = &hash->table[i];
> list_lock = &hash->list_locks[i];
>
> spin_lock_bh(list_lock);
> - batadv_tt_global_roam_purge_list(bat_priv, head);
> + hlist_for_each_entry_safe(tt_common_entry, node, node_tmp,
> + head, hash_entry) {
> + purge = false;
> + tt_global = container_of(tt_common_entry,
> + struct batadv_tt_global_entry,
> + common);
> + if ((tt_global->common.flags & BATADV_TT_CLIENT_ROAM) &&
> + batadv_has_timed_out(tt_global->roam_at,
> + roam_timeout)) {
> + purge = true;
> + msg = "Roaming timeout\n";
> + }
> +
> + if ((tt_global->common.flags & BATADV_TT_CLIENT_TEMP) &&
> + batadv_has_timed_out(tt_global->common.added_at,
> + temp_timeout)) {
> + purge = true;
> + msg = "Temporary client timeout\n";
> + }
> +
> + if (!purge)
> + continue;
> +
> + batadv_dbg(BATADV_DBG_TT, bat_priv,
> + "Deleting global tt entry (%pM): %s\n",
> + tt_global->common.addr, msg);
> +
> + hlist_del_rcu(node);
> + batadv_tt_global_entry_free_ref(tt_global);
> + }
> spin_unlock_bh(list_lock);
> }
Why writing this whole thing in one function? You don't get a price for the
highest indentation level that banged the hardest against the 80 character
limit.
> - if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM)
> + if (tt_common_entry->flags & BATADV_TT_CLIENT_ROAM ||
> + tt_common_entry->flags & BATADV_TT_CLIENT_TEMP)
> return 0;
>
> +
> tt_global_entry = container_of(tt_common_entry,
> struct batadv_tt_global_entry,
> common);
And this new line doesn't make sense
Kind regards,
Sven
signature.asc
Description: This is a digitally signed message part.
