[PATCH 5/6] Add hash support to neigh cache

[Shrijeet Mukherjee]

Hi Thomas, 

ACK on the link object, will allocate that on the stack.

For the variable length keys, alloca can lead to strange situations if you overflow the stack, which is why calloc is a little safer. But more importantly, and we should figure out a way to make this clearer in the API, the key needs to be either packed or memset to zero before use. This is why we used a calloc.

Wondering if a malloc followed by a memset and a comment would be better to make that clear.


On Nov 5, 2012, at 4:13 AM, Thomas Graf <tgraf at suug.ch> wrote:

> On 11/04/12 at 09:13pm, roopa at cumulusnetworks.com wrote:
>> +static void neigh_keygen(struct nl_object *obj, uint32_t *hashkey,
>> +			 uint32_t table_sz)
>> +{
>> +	struct rtnl_neigh *neigh = (struct rtnl_neigh *) obj;
>> +	struct neigh_hash_key *_key;
>> +	char *_key_b;
>> +	unsigned int _key_sz;
>> +	unsigned int sz_dst = 0;
>> +	void *dst = NULL;
>> +	struct neigh_hash_key {
>> +		uint32_t	n_family;
>> +		uint32_t	n_ifindex;
>> +		void 		*n_addr;
>> +	};
>> +
>> +	if (neigh->n_dst) {
>> +	    sz_dst = nl_addr_get_len(neigh->n_dst);
>> +	    dst = nl_addr_get_binary_addr(neigh->n_dst);
>> +	}
>> +
>> +	_key_sz = sizeof(struct neigh_hash_key) -
>> +			sizeof(void *) + sz_dst;
> 
> Can you write the keygen functions like this:
> 
> struct neigh_hash_key {
> 	uint32_t	n_family;
> 	uint32_t	n_ifindex;
> 	void *		n_addr[0];
> };
> 
> key_sz = sizeof(struct neigh_hash_key);
> if (neigh->n_dst)
> 	key_sz += nl_addr_get_len(neigh->n_dst);
> 
> key = alloca(key_sz);
> [...]
> 
> if (neigh->n_dst)
>        memcpy(key->n_addr, nl_addr_get_binary_addr(neigh->n_dst),
> 	       nl_addr_get_len(neigh->n_dst));
> [...]
> 
> That puts the key structure onto the stack which is fine for this.