Search Linux Wireless

Re: [PATCH] wireless: Allow disabling TWT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/8/21 6:25 AM, Johannes Berg wrote:
On Sat, 2021-03-06 at 08:20 -0800, greearb@xxxxxxxxxxxxxxx wrote:

+	/* Apply overrides as needed. */
+	if (ifmgd->flags & IEEE80211_STA_DISABLE_TWT) {
+		if (ext_capa) {
+			if (ext_capa && ext_capa->datalen > 10) {
+				ext_capa->data[9] &= ~(WLAN_EXT_CAPA10_TWT_RESPONDER_SUPPORT);

So apart from the useless code (checking "ext_capa" twice, unnecessary
nested ifs, unnecessary parentheses), this can already be done entirely
by userspace, since it controls the ext capa we send.

So we don't even need HE overrides in this case?


-static bool ieee80211_twt_req_supported(const struct sta_info *sta,
+static bool ieee80211_twt_req_supported(struct ieee80211_sub_if_data *sdata,
+					const struct sta_info *sta,
  					const struct ieee802_11_elems *elems)


This code here seems possibly wrong anyway since it doesn't take local
capabilities into account, maybe it should, and then these changes
wouldn't be necessary? >
+	/* Apply overrides as needed. */
+	if (ifmgd->flags & IEEE80211_STA_DISABLE_TWT) {
+		struct ieee80211_he_cap_elem *hec;
+		hec = (struct ieee80211_he_cap_elem *)(pos);
+		hec->mac_cap_info[0] &= ~(IEEE80211_HE_MAC_CAP0_TWT_REQ);
+		hec->mac_cap_info[0] &= ~(IEEE80211_HE_MAC_CAP0_TWT_RES);
+	}

Wait, we actually have TWT capability *twice*, once in HE and once in
extended capabilities?! Fun.

Evidently, like maybe the spec was planning to add TWT to VHT or
something like that?


But for this shouldn't we have the more general "HE capability override"
stuff that we have also for HT and VHT?

Maybe this can be done, but since TWT is in several different places, it
is going to be a pain to code up that way.

The way I see it, if we do go with overrides, the API is going to be pretty
tricky, basically passing a bunch of u8 around to represent different sets of IE
contents and their mask.  So I think it is cleaner to pass in specific flags such as 'disable-twt'.
Now, maybe those types of flags can be passed in as part of a nested netlink attribute
or a 'override-flags-1' u32 so that each time we add a new one it doesn't need to grow
the main netlink enum...

And, I'd still like to see a way to set station config
while it is still admin down, so that we have all of the desired local config overrides set
before we ever even start scanning...  That would be a better way to do overrides in
general in my opinion.

Thanks,
Ben

--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc  http://www.candelatech.com



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux