| Message ID | 1587768108-25248-3-git-send-email-rmanohar@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | mac80211: add 6 GHz IEs support | expand |
On Fri, 2020-04-24 at 15:41 -0700, Rajkumar Manoharan wrote: > > +static inline bool > +cfg80211_chandef_is_6ghz(const struct cfg80211_chan_def *chandef) > +{ > + return (chandef->center_freq1 > 5940 && chandef->center_freq1 < 7105); > +} Seems like this > +++ b/net/wireless/chan.c > @@ -19,6 +19,29 @@ static bool cfg80211_valid_60g_freq(u32 freq) > return freq >= 58320 && freq <= 70200; > } > > +static bool cfg80211_is_6ghz_freq(u32 freq) > +{ > + return (freq > 5940 && freq < 7105); > +} should use this, by also exposing it, or something. > +static enum nl80211_chan_width cfg80211_chan_to_bw_6ghz(u8 idx) > +{ > + /* channels: 1, 5, 9, 13... */ > + if ((idx & 0x3) == 0x1) > + return NL80211_CHAN_WIDTH_20; > + /* channels 3, 11, 19... */ > + if ((idx & 0x7) == 0x3) > + return NL80211_CHAN_WIDTH_40; > + /* channels 7, 23, 39.. */ > + if ((idx & 0xf) == 0x7) > + return NL80211_CHAN_WIDTH_80; > + /* channels 15, 47, 79...*/ > + if ((idx & 0x1f) == 0xf) > + return NL80211_CHAN_WIDTH_160; > + > + return NL80211_CHAN_WIDTH_20; > +} We haven't really done that for anything else - is that really necessary? > +static bool cfg80211_6ghz_chandef_valid(const struct cfg80211_chan_def *chandef) > +{ > + enum nl80211_chan_width bw; > + int chan_idx; > + > + if (!cfg80211_is_6ghz_freq(chandef->center_freq1)) > + return false; this is kinda pointless, > @@ -213,6 +255,10 @@ bool cfg80211_chandef_valid(const struct cfg80211_chan_def *chandef) > !cfg80211_edmg_chandef_valid(chandef)) > return false; > > + if (cfg80211_chandef_is_6ghz(chandef) && > + !cfg80211_6ghz_chandef_valid(chandef)) > + return false; You only get there if it was in range ... Not sure about this whole patch, it seems a bit pointless? johannes
On 2020-04-29 07:26, Johannes Berg wrote: > On Fri, 2020-04-24 at 15:41 -0700, Rajkumar Manoharan wrote: >> >> +static inline bool >> +cfg80211_chandef_is_6ghz(const struct cfg80211_chan_def *chandef) >> +{ >> + return (chandef->center_freq1 > 5940 && chandef->center_freq1 < >> 7105); >> +} > > Seems like this > >> +++ b/net/wireless/chan.c >> @@ -19,6 +19,29 @@ static bool cfg80211_valid_60g_freq(u32 freq) >> return freq >= 58320 && freq <= 70200; >> } >> >> +static bool cfg80211_is_6ghz_freq(u32 freq) >> +{ >> + return (freq > 5940 && freq < 7105); >> +} > > should use this, by also exposing it, or something. > Sure. Export this and remove the above one. >> +static enum nl80211_chan_width cfg80211_chan_to_bw_6ghz(u8 idx) >> +{ >> + /* channels: 1, 5, 9, 13... */ >> + if ((idx & 0x3) == 0x1) >> + return NL80211_CHAN_WIDTH_20; >> + /* channels 3, 11, 19... */ >> + if ((idx & 0x7) == 0x3) >> + return NL80211_CHAN_WIDTH_40; >> + /* channels 7, 23, 39.. */ >> + if ((idx & 0xf) == 0x7) >> + return NL80211_CHAN_WIDTH_80; >> + /* channels 15, 47, 79...*/ >> + if ((idx & 0x1f) == 0xf) >> + return NL80211_CHAN_WIDTH_160; >> + >> + return NL80211_CHAN_WIDTH_20; >> +} > > We haven't really done that for anything else - is that really > necessary? > Hmm.. to check whether give center_freq1 chan_idx is allowed to operate in given bandwidth. Similar to center_idx_to_bw_6ghz of hostapd, this API is used to chandef bw. [...] >> @@ -213,6 +255,10 @@ bool cfg80211_chandef_valid(const struct >> cfg80211_chan_def *chandef) >> !cfg80211_edmg_chandef_valid(chandef)) >> return false; >> >> + if (cfg80211_chandef_is_6ghz(chandef) && >> + !cfg80211_6ghz_chandef_valid(chandef)) >> + return false; > > You only get there if it was in range ... > > Not sure about this whole patch, it seems a bit pointless? > Don't we have to check chandef bw? If not, I will drop the change. -Rajkumar
> > > +static enum nl80211_chan_width cfg80211_chan_to_bw_6ghz(u8 idx) > > > +{ > > > + /* channels: 1, 5, 9, 13... */ > > > + if ((idx & 0x3) == 0x1) > > > + return NL80211_CHAN_WIDTH_20; > > > + /* channels 3, 11, 19... */ > > > + if ((idx & 0x7) == 0x3) > > > + return NL80211_CHAN_WIDTH_40; > > > + /* channels 7, 23, 39.. */ > > > + if ((idx & 0xf) == 0x7) > > > + return NL80211_CHAN_WIDTH_80; > > > + /* channels 15, 47, 79...*/ > > > + if ((idx & 0x1f) == 0xf) > > > + return NL80211_CHAN_WIDTH_160; > > > + > > > + return NL80211_CHAN_WIDTH_20; > > > +} > > > > We haven't really done that for anything else - is that really > > necessary? > > > Hmm.. to check whether give center_freq1 chan_idx is allowed to operate > in given bandwidth. > Similar to center_idx_to_bw_6ghz of hostapd, this API is used to chandef > bw. Yeah, but good enough if hostapd does that check? I don't really see the kernel caring too much? > Don't we have to check chandef bw? If not, I will drop the change. I'm not really sure why we should, tbh. johannes
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 70e48f66dac8..13d3d8f92c99 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -730,6 +730,20 @@ cfg80211_chandef_is_edmg(const struct cfg80211_chan_def *chandef) } /** + * cfg80211_chandef_is_6ghz - check if chandef represents an 6 GHz channel + * + * @chandef: the channel definition + * + * Return: %true if frequency is in 6 GHz range, %false otherwise. + */ +static inline bool +cfg80211_chandef_is_6ghz(const struct cfg80211_chan_def *chandef) +{ + return (chandef->center_freq1 > 5940 && chandef->center_freq1 < 7105); +} + + +/** * cfg80211_chandef_compatible - check if two channel definitions are compatible * @chandef1: first channel definition * @chandef2: second channel definition diff --git a/net/wireless/chan.c b/net/wireless/chan.c index fcac5c6366e1..42d27cada237 100644 --- a/net/wireless/chan.c +++ b/net/wireless/chan.c @@ -19,6 +19,29 @@ static bool cfg80211_valid_60g_freq(u32 freq) return freq >= 58320 && freq <= 70200; } +static bool cfg80211_is_6ghz_freq(u32 freq) +{ + return (freq > 5940 && freq < 7105); +} + +static enum nl80211_chan_width cfg80211_chan_to_bw_6ghz(u8 idx) +{ + /* channels: 1, 5, 9, 13... */ + if ((idx & 0x3) == 0x1) + return NL80211_CHAN_WIDTH_20; + /* channels 3, 11, 19... */ + if ((idx & 0x7) == 0x3) + return NL80211_CHAN_WIDTH_40; + /* channels 7, 23, 39.. */ + if ((idx & 0xf) == 0x7) + return NL80211_CHAN_WIDTH_80; + /* channels 15, 47, 79...*/ + if ((idx & 0x1f) == 0xf) + return NL80211_CHAN_WIDTH_160; + + return NL80211_CHAN_WIDTH_20; +} + void cfg80211_chandef_create(struct cfg80211_chan_def *chandef, struct ieee80211_channel *chan, enum nl80211_channel_type chan_type) @@ -139,6 +162,25 @@ static bool cfg80211_edmg_chandef_valid(const struct cfg80211_chan_def *chandef) return true; } +static bool cfg80211_6ghz_chandef_valid(const struct cfg80211_chan_def *chandef) +{ + enum nl80211_chan_width bw; + int chan_idx; + + if (!cfg80211_is_6ghz_freq(chandef->center_freq1)) + return false; + + chan_idx = ieee80211_frequency_to_channel(chandef->center_freq1); + if (chan_idx <= 0) + return false; + + bw = cfg80211_chan_to_bw_6ghz(chan_idx); + if (bw != chandef->width) + return false; + + return true; +} + bool cfg80211_chandef_valid(const struct cfg80211_chan_def *chandef) { u32 control_freq; @@ -213,6 +255,10 @@ bool cfg80211_chandef_valid(const struct cfg80211_chan_def *chandef) !cfg80211_edmg_chandef_valid(chandef)) return false; + if (cfg80211_chandef_is_6ghz(chandef) && + !cfg80211_6ghz_chandef_valid(chandef)) + return false; + return true; } EXPORT_SYMBOL(cfg80211_chandef_valid);
Validate the params of set_channel against 6 GHz frequency range and bandwidth allowed. Signed-off-by: Rajkumar Manoharan <rmanohar@codeaurora.org> --- include/net/cfg80211.h | 14 ++++++++++++++ net/wireless/chan.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+)