| Message ID | 20220516215638.1787257-1-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [net-next] net: ifdefy the wireless pointers in struct net_device | expand |
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@kernel.org> 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?
Jakub Kicinski <kuba@kernel.org> writes: > 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@kernel.org> > --- > CC: johannes@sipsolutions.net > CC: alex.aring@gmail.com > CC: stefan@datenfreihafen.org > CC: mareklindner@neomailbox.ch > CC: sw@simonwunderlich.de > CC: a@unstable.cc > CC: sven@narfation.org > CC: linux-wireless@vger.kernel.org > CC: linux-wpan@vger.kernel.org [...] > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -8004,10 +8004,7 @@ int cfg80211_register_netdevice(struct net_device *dev); > * > * Requires the RTNL and wiphy mutex to be held. > */ > -static inline void cfg80211_unregister_netdevice(struct net_device *dev) > -{ > - cfg80211_unregister_wdev(dev->ieee80211_ptr); > -} > +void cfg80211_unregister_netdevice(struct net_device *dev); > > /** > * struct cfg80211_ft_event_params - FT Information Elements [...] > --- a/net/wireless/core.c > +++ b/net/wireless/core.c > @@ -1374,6 +1374,12 @@ int cfg80211_register_netdevice(struct net_device *dev) > } > EXPORT_SYMBOL(cfg80211_register_netdevice); > > +void cfg80211_unregister_netdevice(struct net_device *dev) > +{ > + cfg80211_unregister_wdev(dev->ieee80211_ptr); > +} > +EXPORT_SYMBOL(cfg80211_unregister_netdevice); Why moving this to a proper function? Just curious, I couldn't figure it out.
Hello. On 16.05.22 23:56, 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@kernel.org> > --- > CC: johannes@sipsolutions.net > CC: alex.aring@gmail.com > CC: stefan@datenfreihafen.org > CC: mareklindner@neomailbox.ch > CC: sw@simonwunderlich.de > CC: a@unstable.cc > CC: sven@narfation.org > CC: linux-wireless@vger.kernel.org > CC: linux-wpan@vger.kernel.org > --- > include/linux/netdevice.h | 8 ++++++-- > include/net/cfg80211.h | 5 +---- > include/net/cfg802154.h | 2 ++ > net/batman-adv/hard-interface.c | 2 ++ > net/wireless/core.c | 6 ++++++ > 5 files changed, 17 insertions(+), 6 deletions(-) For the ieee802154 changes: Acked-by: Stefan Schmidt <stefan@datenfreihafen.org> regards Stefan Schmidt
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@kernel.org> > > 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;
On Mon, 2022-05-16 at 14:56 -0700, Jakub Kicinski wrote: > > +#if IS_ENABLED(CONFIG_WIRELESS) > struct wireless_dev *ieee80211_ptr; > +#endif Technically, you should be able to use CONFIG_CFG80211 here, but in practice I'd really hope nobody enables WIRELESS without CFG80211 :) > +++ b/include/net/cfg80211.h > @@ -8004,10 +8004,7 @@ int cfg80211_register_netdevice(struct net_device *dev); > * > * Requires the RTNL and wiphy mutex to be held. > */ > -static inline void cfg80211_unregister_netdevice(struct net_device *dev) > -{ > - cfg80211_unregister_wdev(dev->ieee80211_ptr); > -} > +void cfg80211_unregister_netdevice(struct net_device *dev); Exported functions aren't free either - I think in this case I'd (slightly) prefer the extra ifdef. Anyway, we can do this, but I also like Florian's suggestion about the union, and sent an attempt at a disambiguation patch there. johannes
On Monday, 16 May 2022 23:56:38 CEST Jakub Kicinski wrote: > diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c > index 83fb51b6e299..15d2bb4cd301 100644 > --- a/net/batman-adv/hard-interface.c > +++ b/net/batman-adv/hard-interface.c > @@ -307,9 +307,11 @@ static bool batadv_is_cfg80211_netdev(struct net_device *net_device) > if (!net_device) > return false; > > +#if IS_ENABLED(CONFIG_WIRELESS) > /* cfg80211 drivers have to set ieee80211_ptr */ > if (net_device->ieee80211_ptr) > return true; > +#endif > > return false; > } Acked-by: Sven Eckelmann <sven@narfation.org> On Tuesday, 17 May 2022 09:48:24 CEST Johannes Berg wrote: > 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. Yes, if something like netdev_is_wireless would be available then we could change it to: diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 35fadb924849..50a53e3364bf 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -294,26 +294,6 @@ static bool batadv_is_wext_netdev(struct net_device *net_device) return false; } -/** - * batadv_is_cfg80211_netdev() - check if the given net_device struct is a - * cfg80211 wifi interface - * @net_device: the device to check - * - * Return: true if the net device is a cfg80211 wireless device, false - * otherwise. - */ -static bool batadv_is_cfg80211_netdev(struct net_device *net_device) -{ - if (!net_device) - return false; - - /* cfg80211 drivers have to set ieee80211_ptr */ - if (net_device->ieee80211_ptr) - return true; - - return false; -} - /** * batadv_wifi_flags_evaluate() - calculate wifi flags for net_device * @net_device: the device to check @@ -328,7 +308,7 @@ static u32 batadv_wifi_flags_evaluate(struct net_device *net_device) if (batadv_is_wext_netdev(net_device)) wifi_flags |= BATADV_HARDIF_WIFI_WEXT_DIRECT; - if (batadv_is_cfg80211_netdev(net_device)) + if (netdev_is_wireless(net_device)) wifi_flags |= BATADV_HARDIF_WIFI_CFG80211_DIRECT; real_netdev = batadv_get_real_netdevice(net_device); @@ -341,7 +321,7 @@ static u32 batadv_wifi_flags_evaluate(struct net_device *net_device) if (batadv_is_wext_netdev(real_netdev)) wifi_flags |= BATADV_HARDIF_WIFI_WEXT_INDIRECT; - if (batadv_is_cfg80211_netdev(real_netdev)) + if (netdev_is_wireless(real_netdev)) wifi_flags |= BATADV_HARDIF_WIFI_CFG80211_INDIRECT; out:
On Tue, 17 May 2022 07:36:54 +0300 Kalle Valo wrote: > > +void cfg80211_unregister_netdevice(struct net_device *dev) > > +{ > > + cfg80211_unregister_wdev(dev->ieee80211_ptr); > > +} > > +EXPORT_SYMBOL(cfg80211_unregister_netdevice); > > Why moving this to a proper function? Just curious, I couldn't figure it > out. Sorry, I went too far with the "explain what not why". The header is included in places which get built without WIRELESS while the C source is not.
On Tue, 17 May 2022 09:48:24 +0200 Johannes Berg wrote: > 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@kernel.org> > > > > 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; Can we use enum netdev_ml_priv_type netdev::ml_priv and netdev::ml_priv_type for this?
On Tue, 17 May 2022 09:51:31 +0200 Johannes Berg wrote: > On Mon, 2022-05-16 at 14:56 -0700, Jakub Kicinski wrote: > > > > +#if IS_ENABLED(CONFIG_WIRELESS) > > struct wireless_dev *ieee80211_ptr; > > +#endif > > Technically, you should be able to use CONFIG_CFG80211 here, but in > practice I'd really hope nobody enables WIRELESS without CFG80211 :) ack > > +++ b/include/net/cfg80211.h > > @@ -8004,10 +8004,7 @@ int cfg80211_register_netdevice(struct net_device *dev); > > * > > * Requires the RTNL and wiphy mutex to be held. > > */ > > -static inline void cfg80211_unregister_netdevice(struct net_device *dev) > > -{ > > - cfg80211_unregister_wdev(dev->ieee80211_ptr); > > -} > > +void cfg80211_unregister_netdevice(struct net_device *dev); > > Exported functions aren't free either - I think in this case I'd > (slightly) prefer the extra ifdef. fine > Anyway, we can do this, but I also like Florian's suggestion about the > union, and sent an attempt at a disambiguation patch there. Would you be willing to do that as a follow up? Are you talking about wifi only or all the proto pointers? As a netdev maintainer I'd like to reduce the divergence in whether the proto pointers are ifdef'd or not.
On Tue, 2022-05-17 at 10:44 -0700, Jakub Kicinski wrote: > > Would you be willing to do that as a follow up? > Sure. > Are you talking about > wifi only or all the proto pointers? Well it only makes sense if at least two protocols join forces :-) > As a netdev maintainer I'd like to reduce the divergence in whether > the proto pointers are ifdef'd or not. > Sure, no objection to the ifdef. johannes
On Tue, 2022-05-17 at 10:37 -0700, Jakub Kicinski wrote: > > > > 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; > > Can we use enum netdev_ml_priv_type netdev::ml_priv and > netdev::ml_priv_type for this? > Hm, yeah, I guess we can. I think I'd prefer something along the lines of the below, then we don't even need the ifdef. johannes diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index eaf66e57d891..4bd81767c058 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1702,6 +1702,7 @@ enum netdev_priv_flags { enum netdev_ml_priv_type { ML_PRIV_NONE, ML_PRIV_CAN, + ML_PRIV_WIFI, }; /** @@ -2127,7 +2128,6 @@ struct net_device { #if IS_ENABLED(CONFIG_AX25) void *ax25_ptr; #endif - struct wireless_dev *ieee80211_ptr; struct wpan_dev *ieee802154_ptr; #if IS_ENABLED(CONFIG_MPLS_ROUTING) struct mpls_dev __rcu *mpls_ptr; @@ -2235,7 +2235,10 @@ struct net_device { possible_net_t nd_net; /* mid-layer private */ - void *ml_priv; + union { + void *ml_priv; + struct wireless_dev *ieee80211_ptr; + }; enum netdev_ml_priv_type ml_priv_type; union { diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 83fb51b6e299..7b063adacaa6 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -307,11 +307,7 @@ static bool batadv_is_cfg80211_netdev(struct net_device *net_device) if (!net_device) return false; - /* cfg80211 drivers have to set ieee80211_ptr */ - if (net_device->ieee80211_ptr) - return true; - - return false; + return netdev_get_ml_priv(net_device, ML_PRIV_WIFI); } /** diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 4980c3a50475..50154f7879b6 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -1997,7 +1997,7 @@ int netdev_register_kobject(struct net_device *ndev) *groups++ = &netstat_group; #if IS_ENABLED(CONFIG_WIRELESS_EXT) || IS_ENABLED(CONFIG_CFG80211) - if (ndev->ieee80211_ptr) + if (netdev_get_ml_priv(ndev, ML_PRIV_WIFI)) *groups++ = &wireless_group; #if IS_ENABLED(CONFIG_WIRELESS_EXT) else if (ndev->wireless_handlers) diff --git a/net/wireless/core.c b/net/wireless/core.c index 3e5d12040726..9024bd9f7d46 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -1369,6 +1369,12 @@ int cfg80211_register_netdevice(struct net_device *dev) lockdep_assert_held(&rdev->wiphy.mtx); + /* + * This lets us identify our netdevs in the future, + * the driver already set dev->ieee80211_ptr. + */ + netdev_set_ml_priv(dev, wdev, ML_PRIV_WIFI); + /* we'll take care of this */ wdev->registered = true; wdev->registering = true; @@ -1390,7 +1396,7 @@ 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; + struct wireless_dev *wdev = netdev_get_ml_priv(dev, ML_PRIV_WIFI); struct cfg80211_registered_device *rdev; struct cfg80211_sched_scan_request *pos, *tmp; diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 8d528b5945d0..3a5a7183b959 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -182,9 +182,12 @@ __cfg80211_rdev_from_attrs(struct net *netns, struct nlattr **attrs) netdev = __dev_get_by_index(netns, ifindex); if (netdev) { - if (netdev->ieee80211_ptr) - tmp = wiphy_to_rdev( - netdev->ieee80211_ptr->wiphy); + struct wireless_dev *wdev; + + wdev = netdev_get_ml_priv(netdev, ML_PRIV_WIFI); + + if (wdev) + tmp = wiphy_to_rdev(wdev->wiphy); else tmp = NULL; @@ -2972,15 +2975,17 @@ static int nl80211_dump_wiphy_parse(struct sk_buff *skb, struct net_device *netdev; struct cfg80211_registered_device *rdev; int ifidx = nla_get_u32(tb[NL80211_ATTR_IFINDEX]); + struct wireless_dev *wdev; netdev = __dev_get_by_index(sock_net(skb->sk), ifidx); if (!netdev) { ret = -ENODEV; goto out; } - if (netdev->ieee80211_ptr) { - rdev = wiphy_to_rdev( - netdev->ieee80211_ptr->wiphy); + + wdev = netdev_is_wireless(netdev, ML_PRIV_WIFI); + if (wdev) { + rdev = wiphy_to_rdev(wdev->wiphy); state->filter_wiphy = rdev->wiphy_idx; } } @@ -3364,8 +3369,9 @@ 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) - rdev = wiphy_to_rdev(netdev->ieee80211_ptr->wiphy); + wdev = netdev_is_wireless(netdev); + if (wdev) + rdev = wiphy_to_rdev(wdev->wiphy); else netdev = NULL; } diff --git a/net/wireless/scan.c b/net/wireless/scan.c index b9678801d848..e1bd3f624e1b 100644 --- a/net/wireless/scan.c +++ b/net/wireless/scan.c @@ -2715,6 +2715,7 @@ static struct cfg80211_registered_device * cfg80211_get_dev_from_ifindex(struct net *net, int ifindex) { struct cfg80211_registered_device *rdev; + struct wireless_dev *wdev; struct net_device *dev; ASSERT_RTNL(); @@ -2722,8 +2723,9 @@ cfg80211_get_dev_from_ifindex(struct net *net, int ifindex) dev = dev_get_by_index(net, ifindex); if (!dev) return ERR_PTR(-ENODEV); - if (dev->ieee80211_ptr) - rdev = wiphy_to_rdev(dev->ieee80211_ptr->wiphy); + wdev = netdev_get_ml_priv(dev, ML_PRIV_WIFI); + if (wdev) + rdev = wiphy_to_rdev(wdev->wiphy); else rdev = ERR_PTR(-ENODEV); dev_put(dev); diff --git a/net/wireless/wext-proc.c b/net/wireless/wext-proc.c index cadcf8613af2..a7d903729d2e 100644 --- a/net/wireless/wext-proc.c +++ b/net/wireless/wext-proc.c @@ -40,7 +40,7 @@ static void wireless_seq_printf_stats(struct seq_file *seq, stats = &nullstats; #endif #ifdef CONFIG_CFG80211 - if (dev->ieee80211_ptr) + if (netdev_get_ml_priv(dev, ML_PRIV_WIFI)) stats = &nullstats; #endif }
Hi, On Mon, May 16, 2022 at 10:13 PM Florian Fainelli <f.fainelli@gmail.com> 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@kernel.org> > > 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? note that ieee802154 has also functionality like __dev_get_by_index() and checks via "if (netdev->ieee802154_ptr)" if it's a wpan interface or not, guess the solution would be like it's done in wireless then. - Alex
On Tue, 17 May 2022 15:33:02 -0400 Alexander Aring wrote: > > 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? > > note that ieee802154 has also functionality like __dev_get_by_index() > and checks via "if (netdev->ieee802154_ptr)" if it's a wpan interface > or not, guess the solution would be like it's done in wireless then. Ack, but that code must live somewhere under #if IS_ENABLED(CONFIG_IEEE802154) || IS_ENABLED(CONFIG_6LOWPAN) otherwise I think I'd see a build failure. I guess a nice thing about having the typed pointers is that we can depend on the compiler for basic checking :)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 536321691c72..f0604863f18c 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2119,6 +2119,8 @@ struct net_device { /* Protocol-specific pointers */ + struct in_device __rcu *ip_ptr; + struct inet6_dev __rcu *ip6_ptr; #if IS_ENABLED(CONFIG_VLAN_8021Q) struct vlan_info __rcu *vlan_info; #endif @@ -2131,16 +2133,18 @@ struct net_device { #if IS_ENABLED(CONFIG_ATALK) void *atalk_ptr; #endif - struct in_device __rcu *ip_ptr; #if IS_ENABLED(CONFIG_DECNET) struct dn_dev __rcu *dn_ptr; #endif - struct inet6_dev __rcu *ip6_ptr; #if IS_ENABLED(CONFIG_AX25) void *ax25_ptr; #endif +#if IS_ENABLED(CONFIG_WIRELESS) struct wireless_dev *ieee80211_ptr; +#endif +#if IS_ENABLED(CONFIG_IEEE802154) || IS_ENABLED(CONFIG_6LOWPAN) struct wpan_dev *ieee802154_ptr; +#endif #if IS_ENABLED(CONFIG_MPLS_ROUTING) struct mpls_dev __rcu *mpls_ptr; #endif diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 68713388b617..a4a7fc3241cf 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -8004,10 +8004,7 @@ int cfg80211_register_netdevice(struct net_device *dev); * * Requires the RTNL and wiphy mutex to be held. */ -static inline void cfg80211_unregister_netdevice(struct net_device *dev) -{ - cfg80211_unregister_wdev(dev->ieee80211_ptr); -} +void cfg80211_unregister_netdevice(struct net_device *dev); /** * struct cfg80211_ft_event_params - FT Information Elements diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index 85f9e8417688..d8d8719315fd 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -373,6 +373,7 @@ struct wpan_dev { #define to_phy(_dev) container_of(_dev, struct wpan_phy, dev) +#if IS_ENABLED(CONFIG_IEEE802154) || IS_ENABLED(CONFIG_6LOWPAN) static inline int wpan_dev_hard_header(struct sk_buff *skb, struct net_device *dev, const struct ieee802154_addr *daddr, @@ -383,6 +384,7 @@ wpan_dev_hard_header(struct sk_buff *skb, struct net_device *dev, return wpan_dev->header_ops->create(skb, dev, daddr, saddr, len); } +#endif struct wpan_phy * wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size); diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 83fb51b6e299..15d2bb4cd301 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -307,9 +307,11 @@ static bool batadv_is_cfg80211_netdev(struct net_device *net_device) if (!net_device) return false; +#if IS_ENABLED(CONFIG_WIRELESS) /* cfg80211 drivers have to set ieee80211_ptr */ if (net_device->ieee80211_ptr) return true; +#endif return false; } diff --git a/net/wireless/core.c b/net/wireless/core.c index f08d4b3bb148..a24944e6d01e 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -1374,6 +1374,12 @@ int cfg80211_register_netdevice(struct net_device *dev) } EXPORT_SYMBOL(cfg80211_register_netdevice); +void cfg80211_unregister_netdevice(struct net_device *dev) +{ + cfg80211_unregister_wdev(dev->ieee80211_ptr); +} +EXPORT_SYMBOL(cfg80211_unregister_netdevice); + static int cfg80211_netdev_notifier_call(struct notifier_block *nb, unsigned long state, void *ptr) {
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@kernel.org> --- CC: johannes@sipsolutions.net CC: alex.aring@gmail.com CC: stefan@datenfreihafen.org CC: mareklindner@neomailbox.ch CC: sw@simonwunderlich.de CC: a@unstable.cc CC: sven@narfation.org CC: linux-wireless@vger.kernel.org CC: linux-wpan@vger.kernel.org --- include/linux/netdevice.h | 8 ++++++-- include/net/cfg80211.h | 5 +---- include/net/cfg802154.h | 2 ++ net/batman-adv/hard-interface.c | 2 ++ net/wireless/core.c | 6 ++++++ 5 files changed, 17 insertions(+), 6 deletions(-)