Re: [B.A.T.M.A.N.] [PATCHv5 3/3] batman-adv: Modified forwarding behaviour for multicast packets

[Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>]

[-- Attachment #1: Type: text/plain, Size: 2741 bytes --]

On Fri, Jun 14, 2013 at 11:02:05AM +0200, Linus Lüssing wrote:
  
>  /**
> + * batadv_tt_orig_entries_count - count the number of originators
> + * @head: a list of originators
> + *
> + * Return the number of originator entries in the given list.
> + *
> + * The caller needs to hold the rcu_read_lock().
> + */
> +static int batadv_tt_orig_entries_count(struct hlist_head *head)
> +{
> +	struct batadv_tt_orig_list_entry *orig_entry;
> +	int count = 0;
> +
> +	hlist_for_each_entry_rcu(orig_entry, head, list) {
> +		if (!atomic_read(&orig_entry->refcount))
> +			continue;
> +
> +		count++;
> +	}
> +
> +	return count;
> +}
> +
> +/**
> + * batadv_tt_global_hash_count - count the number of orig entries
> + * @hash: hash table containing the tt entries
> + * @addr: the mac address of the client to count entries for
> + * @vid: VLAN identifier
> + *
> + * Return the number of originators advertising the given address/data
> + * (excluding ourself).
> + */
> +int batadv_tt_global_hash_count(struct batadv_priv *bat_priv,
> +				const uint8_t *addr, unsigned short vid)
> +{
> +	struct hlist_head *head, *orig_list;
> +	struct batadv_tt_common_entry to_search, *tt_common_entry;
> +	struct batadv_tt_global_entry *tt_global_entry;
> +	uint32_t index;
> +	int count = 0;
> +
> +	if (!bat_priv->tt.global_hash)
> +		goto out;
> +
> +	memcpy(to_search.addr, addr, ETH_ALEN);
> +	to_search.vid = vid;
> +
> +	index = batadv_choose_tt(&to_search, bat_priv->tt.global_hash->size);
> +	head = &bat_priv->tt.global_hash->table[index];
> +
> +	rcu_read_lock();
> +	hlist_for_each_entry_rcu(tt_common_entry, head, hash_entry) {
> +		if (!batadv_compare_eth(tt_common_entry, addr))
> +			continue;
> +
> +		if (!atomic_read(&tt_common_entry->refcount))
> +			continue;
> +
> +		tt_global_entry = container_of(tt_common_entry,
> +					       struct batadv_tt_global_entry,
> +					       common);
> +		orig_list = &tt_global_entry->orig_list;
> +		count = batadv_tt_orig_entries_count(orig_list);
> +		break;
> +	}
> +	rcu_read_unlock();
> +
> +out:
> +	return count;
> +}
> +
> +/**

I've been thinking about this part ... it is not good to iterate over the whole
hash for every multicast packet - this is the fastpath after all, so it's critical.

Can't you just use batadv_tt_global_hash_find(), count the list entries within
struct batadv_tt_global_entry (or find out if it is empty, one entry or more
than one entry) and be done? This should be much faster.

Apart from that, the patch series appears to be pretty mature now, I've done
some more testing in my VMs and it looks good. Should be (hopefully) the last
thing from my side. :)

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]