| Message ID | 20200826131709.25530-1-shay.bar@celeno.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | nl80211: Trigger channel switch from driver | expand |
Hi Aviad, Shay, > This patch add channel switch event from driver to hostap. > same as can be triggered from US via hostapd_cli chan_switch. (had to think about "US" here for a bit, "channel switch" totally sent my brain on the regulatory road ... heh) > using the already existing NL80211_CMD_CHANNEL_SWITCH. I'm a little confused as to how this would work - did the driver just *do* a channel switch? Or does it want to do a channel switch, and then hostapd has to deal with it? Is it able to? What happens if it doesn't do anything when this happens, e.g. an older version? > Signed-off-by: Aviad Brikman <aviad.brikman@celeno.com> > Signed-off-by: Shay Bar <shay.bar@celeno.com> > --- > include/net/cfg80211.h | 11 ++++++ > net/wireless/nl80211.c | 83 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+) > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > index d9e6b9fbd95b..ae02d96eb8ec 100644 > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -7416,6 +7416,17 @@ void cfg80211_ch_switch_started_notify(struct net_device *dev, > struct cfg80211_chan_def *chandef, > u8 count); > > +/* > + * cfg80211_ch_switch - trigger channel switch from driver > + * same as is can be triggered from hostapd_cli chan_switch Not sure "same as ... hostapd_cli chan_switch" makes a lot of sense here in the kernel-doc, tbh. Also, you say "trigger" here, but ... > + * @dev: the device which switched channels "switched" here, which didn't help my above question at all. > + * @chandef: the new channel definition > + * @csa_count: the number of TBTTs until the channel switch happens though I guess this makes it a bit clearer, unless it can be zero? :) > + */ > +bool cfg80211_ch_switch(struct net_device *dev, > + struct cfg80211_chan_def *chandef, > + u8 csa_count); nit: indentation > + if (chandef->width == NL80211_CHAN_WIDTH_40) { > + enum nl80211_channel_type chan_type = NL80211_CHAN_HT40MINUS; > + > + if (chandef->center_freq1 > chandef->chan->center_freq) > + chan_type = NL80211_CHAN_HT40PLUS; > + > + if (nla_put_u32(msg, NL80211_ATTR_WIPHY_CHANNEL_TYPE, > + chan_type)) > + goto nla_put_failure; > + } This a bit ties in with the "compatibility" question above - does older hostapd even understand this? I'd suspect not, and then I'm not sure why you'd include these attributes? > The information transmitted is intended only for the person or entity > to which it is addressed and may contain confidential and/or > privileged material. Any retransmission, dissemination, copying or > other use of, or taking of any action in reliance upon this > information is prohibited. If you received this in error, please > contact the sender and delete the material from any computer. Nothing > contained herein shall be deemed as a representation, warranty or a > commitment by Celeno. No warranties are expressed or implied, > including, but not limited to, any implied warranties of non- > infringement, merchantability and fitness for a particular purpose. Hm. I guess I'm the "intended [...] person or entity" but I'm still a bit wary about applying patches with that, I guess. Thanks, johannes
On 28/09/2020 14:30, Johannes Berg wrote >> using the already existing NL80211_CMD_CHANNEL_SWITCH. > I'm a little confused as to how this would work - did the driver just > *do* a channel switch? Or does it want to do a channel switch, and then > hostapd has to deal with it? Is it able to? What happens if it doesn't > do anything when this happens, e.g. an older version? The driver "wants" to do channel switch and hostapd, once receiving the above command, will take the same action as if it received a "hostapd_cli chan_switch. The driver will not assume channel was switched upon sending this command (it will have to wait until hostapd will handle the entire channel switch "loop"). There are many reasons why kernel driver will want to trigger a channel switch (e.g. dynamic channel selection). With older version of hostapd, it will not be able to handle this command and nothing will happen. >> +/* >> + * cfg80211_ch_switch - trigger channel switch from driver >> + * same as is can be triggered from hostapd_cli chan_switch > Not sure "same as ... hostapd_cli chan_switch" makes a lot of sense here > in the kernel-doc, tbh. can remove the "hostapd_cli chan_switch" reference. > Also, you say "trigger" here, but ... > >> + * @dev: the device which switched channels > "switched" here, which didn't help my above question at all. Agree "wants to switch" is clearer >> + * @chandef: the new channel definition >> + * @csa_count: the number of TBTTs until the channel switch happens > though I guess this makes it a bit clearer, unless it can be zero? :) csa_count of 0/1 is valid, but it means the channel switch will happen at the current TBTT (see ieee80211_set_csa_beacon()) >> + */ >> +bool cfg80211_ch_switch(struct net_device *dev, >> + struct cfg80211_chan_def *chandef, >> + u8 csa_count); > nit: indentation will fix >> + if (chandef->width == NL80211_CHAN_WIDTH_40) { >> + enum nl80211_channel_type chan_type = NL80211_CHAN_HT40MINUS; >> + >> + if (chandef->center_freq1 > chandef->chan->center_freq) >> + chan_type = NL80211_CHAN_HT40PLUS; >> + >> + if (nla_put_u32(msg, NL80211_ATTR_WIPHY_CHANNEL_TYPE, >> + chan_type)) >> + goto nla_put_failure; >> + } > This a bit ties in with the "compatibility" question above - does older > hostapd even understand this? I'd suspect not, and then I'm not sure why > you'd include these attributes? older hostapd will not handle NL80211_CMD_CHANNEL_SWITCH anyway. >> The information transmitted is intended only for the person or entity >> to which it is addressed and may contain confidential and/or >> privileged material. Any retransmission, dissemination, copying or >> other use of, or taking of any action in reliance upon this >> information is prohibited. If you received this in error, please >> contact the sender and delete the material from any computer. Nothing >> contained herein shall be deemed as a representation, warranty or a >> commitment by Celeno. No warranties are expressed or implied, >> including, but not limited to, any implied warranties of non- >> infringement, merchantability and fitness for a particular purpose. > Hm. I guess I'm the "intended [...] person or entity" but I'm still a > bit wary about applying patches with that, I guess. :) the disclaimer will be removed from now on.
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index d9e6b9fbd95b..ae02d96eb8ec 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -7416,6 +7416,17 @@ void cfg80211_ch_switch_started_notify(struct net_device *dev, struct cfg80211_chan_def *chandef, u8 count); +/* + * cfg80211_ch_switch - trigger channel switch from driver + * same as is can be triggered from hostapd_cli chan_switch + * @dev: the device which switched channels + * @chandef: the new channel definition + * @csa_count: the number of TBTTs until the channel switch happens + */ +bool cfg80211_ch_switch(struct net_device *dev, + struct cfg80211_chan_def *chandef, + u8 csa_count); + /** * ieee80211_operating_class_to_band - convert operating class to band * diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 19dc0ee807f6..c9ed073e5ab3 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -16854,6 +16854,89 @@ void cfg80211_ch_switch_started_notify(struct net_device *dev, } EXPORT_SYMBOL(cfg80211_ch_switch_started_notify); +static void nl80211_ch_switch(struct cfg80211_registered_device *rdev, + struct net_device *netdev, + struct cfg80211_chan_def *chandef, + u8 csa_count, + gfp_t gfp) +{ + struct sk_buff *msg; + void *hdr; + + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp); + if (!msg) + return; + + hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_CHANNEL_SWITCH); + if (!hdr) { + nlmsg_free(msg); + return; + } + + if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx)) + goto nla_put_failure; + + if (nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex)) + goto nla_put_failure; + + if (nla_put_u32(msg, NL80211_ATTR_WIPHY_FREQ, + chandef->chan->center_freq)) + goto nla_put_failure; + + if (nla_put_u32(msg, NL80211_ATTR_CENTER_FREQ1, chandef->center_freq1)) + goto nla_put_failure; + + if (nla_put_u32(msg, NL80211_ATTR_CHANNEL_WIDTH, chandef->width)) + goto nla_put_failure; + + if (chandef->width == NL80211_CHAN_WIDTH_40) { + enum nl80211_channel_type chan_type = NL80211_CHAN_HT40MINUS; + + if (chandef->center_freq1 > chandef->chan->center_freq) + chan_type = NL80211_CHAN_HT40PLUS; + + if (nla_put_u32(msg, NL80211_ATTR_WIPHY_CHANNEL_TYPE, + chan_type)) + goto nla_put_failure; + } + + if (nla_put_u32(msg, NL80211_ATTR_CH_SWITCH_COUNT, csa_count)) + goto nla_put_failure; + + genlmsg_end(msg, hdr); + + genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0, + NL80211_MCGRP_MLME, gfp); + return; + + nla_put_failure: + genlmsg_cancel(msg, hdr); + nlmsg_free(msg); +} + +bool cfg80211_ch_switch(struct net_device *dev, + struct cfg80211_chan_def *chandef, + u8 csa_count) +{ + struct wireless_dev *wdev = dev->ieee80211_ptr; + struct wiphy *wiphy = wdev->wiphy; + struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy); + + if (WARN_ON(wdev->iftype != NL80211_IFTYPE_AP && + wdev->iftype != NL80211_IFTYPE_P2P_GO && + wdev->iftype != NL80211_IFTYPE_ADHOC && + wdev->iftype != NL80211_IFTYPE_MESH_POINT)) + return false; + + if (WARN_ON(!chandef || !chandef->chan)) + return false; + + nl80211_ch_switch(rdev, dev, chandef, csa_count, GFP_ATOMIC); + + return true; +} +EXPORT_SYMBOL(cfg80211_ch_switch); + void nl80211_radar_notify(struct cfg80211_registered_device *rdev, const struct cfg80211_chan_def *chandef,