netdev - Re: [PATCH] net: usbnet: allow overriding of default USB interface naming

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <469dd530-ebd2-37a4-9c6a-9de86e7a38dc@nutanix.com>
Date:   Mon, 14 Jun 2021 10:32:05 +0100
From:   Jonathan Davies <jonathan.davies@...anix.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Oliver Neukum <oneukum@...e.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        andrew@...n.ch
Subject: Re: [PATCH] net: usbnet: allow overriding of default USB interface
 naming

On 12/06/2021 08:01, Greg KH wrote:
> On Fri, Jun 11, 2021 at 03:23:39PM +0000, Jonathan Davies wrote:
>> When the predictable device naming scheme for NICs is not in use, it is
>> common for there to be udev rules to rename interfaces to names with
>> prefix "eth".
>>
>> Since the timing at which USB NICs are discovered is unpredictable, it
>> can be interfere with udev's attempt to rename another interface to
>> "eth0" if a freshly discovered USB interface is initially given the name
>> "eth0".
>>
>> Hence it is useful to be able to override the default name. A new usbnet
>> module parameter allows this to be configured.
>>
>> Signed-off-by: Jonathan Davies <jonathan.davies@...anix.com>
>> Suggested-by: Prashanth Sreenivasa <prashanth.sreenivasa@...anix.com>
>> ---
>>   drivers/net/usb/usbnet.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
>> index ecf6284..55f6230 100644
>> --- a/drivers/net/usb/usbnet.c
>> +++ b/drivers/net/usb/usbnet.c
>> @@ -72,6 +72,13 @@ static int msg_level = -1;
>>   module_param (msg_level, int, 0);
>>   MODULE_PARM_DESC (msg_level, "Override default message level");
>>   
>> +#define DEFAULT_ETH_DEV_NAME "eth%d"
>> +
>> +static char *eth_device_name = DEFAULT_ETH_DEV_NAME;
>> +module_param(eth_device_name, charp, 0644);
>> +MODULE_PARM_DESC(eth_device_name, "Device name pattern for Ethernet devices"
>> +				  " (default: \"" DEFAULT_ETH_DEV_NAME "\")");
> 
> This is not the 1990's, please do not add new module parameters as they
> are on a global driver level, and not on a device level.

The initial name is set at probe-time, so the device doesn't exist yet. 
So I felt like it was a choice between either changing the hard-coded 
"eth%d" string or providing a driver-level module parameter. Is there a 
better alternative?

> Also changing the way usb network devices are named is up to userspace,
> the kernel should not be involved in this.  What is wrong with just
> renaming it in userspace as you want to today?

Yes, renaming devices is the responsibility of userspace. Normally udev 
will rename a device shortly after it is probed. But there's a window 
during which it has the name the kernel initially assigns. If there's 
other renaming activity happening during that window there's a chance of 
collisions.

Userspace solutions include:
  1. udev backing off and retrying in the event of a collision; or
  2. avoiding ever renaming a device to a name in the "eth%d" namespace.

Solution 1 is ugly and slow. It's much neater to avoid the collisions in 
the first place where possible.

Solution 2 arises naturally from use of the predictable device naming 
scheme. But when userspace is not using that, solution 2 may not apply.

Yes, the problem is a result of userspace decisions, but that doesn't 
mean the kernel can't help make things easier.

Thanks,
Jonathan