[ovs-dev] [PATCH V2] netdev-dpdk: fix ifindex assignment for DPDK ports

[Daniele Di Proietto]

On 31/01/2017 13:52, "Ben Pfaff" <blp at ovn.org> wrote:

>On Thu, Dec 08, 2016 at 01:16:22PM +0100, Przemyslaw Lal wrote:
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index de78ddd..ef99eb3 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -2075,7 +2075,13 @@ netdev_dpdk_get_ifindex(const struct netdev *netdev)
>>      int ifindex;
>>  
>>      ovs_mutex_lock(&dev->mutex);
>> -    ifindex = dev->port_id;
>> +    /* Calculate hash from the netdev name using hash_bytes() function.
>> +     * Because ifindex is declared as signed int in the kernel sources and
>> +     * OVS follows this implementation right shift is needed to set sign bit
>> +     * to 0 and then XOR to slightly improve collision rate.
>> +     */
>> +    uint32_t h = hash_bytes(netdev->name, strlen(netdev->name), 0);
>> +    ifindex = (int)((h >> 1) ^ (h & 0x0FFFFFFF));
>
>To hash a string, please use hash_string().
>
>Daniele, are you planning to review this?

Sorry for the delay.

At some point, with vhost-pmd we will have port_ids also for vhost interfaces.  Maybe we can revisit this approach when that becomes available.

I wish there was a better way to avoid collisions with the linux kernel, but I can't think of anything generic.

Unless someone else has an objection I'm fine with the approach.  Two minor comments:

Could you please use hash_string(), as suggested by Ben?

I guess the result after hashing and XOR could still be zero.  Could you maybe add a check for that case and set it to something else?

Thanks,

Daniele