[systemd-devel] [RFC][PATCH] net: set default DEVTYPE for all ethernet based devices

[Marcel Holtmann]

Hi Tom,

>>> In systemd's networkd and udevd, we would like to give the administrator a
>>> simple way to filter ethernet devices by their DEVTYPE. In order to avoid
>>> having a special treatment of the case where DEVTYPE=(null), initialize it to
>>> a default value, "ethernet", in the kernel.
>>> 
>>> Signed-off-by: Tom Gundersen <teg at jklm.no>
>>> Cc: Marcel Holtmann <marcel at holtmann.org>
>>> Cc: Greg KH <gregkh at linuxfoundation.org>
>>> ---
>>> 
>>> Hi Greg and Marcel,
>>> 
>>> This patch seems to do the right thing for me. Any comments before I send it
>>> off to LKML?
>>> 
>>> I suppose it may make sense to hide this behind a kernel option in case we are
>>> worried about breaking existing users (but if ConnMan is adapted, I don't know
>>> of any other issues, NetworkManager is not affected at least).
>>> 
>>> Cheers,
>>> 
>>> Tom
>>> 
>>> net/core/dev.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>> 
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index ba3b7ea..62881e0 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -6215,6 +6215,10 @@ void netdev_freemem(struct net_device *dev)
>>>              kfree(addr);
>>> }
>>> 
>>> +static const struct device_type ethernet_type = {
>>> +     .name = "ethernet",
>>> +};
>>> +
>>> /**
>>> *    alloc_netdev_mqs - allocate network device
>>> *    @sizeof_priv:   size of private data to allocate space for
>>> @@ -6305,6 +6309,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>>>              goto free_all;
>>> #endif
>>> 
>>> +     SET_NETDEV_DEVTYPE(dev, &ethernet_type);
>>> +
>>>      strcpy(dev->name, name);
>>>      dev->group = INIT_NETDEV_GROUP;
>>>      if (!dev->ethtool_ops)
>> 
>> this means that every single netdev is defaulting to Ethernet. This includes also the fake ones like IrDA or raw IP ones like PPP or TUN devices. I do not think that is something we really want here.
> 
> Hm, so then the assumption I first worked under (DEVTYPE==(null) means
> ethernet) is not really correct. Doesn't this give you problems in
> ConnMan at the moment?

that is why I mentioned ARPHRD_* earlier. It is important to check that value as well. You will need both at some point.

> What do you think would be the correct behavior? To use this patch,
> but also patch up IrDA, PPP, TUN and whatever else to set real
> DEVTYPE's; or to leave them as essentially 'unknown', and go through
> the ethernet drivers and set them to DEVTYPE='ethernet' one-by-one?

The kernel has an alloc_etherdev_mqs() that might be a better starting point. However even then, you might still need to fix some false positives.

Regards

Marcel