| Message ID | 1644914581-21682-1-git-send-email-quic_ramess@quicinc.com |
|---|---|
| Headers | show |
| Series | Add support to configure 6GHz non-ht duplicate transmission | expand |
Hi, Couple of notes below: > @@ -704,6 +704,7 @@ struct ieee80211_bss_conf { > struct cfg80211_he_bss_color he_bss_color; > struct ieee80211_fils_discovery fils_discovery; > u32 unsol_bcast_probe_resp_interval; > + bool he_6g_nonht_dup_beacon_set; This is missing documentation. > + cap = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, > + params->tail, params->tail_len); > + if (cap && cap->datalen >= sizeof(*he_oper) + 1) { > + he_oper = (void *)(cap->data + 1); > + he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper); > + if (he_6ghz_oper) { > + sdata->vif.bss_conf.he_6g_nonht_dup_beacon_set = false; > + if (u8_get_bits(he_6ghz_oper->control, > + IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON)) { > + sdata->vif.bss_conf.he_6g_nonht_dup_beacon_set = true; > + } no braces needed there, and no u8_get_bits() either, you can just & ? > + } > + } I am wondering though if this should even be detected from the HE operation element itself, rather than from the beacon rate settings that are separate in nl80211? johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Friday, March 11, 2022 4:33 PM > To: Rameshkumar Sundaram (QUIC) <quic_ramess@quicinc.com> > Cc: linux-wireless@vger.kernel.org; ath11k@lists.infradead.org > Subject: Re: [PATCH 1/2] mac80211: add support to configure 6GHz non-ht > duplicate transmission > > Hi, > > Couple of notes below: > > > @@ -704,6 +704,7 @@ struct ieee80211_bss_conf { > > struct cfg80211_he_bss_color he_bss_color; > > struct ieee80211_fils_discovery fils_discovery; > > u32 unsol_bcast_probe_resp_interval; > > + bool he_6g_nonht_dup_beacon_set; > > This is missing documentation. Thanks, will add it in next patch > > > + cap = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, > > + params->tail, params->tail_len); > > + if (cap && cap->datalen >= sizeof(*he_oper) + 1) { > > + he_oper = (void *)(cap->data + 1); > > + he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper); > > + if (he_6ghz_oper) { > > + sdata->vif.bss_conf.he_6g_nonht_dup_beacon_set = > false; > > + if (u8_get_bits(he_6ghz_oper->control, > > + > IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON)) { > > + sdata- > >vif.bss_conf.he_6g_nonht_dup_beacon_set = true; > > + } > > no braces needed there, and no u8_get_bits() either, you can just & ? Sure, I remember I got sparse warnings last time without this (also took reference from net/mac80211/util.c ieee80211_chandef_he_6ghz_oper()), will send v2 with 'just &' anyway. > > > + } > > + } > > I am wondering though if this should even be detected from the HE > operation element itself, rather than from the beacon rate settings that are > separate in nl80211? This is a BW dependent bit in HE Oper IE and user space can switch BW (CSA/chan switch scenarios). Which can call assign_beacon directly, Hence adding the logic here to cover all Beacon change cases. > > johannes
On Mon, 2022-03-21 at 05:12 +0000, Rameshkumar Sundaram (QUIC) wrote: > > > > > I am wondering though if this should even be detected from the HE > > operation element itself, rather than from the beacon rate settings > > that are > > separate in nl80211? > This is a BW dependent bit in HE Oper IE and user space can switch BW > (CSA/chan switch scenarios). > Which can call assign_beacon directly, Hence adding the logic here to > cover all Beacon change cases. > Yeah but still ... why do we take it from the content rather than adding a control for it? johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Monday, March 21, 2022 2:28 PM > To: Rameshkumar Sundaram (QUIC) <quic_ramess@quicinc.com> > Cc: linux-wireless@vger.kernel.org; ath11k@lists.infradead.org > Subject: Re: [PATCH 1/2] mac80211: add support to configure 6GHz non-ht > duplicate transmission > > On Mon, 2022-03-21 at 05:12 +0000, Rameshkumar Sundaram (QUIC) wrote: > > > > > > > > I am wondering though if this should even be detected from the HE > > > operation element itself, rather than from the beacon rate settings > > > that are separate in nl80211? > > This is a BW dependent bit in HE Oper IE and user space can switch BW > > (CSA/chan switch scenarios). > > Which can call assign_beacon directly, Hence adding the logic here to > > cover all Beacon change cases. > > > > Yeah but still ... why do we take it from the content rather than adding a > control for it? So, Suggestion here is to add a new NL attribute to carry the information from user space for START_AP and SET_BEACON NL commands and use it here, is that understanding correct ? > > johannes
On Tue, 2022-03-22 at 13:50 +0000, Rameshkumar Sundaram (QUIC) wrote: > > > So, Suggestion here is to add a new NL attribute to carry the > information from user space for > START_AP and SET_BEACON NL commands and use it here, is that > understanding correct ? > Well, I was thinking it would reasonably come with the beacon rate, i.e. as a new attribute like NL80211_TXRATE_DUP? johannes
> -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Tuesday, March 22, 2022 8:34 PM > To: Rameshkumar Sundaram (QUIC) <quic_ramess@quicinc.com> > Cc: linux-wireless@vger.kernel.org; ath11k@lists.infradead.org > Subject: Re: [PATCH 1/2] mac80211: add support to configure 6GHz non-ht > duplicate transmission > > WARNING: This email originated from outside of Qualcomm. Please be wary > of any links or attachments, and do not enable macros. > > On Tue, 2022-03-22 at 13:50 +0000, Rameshkumar Sundaram (QUIC) wrote: > > > > > So, Suggestion here is to add a new NL attribute to carry the > > information from user space for START_AP and SET_BEACON NL commands > > and use it here, is that understanding correct ? > > > > Well, I was thinking it would reasonably come with the beacon rate, i.e. > as a new attribute like NL80211_TXRATE_DUP? Sorry not getting this, you mean hostapd would need to send new set of rates for this configuration ? The expectation is that user-space will set this bit in HE oper IE only if legacy rates are present in beacon or basic rates. > > johannes
A 6GHz AP can decide to transmit Beacon, Broadcast probe response and FILS discovery frames in a non-HT duplicate PPDU when operating in non 20MHz Bandwidth to enhance its discoverability. (IEEE Std 802.11ax‐2021-26.17.2.2) Add changes to configure 6GHz non-HT duplicate beacon transmission based on Duplicate Beacon subfield of 6GHz Operation Information field of the HE Operation element in Beacon. Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com> Rameshkumar Sundaram (2): mac80211: add support to configure 6G non-ht duplicate transmission ath11k: add support to configure 6G non-ht duplicate transmission drivers/net/wireless/ath/ath11k/mac.c | 38 +++++++++++++++++++++++++++++++++++ drivers/net/wireless/ath/ath11k/wmi.h | 8 ++++++++ include/net/mac80211.h | 1 + net/mac80211/cfg.c | 17 ++++++++++++++++ 4 files changed, 64 insertions(+)