Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-30 Thread Jiri Pirko
Thu, Aug 30, 2018 at 12:13:40PM CEST, mar...@holtmann.org wrote:
>Hi Jiri,
>
> 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 
> ---
> 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.
>>> 
>>> it is not just "device" related since this is a sub-classification of 
>>> ARPHRD_ETHER type as a wrote above. Don't get hang up that this information 
>>> is part of struct device. The dev->dev.type contains strings like "wlan", 
>>> "bluetooth", "wimax", "gadget" etc. so that you can tell what kind of 
>>> Ethernet card you have.
>> 
>> I understand. But using dev->dev.type for that purpose seems like an
>> abuse. If this is subtype of netdev, it should not be stored in parent
>> device. I believe that you need to re-introduce this as a part of struct
>> net_device - preferably an enum - and that you can expose over rtnl (and
>> net-sysfs).
>
>it is not stored in the parent device. It is stored in the device of the 
>netdev.

Yeah. That is what I ment. "Device struct" associated with the
netdevice.

>
>As Stephen said, there is no device API. And why would I add the same info 
>again and bloat netdev?
>
>drivers/net/bonding/bond_main.c:SET_NETDEV_DEVTYPE(bond_dev, 
>_type);
>drivers/net/geneve.c:   SET_NETDEV_DEVTYPE(dev, _type);
>drivers/net/macsec.c:   SET_NETDEV_DEVTYPE(dev, _type);
>drivers/net/ppp/ppp_generic.c:  SET_NETDEV_DEVTYPE(dev, _type);
>drivers/net/usb/hso.c:  SET_NETDEV_DEVTYPE(net, _type);
>drivers/net/usb/usbnet.c:   SET_NETDEV_DEVTYPE(net, _type);
>drivers/net/usb/usbnet.c:   SET_NETDEV_DEVTYPE(net, _type);
>drivers/net/vxlan.c:SET_NETDEV_DEVTYPE(dev, _type);
>drivers/net/wimax/i2400m/usb.c: SET_NETDEV_DEVTYPE(net_dev, _type);
>drivers/net/wireless/intersil/prism54/islpci_dev.c: 
>SET_NETDEV_DEVTYPE(ndev, _type);
>drivers/staging/gdm724x/gdm_lte.c:  SET_NETDEV_DEVTYPE(net, 
>_type);
>drivers/usb/gadget/function/u_ether.c:  SET_NETDEV_DEVTYPE(net, _type);
>drivers/usb/gadget/function/u_ether.c:  SET_NETDEV_DEVTYPE(net, _type);
>net/8021q/vlan_dev.c:   SET_NETDEV_DEVTYPE(dev, _type);
>net/bluetooth/6lowpan.c:SET_NETDEV_DEVTYPE(netdev, _type);
>net/bluetooth/bnep/core.c:  SET_NETDEV_DEVTYPE(dev, _type);
>net/bridge/br_device.c: SET_NETDEV_DEVTYPE(dev, _type);
>net/dsa/slave.c:SET_NETDEV_DEVTYPE(slave_dev, _type);
>net/hsr/hsr_device.c:   SET_NETDEV_DEVTYPE(dev, _type);
>net/l2tp/l2tp_eth.c:SET_NETDEV_DEVTYPE(dev, _type);
>net/wireless/core.c:SET_NETDEV_DEVTYPE(dev, _type);
>
>So for all these devices we have to add duplicate information now?

The fact the things are done now it a wrong way does not justify
extending UAPI in the same wrong way. So yes, change them all if you
need it. The "ethernet-subtype" should not an attribute of "struct
device" but rather "struct net_device".

>
>I actually don't like that idea. I can rename it to IFLA_FOO or any other 
>proposal, but adding the same information twice is pointless.

So don't expose the "struct device" attribute over rtnetlink. Use
"struct device"-specific interface for that.

>
>>> We can revive the patches for adding this information as IFLA_INFO_KIND, 
>>> but last time they where reverted since NetworkManager did all the wrong 
>>> decisions based on that. That part is fixed now and we can put that back 
>>> and declare the sub-type classification of Ethernet device in two places. 
>>> Or we just take the information that we added a long time ago for exactly 
>>> this sub-classification and provide them to userspace via RTNL.
>> 
>> IFLA_INFO_KIND serves for different purpose. It is for drivers that
>> register rtnl_link_ops. I don't think it would be wise to mix it with
>> this.
>
>We could register rtnl_link_ops for these Ethernet 

Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-30 Thread Marcel Holtmann
Hi Jiri,

 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 
 ---
 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.
>> 
>> it is not just "device" related since this is a sub-classification of 
>> ARPHRD_ETHER type as a wrote above. Don't get hang up that this information 
>> is part of struct device. The dev->dev.type contains strings like "wlan", 
>> "bluetooth", "wimax", "gadget" etc. so that you can tell what kind of 
>> Ethernet card you have.
> 
> I understand. But using dev->dev.type for that purpose seems like an
> abuse. If this is subtype of netdev, it should not be stored in parent
> device. I believe that you need to re-introduce this as a part of struct
> net_device - preferably an enum - and that you can expose over rtnl (and
> net-sysfs).

it is not stored in the parent device. It is stored in the device of the netdev.

As Stephen said, there is no device API. And why would I add the same info 
again and bloat netdev?

drivers/net/bonding/bond_main.c:SET_NETDEV_DEVTYPE(bond_dev, 
_type);
drivers/net/geneve.c:   SET_NETDEV_DEVTYPE(dev, _type);
drivers/net/macsec.c:   SET_NETDEV_DEVTYPE(dev, _type);
drivers/net/ppp/ppp_generic.c:  SET_NETDEV_DEVTYPE(dev, _type);
drivers/net/usb/hso.c:  SET_NETDEV_DEVTYPE(net, _type);
drivers/net/usb/usbnet.c:   SET_NETDEV_DEVTYPE(net, _type);
drivers/net/usb/usbnet.c:   SET_NETDEV_DEVTYPE(net, _type);
drivers/net/vxlan.c:SET_NETDEV_DEVTYPE(dev, _type);
drivers/net/wimax/i2400m/usb.c: SET_NETDEV_DEVTYPE(net_dev, _type);
drivers/net/wireless/intersil/prism54/islpci_dev.c: 
SET_NETDEV_DEVTYPE(ndev, _type);
drivers/staging/gdm724x/gdm_lte.c:  SET_NETDEV_DEVTYPE(net, 
_type);
drivers/usb/gadget/function/u_ether.c:  SET_NETDEV_DEVTYPE(net, _type);
drivers/usb/gadget/function/u_ether.c:  SET_NETDEV_DEVTYPE(net, _type);
net/8021q/vlan_dev.c:   SET_NETDEV_DEVTYPE(dev, _type);
net/bluetooth/6lowpan.c:SET_NETDEV_DEVTYPE(netdev, _type);
net/bluetooth/bnep/core.c:  SET_NETDEV_DEVTYPE(dev, _type);
net/bridge/br_device.c: SET_NETDEV_DEVTYPE(dev, _type);
net/dsa/slave.c:SET_NETDEV_DEVTYPE(slave_dev, _type);
net/hsr/hsr_device.c:   SET_NETDEV_DEVTYPE(dev, _type);
net/l2tp/l2tp_eth.c:SET_NETDEV_DEVTYPE(dev, _type);
net/wireless/core.c:SET_NETDEV_DEVTYPE(dev, _type);

So for all these devices we have to add duplicate information now?

I actually don't like that idea. I can rename it to IFLA_FOO or any other 
proposal, but adding the same information twice is pointless.

>> We can revive the patches for adding this information as IFLA_INFO_KIND, but 
>> last time they where reverted since NetworkManager did all the wrong 
>> decisions based on that. That part is fixed now and we can put that back and 
>> declare the sub-type classification of Ethernet device in two places. Or we 
>> just take the information that we added a long time ago for exactly this 
>> sub-classification and provide them to userspace via RTNL.
> 
> IFLA_INFO_KIND serves for different purpose. It is for drivers that
> register rtnl_link_ops. I don't think it would be wise to mix it with
> this.

We could register rtnl_link_ops for these Ethernet drivers, but then we are 
duplicating the information as well.

Regards

Marcel



Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-29 Thread Jiri Pirko
Wed, Aug 29, 2018 at 05:24:28PM CEST, step...@networkplumber.org wrote:
>On Wed, 29 Aug 2018 09:18:55 +0200
>Jiri Pirko  wrote:
>
>> Tue, Aug 28, 2018 at 10:58:11PM CEST, mar...@holtmann.org wrote:
>> >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 
>> >---
>> > 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.

I understand. My point is to avoid exposing generic "struct device"
values over rtnetlink. It mixes apples and oranges.


Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-29 Thread Jiri Pirko
Wed, Aug 29, 2018 at 05:23:58PM CEST, mar...@holtmann.org wrote:
>Hi Jiri,
>
>>> 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 
>>> ---
>>> 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.
>
>it is not just "device" related since this is a sub-classification of 
>ARPHRD_ETHER type as a wrote above. Don't get hang up that this information is 
>part of struct device. The dev->dev.type contains strings like "wlan", 
>"bluetooth", "wimax", "gadget" etc. so that you can tell what kind of Ethernet 
>card you have.

I understand. But using dev->dev.type for that purpose seems like an
abuse. If this is subtype of netdev, it should not be stored in parent
device. I believe that you need to re-introduce this as a part of struct
net_device - preferably an enum - and that you can expose over rtnl (and
net-sysfs).

>
>We can revive the patches for adding this information as IFLA_INFO_KIND, but 
>last time they where reverted since NetworkManager did all the wrong decisions 
>based on that. That part is fixed now and we can put that back and declare the 
>sub-type classification of Ethernet device in two places. Or we just take the 
>information that we added a long time ago for exactly this sub-classification 
>and provide them to userspace via RTNL.

IFLA_INFO_KIND serves for different purpose. It is for drivers that
register rtnl_link_ops. I don't think it would be wise to mix it with
this.

Btw, could you wrap the sentences at 72 cols? Would be easier to read
that way.


>
>Regards
>
>Marcel
>


Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-29 Thread Marcel Holtmann
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 
>>> ---
>>> 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



Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-29 Thread Stephen Hemminger
On Wed, 29 Aug 2018 09:18:55 +0200
Jiri Pirko  wrote:

> Tue, Aug 28, 2018 at 10:58:11PM CEST, mar...@holtmann.org wrote:
> >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 
> >---
> > 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.


Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-29 Thread Marcel Holtmann
Hi Jiri,

>> 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 
>> ---
>> 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.

it is not just "device" related since this is a sub-classification of 
ARPHRD_ETHER type as a wrote above. Don't get hang up that this information is 
part of struct device. The dev->dev.type contains strings like "wlan", 
"bluetooth", "wimax", "gadget" etc. so that you can tell what kind of Ethernet 
card you have.

We can revive the patches for adding this information as IFLA_INFO_KIND, but 
last time they where reverted since NetworkManager did all the wrong decisions 
based on that. That part is fixed now and we can put that back and declare the 
sub-type classification of Ethernet device in two places. Or we just take the 
information that we added a long time ago for exactly this sub-classification 
and provide them to userspace via RTNL.

Regards

Marcel



Re: [PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-29 Thread Jiri Pirko
Tue, Aug 28, 2018 at 10:58:11PM CEST, mar...@holtmann.org wrote:
>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 
>---
> 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.


[PATCH] rtnetlink: expose value from SET_NETDEV_DEVTYPE via IFLA_DEVTYPE attribute

2018-08-28 Thread Marcel Holtmann
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 
---
 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 */
+#define IFLA_DEVTYPE IFLA_DEVTYPE
__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 24431e578310..bd288710f9bf 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -970,6 +970,14 @@ static size_t rtnl_xdp_size(void)
return xdp_size;
 }
 
+static size_t rtnl_devtype_size(const struct net_device *dev)
+{
+   if (!dev->dev.type || !dev->dev.type->name)
+   return 0;
+
+   return strlen(dev->dev.type->name) + 1;
+}
+
 static noinline size_t if_nlmsg_size(const struct net_device *dev,
 u32 ext_filter_mask)
 {
@@ -1017,6 +1025,7 @@ static noinline size_t if_nlmsg_size(const struct 
net_device *dev,
   + nla_total_size(4)  /* IFLA_CARRIER_DOWN_COUNT */
   + nla_total_size(4)  /* IFLA_MIN_MTU */
   + nla_total_size(4)  /* IFLA_MAX_MTU */
+  + rtnl_devtype_size(dev) /* IFLA_DEVTYPE */
   + 0;
 }
 
@@ -1679,6 +1688,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
nla_put_s32(skb, IFLA_NEW_IFINDEX, new_ifindex) < 0)
goto nla_put_failure;
 
+   if (dev->dev.type && dev->dev.type->name)
+   nla_put_string(skb, IFLA_DEVTYPE, dev->dev.type->name);
 
rcu_read_lock();
if (rtnl_fill_link_af(skb, dev, ext_filter_mask))
@@ -1738,6 +1749,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
[IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
[IFLA_MIN_MTU]  = { .type = NLA_U32 },
[IFLA_MAX_MTU]  = { .type = NLA_U32 },
+   [IFLA_DEVTYPE]  = { .type = NLA_STRING },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
-- 
2.14.4