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

[Tom Gundersen]

On Sun, Jan 5, 2014 at 3:38 AM, Marcel Holtmann <marcel at holtmann.org> wrote:
> 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.

Hm, I thought that was to fix false negatives. I.e., that some devices
with, say, DEVTYPE=bluetooth should in fact be treated as regular
ethernet?

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

Thanks for the hint. I'll take a look at starting from there, and then
rather hunt down any false positives we might produce.

Cheers,

Tom