netdev - Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA

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: <DA3E0CC5-DA62-4E8D-A28B-AA593D9DE709@holtmann.org>
Date:   Wed, 29 Aug 2018 08:31:31 -0700
From:   Marcel Holtmann <marcel@...tmann.org>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via
 IFLA_DEVTYPE attribute

Hi Stephen,

>>> The name value from SET_NETDEV_DEVTYPE only ended up in the uevent sysfs
>>> file as DEVTYPE= information. To avoid any kind of race conditions
>>> between netlink messages and reading from sysfs, it is useful to add the
>>> same string as new IFLA_DEVTYPE attribute included in the RTM_NEWLINK
>>> messages.
>>> 
>>> For network managing daemons that have to classify ARPHRD_ETHER network
>>> devices into different types (like Wireless LAN, Bluetooth etc.), this
>>> avoids the extra round trip to sysfs and parsing of the uevent file.
>>> 
>>> Signed-off-by: Marcel Holtmann <marcel@...tmann.org>
>>> ---
>>> include/uapi/linux/if_link.h |  2 ++
>>> net/core/rtnetlink.c         | 12 ++++++++++++
>>> 2 files changed, 14 insertions(+)
>>> 
>>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>>> index 43391e2d1153..781294972bb4 100644
>>> --- a/include/uapi/linux/if_link.h
>>> +++ b/include/uapi/linux/if_link.h
>>> @@ -166,6 +166,8 @@ enum {
>>> 	IFLA_NEW_IFINDEX,
>>> 	IFLA_MIN_MTU,
>>> 	IFLA_MAX_MTU,
>>> +	IFLA_DEVTYPE,		/* Name value from SET_NETDEV_DEVTYPE */  
>> 
>> This is not something netdev-related. dev->dev.type is struct device_type.
>> This is a generic "device" thing. Incorrect to expose over
>> netdev-specific API. Please use "device" API for this.
> 
> There is no device API in netlink. The whole point of this patch is to
> do it in one message. It might be a performance optimization, but I can't
> see how it could be a race condition. Devices set type before registering.

the only way right now to pick up the DEVTYPE= value is from the /sys/class/net/*/uevent file. That is based on the ifname and not the index. When udev + systemd start renaming things behind your back, your daemon does not have a clean one-shot way of getting that information. As stated, the information in DEVTYPE= are a sub-classification of ARPHRD_ETHER and allows to differentiate a wired Ethernet card from a WiFi interface, from a Bluetooth interface, from WiMAX and so on. They just happen to be in dev.dev_type data structure at the moment and I didn't want to duplicate that information.

I am actually fine doing this via IFLA_INFO_KIND since NetworkManager seems to be fixed now or do this via a different method or maybe just a different attribute name. I really just want to get the sub-classification of ARPHRD_ETHER that we need in userspace networking daemons from the kernel without having to go poke left and right over sysfs or interact with udev or systemd.

Regards

Marcel