Re: [PATCH 1/2] ieee802154: register a device type for the wpan netdev

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Commit message mischaracterizes both the pre-patch state and
  the effect of the change in net/ieee802154/core.c.

--

ieee802154: register a device type for the wpan netdev

This adds a wpan_dev_type with only a .name field and assigns it from a
new NETDEV_POST_INIT case in cfg802154_netdev_notifier_call(), replacing
the "TODO NETDEV_DEVTYPE" comment.  It mirrors what cfg80211 does with
wiphy_type in cfg80211_netdev_notifier_call().

> ieee802154: register a device type for the wpan netdev
>
>     The cfg802154 netdev notifier never sets a device type, so wpan
>     interfaces lack a sysfs "type". Mirror cfg80211 by calling
>     SET_NETDEV_DEVTYPE() in a NETDEV_POST_INIT case with a new
>     wpan_dev_type.

[Severity: Medium]
Is the statement "wpan interfaces lack a sysfs 'type'" accurate?  Every
netdev already gets a type file unconditionally from net/core/net-sysfs.c:

    NETDEVICE_SHOW_RO(type, fmt_dec);
    ...
    static struct attribute *net_class_attrs[] __ro_after_init = {
            ...
            &dev_attr_type.attr,

That attribute shows struct net_device::type, which for wpan interfaces is
already 804 (ARPHRD_IEEE802154), so /sys/class/net/<if>/type exists before
this patch.

The macro used here touches a different field:

    #define SET_NETDEV_DEVTYPE(net, devtype)        ((net)->dev.type = (devtype))

and a struct device_type carrying only .name creates no sysfs attribute
file at all.  The one observable effect is in dev_uevent() in
drivers/base/core.c:

    if (dev->type && dev->type->name)
            add_uevent_var(env, "DEVTYPE=%s", dev->type->name);

So the change adds a DEVTYPE=wpan key to the netdev uevent environment
(also visible in /sys/class/net/<if>/uevent), which is what the cfg80211
precedent does with .name = "wlan".

Could the changelog be reworded to describe that effect instead, and also
mention that this is a new userspace-visible uevent key?  udev and
systemd-networkd rules that match on Type= will now see wpan for
interfaces that previously exported no DEVTYPE.

The code change itself looks fine and consistent with
cfg80211_netdev_notifier_call().

> diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
> index c0b8712018a16..c6213412d2914 100644
> --- a/net/ieee802154/core.c
> +++ b/net/ieee802154/core.c
> @@ -272,6 +272,10 @@ void cfg802154_dev_free(struct cfg802154_registered_device *rdev)
>  	kfree(rdev);
>  }
>  
> +static const struct device_type wpan_dev_type = {
> +	.name	= "wpan",
> +};
> +
>  static void
>  cfg802154_update_iface_num(struct cfg802154_registered_device *rdev,
>  			   int iftype, int num)
> @@ -296,7 +300,9 @@ static int cfg802154_netdev_notifier_call(struct notifier_block *nb,
>  	/* TODO WARN_ON unspec type */
>  
>  	switch (state) {
> -		/* TODO NETDEV_DEVTYPE */
> +	case NETDEV_POST_INIT:
> +		SET_NETDEV_DEVTYPE(dev, &wpan_dev_type);
> +		break;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260905212007.304875-1-adrianox%40gmail.com