[PATH] dev: reusing unregistered ifindex values in net_device

[PATH] dev: reusing unregistered ifindex values in net_device

[Daniel Turull]

When a new index is going to be assigned in register_netdevice,
the dev_new_index starts to search possible values from the last index
given to a device although there might be some free ifindex that has been
previously unregistered.  This behaviour may create gap(s) in the ifindex list.

This patch checks for unused values from 1 and gives to the new device the
first available value. This limits the maximum ifindex to a smaller value.

The ifindex will still be unique since the old value is no longer in use.

Reported-by: Voravit Tanyingyong  <voravit@kth.se>
Signed-off-by: Daniel Turull <daniel.turull@gmail.com>

---
diff --git a/net/core/dev.c b/net/core/dev.c
index 381b8e2..a7babab 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4871,7 +4871,7 @@ int dev_ioctl(struct net *net, unsigned int cmd,
void __user *arg)
  */
 static int dev_new_index(struct net *net)
 {
-	static int ifindex;
+	int ifindex;
 	for (;;) {
 		if (++ifindex <= 0)
 			ifindex = 1;

Re: [PATH] dev: reusing unregistered ifindex values in net_device

[Eric Dumazet]

Le jeudi 18 novembre 2010 à 16:25 +0100, Daniel Turull a écrit :
> When a new index is going to be assigned in register_netdevice,
> the dev_new_index starts to search possible values from the last index
> given to a device although there might be some free ifindex that has been
> previously unregistered.  This behaviour may create gap(s) in the ifindex list.
> 
> This patch checks for unused values from 1 and gives to the new device the
> first available value. This limits the maximum ifindex to a smaller value.
> 
> The ifindex will still be unique since the old value is no longer in use.
> 
> Reported-by: Voravit Tanyingyong  <voravit@kth.se>
> Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
> 
> ---
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 381b8e2..a7babab 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4871,7 +4871,7 @@ int dev_ioctl(struct net *net, unsigned int cmd,
> void __user *arg)
>   */
>  static int dev_new_index(struct net *net)
>  {
> -	static int ifindex;
> +	int ifindex;
>  	for (;;) {
>  		if (++ifindex <= 0)
>  			ifindex = 1;
> --

NACK

Two bugs 

1) ifindex is not initialized : you'll be suprised of random values

2) ifindex should not be reused. You'll be surprised so applications can
break. SNMP comes to mind.

Re: [PATH] dev: reusing unregistered ifindex values in net_device

[Daniel Turull]

Ok,
thanks you for you quick response.

//Daniel



On Thu, Nov 18, 2010 at 16:37, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 18 novembre 2010 à 16:25 +0100, Daniel Turull a écrit :
>> When a new index is going to be assigned in register_netdevice,
>> the dev_new_index starts to search possible values from the last index
>> given to a device although there might be some free ifindex that has been
>> previously unregistered.  This behaviour may create gap(s) in the ifindex list.
>>
>> This patch checks for unused values from 1 and gives to the new device the
>> first available value. This limits the maximum ifindex to a smaller value.
>>
>> The ifindex will still be unique since the old value is no longer in use.
>>
>> Reported-by: Voravit Tanyingyong  <voravit@kth.se>
>> Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
>>
>> ---
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 381b8e2..a7babab 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -4871,7 +4871,7 @@ int dev_ioctl(struct net *net, unsigned int cmd,
>> void __user *arg)
>>   */
>>  static int dev_new_index(struct net *net)
>>  {
>> -     static int ifindex;
>> +     int ifindex;
>>       for (;;) {
>>               if (++ifindex <= 0)
>>                       ifindex = 1;
>> --
>
> NACK
>
> Two bugs
>
> 1) ifindex is not initialized : you'll be suprised of random values
>
> 2) ifindex should not be reused. You'll be surprised so applications can
> break. SNMP comes to mind.
>
>
>
>

Re: [PATH] dev: reusing unregistered ifindex values in net_device

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 18 Nov 2010 16:37:08 +0100

> Two bugs 
> 
> 1) ifindex is not initialized : you'll be suprised of random values
> 
> 2) ifindex should not be reused. You'll be surprised so applications can
> break. SNMP comes to mind.

Right, the current algorithm is intentionally trying to avoid new
devices from using indexes that were used by another device in the
past.

Re: [PATH] dev: reusing unregistered ifindex values in net_device

[Stephen Hemminger]

On Thu, 18 Nov 2010 08:01:39 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 18 Nov 2010 16:37:08 +0100
> 
> > Two bugs 
> > 
> > 1) ifindex is not initialized : you'll be suprised of random values
> > 
> > 2) ifindex should not be reused. You'll be surprised so applications can
> > break. SNMP comes to mind.
> 
> Right, the current algorithm is intentionally trying to avoid new
> devices from using indexes that were used by another device in the
> past.

Also think of the case of adding 10,000 VLAN's. If the search started from
beginning on each insertion dev_new_index would go from O(N) to O(N^2) which
really hurts.