Re: [B.A.T.M.A.N.] [PATCH 07/19] batman-adv: Prepare and send own multicast tracker packets

Hi,

please see comments inline.

On Sat, Jan 22, 2011 at 02:21:30AM +0100, Linus Lüssing wrote:

[...] +static int add_router_of_dest(struct dest_entries_list *next_hops,

• 	      uint8_t *dest, struct bat_priv *bat_priv)
  

+{

• struct dest_entries_list *next_hop_tmp, *next_hop_entry;
• struct element_t *bucket;
• struct orig_node *orig_node;
• struct hashtable_t *hash = bat_priv->orig_hash;
• struct hlist_node *walk;
• struct hlist_head *head;
• int i;
• next_hop_entry = kmalloc(sizeof(struct dest_entries_list), GFP_ATOMIC);
• if (!next_hop_entry)

• return 1;
  

• next_hop_entry->batman_if = NULL;
• for (i = 0; i < hash->size; i++) {

• head = &hash->table[i];
  

• rcu_read_lock();
  

• hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
  

• 	orig_node = bucket->data;
  

• 	if (memcmp(orig_node->orig, dest, ETH_ALEN))
  

• 		continue;
  

Traversing the hash yourself seems pretty redundant. If you need to find a specific node, you can just use hash_find().

• 	if (!orig_node->router) {
  

• 		i = hash->size;
  

• 		break;
  

• 	}
  

• 	memcpy(next_hop_entry->dest, orig_node->router->addr,
  

• 	       ETH_ALEN);
  

• 	next_hop_entry->batman_if =
  

• 				orig_node->router->if_incoming;
  

• 	i = hash->size;
  

• 	break;
  

• }
  

• rcu_read_unlock();
  

• }

[...] +static void zero_tracker_packet(struct mcast_tracker_packet *tracker_packet,

• 		uint8_t *next_hop, struct bat_priv *bat_priv)
  

+{

• struct tracker_packet_state state;
• struct element_t *bucket;
• struct orig_node *orig_node;
• struct hashtable_t *hash = bat_priv->orig_hash;
• struct hlist_node *walk;
• struct hlist_head *head;
• int i;
• tracker_packet_for_each_dest(&state, tracker_packet) {

• for (i = 0; i < hash->size; i++) {
  

• 	head = &hash->table[i];
  

• 	rcu_read_lock();
  

• 	hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
  

• 		orig_node = bucket->data;
  

• 		if (memcmp(orig_node->orig, state.dest_entry,
  

• 			   ETH_ALEN))
  

• 			continue;
  

Same here. Just use hash_find().

• 		/* is the next hop already our destination? */
  

• 		if (!memcmp(orig_node->orig, next_hop,
  

• 			    ETH_ALEN))
  

• 			memset(state.dest_entry, '\0',
  

• 			       ETH_ALEN);
  

• 		else if (!orig_node->router)
  

• 			memset(state.dest_entry, '\0',
  

• 			       ETH_ALEN);
  

• 		else if (!memcmp(orig_node->orig,
  

• 				 orig_node->router->orig_node->
  

• 				 primary_addr, ETH_ALEN))
  

• 			memset(state.dest_entry, '\0',
  

• 			       ETH_ALEN);
  

• 		/* is this the wrong next hop for our
  

• 		 * destination? */
  

• 		else if (memcmp(orig_node->router->addr,
  

• 				next_hop, ETH_ALEN))
  

• 			memset(state.dest_entry, '\0',
  

• 			       ETH_ALEN);
  

• 		i = hash->size;
  

• 		break;
  

• 	}
  

• 	rcu_read_unlock();
  

• }
  

• }

+}