Re: [B.A.T.M.A.N.] [PATCH 4/5] batman-adv: detect not yet announced clients

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 ordex@autistici.org

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