diff mbox series

[net-next,v2] net: sysfs: also pass network device driver to uevent

Message ID 20241116163206.7585-2-mail@tk154.de (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v2] net: sysfs: also pass network device driver to uevent | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 3 maintainers not CCed: edumazet@google.com horms@kernel.org pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4 this patch: 4
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 19 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-11-17--12-00 (tests: 789)

Commit Message

Til Kaiser Nov. 16, 2024, 4:30 p.m. UTC
Currently, for uevent, the interface name and
index are passed via shell variables.

This commit also passes the network device
driver as a shell variable to uevent.

One way to retrieve a network interface's driver
name is to resolve its sysfs device/driver symlink
and then substitute leading directory components.

You could implement this yourself (e.g., like udev from
systemd does) or with Linux tools by using a combination
of readlink and shell substitution or basename.

The advantages of passing the driver directly through uevent are:
 - Linux distributions don't need to implement additional code
   to retrieve the driver when, e.g., interface events happen.
 - There is no need to create additional process forks in shell
   scripts for readlink or basename.
 - If a user wants to check his network interface's driver on the
   command line, he can directly read it from the sysfs uevent file.

Signed-off-by: Til Kaiser <mail@tk154.de>
---
 net/core/net-sysfs.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Jakub Kicinski Nov. 19, 2024, 1:55 a.m. UTC | #1
On Sat, 16 Nov 2024 17:30:30 +0100 Til Kaiser wrote:
> Currently, for uevent, the interface name and
> index are passed via shell variables.
> 
> This commit also passes the network device
> driver as a shell variable to uevent.
> 
> One way to retrieve a network interface's driver
> name is to resolve its sysfs device/driver symlink
> and then substitute leading directory components.
> 
> You could implement this yourself (e.g., like udev from
> systemd does) or with Linux tools by using a combination
> of readlink and shell substitution or basename.
> 
> The advantages of passing the driver directly through uevent are:
>  - Linux distributions don't need to implement additional code
>    to retrieve the driver when, e.g., interface events happen.
>  - There is no need to create additional process forks in shell
>    scripts for readlink or basename.
>  - If a user wants to check his network interface's driver on the
>    command line, he can directly read it from the sysfs uevent file.

Thanks for the info, since you're working on an open source project 
- I assume your exact use case is not secret, could you spell it 
out directly? What device naming are you trying to achieve based on
what device drivers? In my naive view we have 200+ Ethernet drivers 
so listing Ethernet is not scalable. I'm curious what you're matching,
how many drivers you need to list, and whether we could instead add a
more general attribute...

Those questions aside, I'd like to get an ack from core driver experts
like GregKH on this. IDK what (if any) rules there are on uevents.
The merge window has started so we are very unlikely to hear from them
now, all maintainers will be very busy. Please repost v3 in >=two weeks
and CC Greg (and whoever else is reviewing driver core and/or uevent
changes according to git logs).
Til Kaiser Dec. 5, 2024, 4:28 p.m. UTC | #2
On 19.11.24 02:55, Jakub Kicinski wrote:
> On Sat, 16 Nov 2024 17:30:30 +0100 Til Kaiser wrote:
>> Currently, for uevent, the interface name and
>> index are passed via shell variables.
>>
>> This commit also passes the network device
>> driver as a shell variable to uevent.
>>
>> One way to retrieve a network interface's driver
>> name is to resolve its sysfs device/driver symlink
>> and then substitute leading directory components.
>>
>> You could implement this yourself (e.g., like udev from
>> systemd does) or with Linux tools by using a combination
>> of readlink and shell substitution or basename.
>>
>> The advantages of passing the driver directly through uevent are:
>>   - Linux distributions don't need to implement additional code
>>     to retrieve the driver when, e.g., interface events happen.
>>   - There is no need to create additional process forks in shell
>>     scripts for readlink or basename.
>>   - If a user wants to check his network interface's driver on the
>>     command line, he can directly read it from the sysfs uevent file.
> 
> Thanks for the info, since you're working on an open source project
> - I assume your exact use case is not secret, could you spell it
> out directly? What device naming are you trying to achieve based on
> what device drivers? In my naive view we have 200+ Ethernet drivers
> so listing Ethernet is not scalable. I'm curious what you're matching,
> how many drivers you need to list, and whether we could instead add a
> more general attribute...
> 
> Those questions aside, I'd like to get an ack from core driver experts
> like GregKH on this. IDK what (if any) rules there are on uevents.
> The merge window has started so we are very unlikely to hear from them
> now, all maintainers will be very busy. Please repost v3 in >=two weeks
> and CC Greg (and whoever else is reviewing driver core and/or uevent
> changes according to git logs).

We have some Mellanox Spectrum Switches here whose network interface 
names don't match their faceplate. They are called eth... and their 
numbering is also out of order, so we would like to rename them 
accordingly. They are using the mlxsw_spectrum driver.

Generally, you could do that once at boot time, but those Spectrum 
Switches also support port splitting. That means you can attach a 
breakout cable to one of its ports and then use the devlink tool to 
split the network interface into multiple ones in Linux. But the split 
network interfaces are then called eth... again:

root@SN2100:~# ip l | tail -n2
26: swp1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
     link/ether 50:6b:4b:9f:04:59 brd ff:ff:ff:ff:ff:ff
root@SN2100:~# devlink port split swp1 count 4
root@SN2100:~# ip l | tail -n8
27: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
     link/ether 50:6b:4b:9f:04:59 brd ff:ff:ff:ff:ff:ff
28: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
     link/ether 50:6b:4b:9f:04:5a brd ff:ff:ff:ff:ff:ff
29: eth2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
     link/ether 50:6b:4b:9f:04:5b brd ff:ff:ff:ff:ff:ff
30: eth3: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
     link/ether 50:6b:4b:9f:04:5c brd ff:ff:ff:ff:ff:ff

In their GitHub wiki [1], Mellanox recommends using a udev rule for 
renaming. udev has its implementation for retrieving the driver of a 
network interface, whereas OpenWrt's hotplug doesn't have such an 
implementation. With this patch, the driver name would be already 
available inside such hotplug scripts.

[1] 
https://github.com/Mellanox/mlxsw/wiki/Switch-Port-Configuration#using-udev-rules
Greg KH Dec. 6, 2024, 7:13 a.m. UTC | #3
On Thu, Dec 05, 2024 at 05:28:47PM +0100, Til Kaiser wrote:
> On 19.11.24 02:55, Jakub Kicinski wrote:
> > On Sat, 16 Nov 2024 17:30:30 +0100 Til Kaiser wrote:
> > > Currently, for uevent, the interface name and
> > > index are passed via shell variables.
> > > 
> > > This commit also passes the network device
> > > driver as a shell variable to uevent.
> > > 
> > > One way to retrieve a network interface's driver
> > > name is to resolve its sysfs device/driver symlink
> > > and then substitute leading directory components.
> > > 
> > > You could implement this yourself (e.g., like udev from
> > > systemd does) or with Linux tools by using a combination
> > > of readlink and shell substitution or basename.
> > > 
> > > The advantages of passing the driver directly through uevent are:
> > >   - Linux distributions don't need to implement additional code
> > >     to retrieve the driver when, e.g., interface events happen.

Why does the driver name matter for something like a network device?
I.e. why are network devices "special" here and unlike all other class
devices in the kernel like keyboards, mice, serial ports, etc.?

> > >   - There is no need to create additional process forks in shell
> > >     scripts for readlink or basename.

"Do it in the kernel and have someone else maintain it for me to make my
life easier in userspace" isn't always the best reason to do something.
Generally if "you can do this in userspace" means "you SHOULD do this in
userspace", it's not a good reason to add it to the kernel.

And note, driver names change!  How are you going to handle that?

> > >   - If a user wants to check his network interface's driver on the
> > >     command line, he can directly read it from the sysfs uevent file.

The symlink in sysfs shows this already, no need to
open/read/parse/close the uevent file.

Also, where did you now document this new user/kernel API that we have
to maintain for 40+ years?  And what userspace tool is going to use it?

But really, again, why are network devices so special that they need
this but no other type of device in the kernel does?  And what changed
to require this suddenly?

thanks,

greg k-h
diff mbox series

Patch

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 05cf5347f25e..67aad5ca82f8 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -2000,6 +2000,7 @@  EXPORT_SYMBOL_GPL(net_ns_type_operations);
 static int netdev_uevent(const struct device *d, struct kobj_uevent_env *env)
 {
 	const struct net_device *dev = to_net_dev(d);
+	const char *driver = netdev_drivername(dev);
 	int retval;
 
 	/* pass interface to uevent. */
@@ -2012,6 +2013,12 @@  static int netdev_uevent(const struct device *d, struct kobj_uevent_env *env)
 	 * and is what RtNetlink uses natively.
 	 */
 	retval = add_uevent_var(env, "IFINDEX=%d", dev->ifindex);
+	if (retval)
+		goto exit;
+
+	if (driver[0])
+		/* pass driver to uevent. */
+		retval = add_uevent_var(env, "DRIVER=%s", driver);
 
 exit:
 	return retval;