On Mon, 2022-05-16 at 19:12 -0700, Florian Fainelli wrote: > > On 5/16/2022 2:56 PM, Jakub Kicinski wrote: > > Most protocol-specific pointers in struct net_device are under > > a respective ifdef. Wireless is the notable exception. Since > > there's a sizable number of custom-built kernels for datacenter > > workloads which don't build wireless it seems reasonable to > > ifdefy those pointers as well. > > > > While at it move IPv4 and IPv6 pointers up, those are special > > for obvious reasons. > > > > Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx> > > Could not we move to an union of pointers in the future since in many > cases a network device can only have one of those pointers at any given > time? Then at the very least we'd need some kind of type that we can assign to disambiguate, because today e.g. we have a netdev notifier (and other code) that could get a non-wireless netdev and check like this: static int cfg80211_netdev_notifier_call(struct notifier_block *nb, unsigned long state, void *ptr) { struct net_device *dev = netdev_notifier_info_to_dev(ptr); struct wireless_dev *wdev = dev->ieee80211_ptr; [...] if (!wdev) return NOTIFY_DONE; We could probably use the netdev->dev.type for this though, that's just a pointer we can compare to. We'd have to set it differently (today cfg80211 sets it based on whether or not you have ieee80211_ptr, we'd have to turn that around), but that's not terribly hard, especially since wireless drivers now have to call cfg80211_register_netdevice() anyway, rather than register_netdevice() directly. Something like the patch below might do that, but I haven't carefully checked it yet, nor checked if there are any paths in mac80211/drivers that might be doing this check - and it looks from Jakub's patch that batman code would like to check this too. johannes diff --git a/net/wireless/core.c b/net/wireless/core.c index 3e5d12040726..6ea2a597f4ca 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -1192,7 +1192,7 @@ void cfg80211_unregister_wdev(struct wireless_dev *wdev) } EXPORT_SYMBOL(cfg80211_unregister_wdev); -static const struct device_type wiphy_type = { +const struct device_type wiphy_type = { .name = "wlan", }; @@ -1369,6 +1369,9 @@ int cfg80211_register_netdevice(struct net_device *dev) lockdep_assert_held(&rdev->wiphy.mtx); + /* this lets us identify our netdevs in the future */ + SET_NETDEV_DEVTYPE(dev, &wiphy_type); + /* we'll take care of this */ wdev->registered = true; wdev->registering = true; @@ -1394,7 +1397,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb, struct cfg80211_registered_device *rdev; struct cfg80211_sched_scan_request *pos, *tmp; - if (!wdev) + if (!netdev_is_wireless(dev)) return NOTIFY_DONE; rdev = wiphy_to_rdev(wdev->wiphy); @@ -1403,7 +1406,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb, switch (state) { case NETDEV_POST_INIT: - SET_NETDEV_DEVTYPE(dev, &wiphy_type); wdev->netdev = dev; /* can only change netns with wiphy */ dev->features |= NETIF_F_NETNS_LOCAL; diff --git a/net/wireless/core.h b/net/wireless/core.h index 2c195067ddff..e256ea5caf49 100644 --- a/net/wireless/core.h +++ b/net/wireless/core.h @@ -219,6 +219,13 @@ void cfg80211_init_wdev(struct wireless_dev *wdev); void cfg80211_register_wdev(struct cfg80211_registered_device *rdev, struct wireless_dev *wdev); +extern const struct device_type wiphy_type; + +static inline bool netdev_is_wireless(struct net_device *dev) +{ + return dev && dev->dev.type == &wiphy_type && dev->ieee80211_ptr; +} + static inline void wdev_lock(struct wireless_dev *wdev) __acquires(wdev) { diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 342dfefb6eca..58bc3750c380 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -182,7 +182,7 @@ __cfg80211_rdev_from_attrs(struct net *netns, struct nlattr **attrs) netdev = __dev_get_by_index(netns, ifindex); if (netdev) { - if (netdev->ieee80211_ptr) + if (netdev_is_wireless(netdev)) tmp = wiphy_to_rdev( netdev->ieee80211_ptr->wiphy); else @@ -2978,7 +2978,7 @@ static int nl80211_dump_wiphy_parse(struct sk_buff *skb, ret = -ENODEV; goto out; } - if (netdev->ieee80211_ptr) { + if (netdev_is_wireless(netdev)) { rdev = wiphy_to_rdev( netdev->ieee80211_ptr->wiphy); state->filter_wiphy = rdev->wiphy_idx; @@ -3364,7 +3364,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info) int ifindex = nla_get_u32(info->attrs[NL80211_ATTR_IFINDEX]); netdev = __dev_get_by_index(genl_info_net(info), ifindex); - if (netdev && netdev->ieee80211_ptr) + if (netdev_is_wireless(netdev)) rdev = wiphy_to_rdev(netdev->ieee80211_ptr->wiphy); else netdev = NULL;