diff mbox series

mac80211: Allow STA to connect in AX mode to MLD AP.

Message ID 20231003164326.857433-1-greearb@candelatech.com (mailing list archive)
State Rejected
Delegated to: Johannes Berg
Headers show
Series mac80211: Allow STA to connect in AX mode to MLD AP. | expand

Commit Message

Ben Greear Oct. 3, 2023, 4:43 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

Check if user has configured STA to be AX mode, and if so,
skip the check for MLD elements (as they would not be needed
in AX mode).

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 net/mac80211/mlme.c | 51 +++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

Comments

Johannes Berg Oct. 4, 2023, 7:38 p.m. UTC | #1 
On Tue, 2023-10-03 at 09:43 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> Check if user has configured STA to be AX mode, and if so,
> skip the check for MLD elements (as they would not be needed
> in AX mode).

Eh, no, I think this is wrong.

> 
>  		if (ieee80211_vif_is_mld(&sdata->vif)) {

If you get past this if, you've (locally) committed to doing EHT
already, not just HE (née 11ax), so should have an EHT connection?

Though the change is hard to read - but why are you telling the
supplicant to connect with MLO if you wanted to not use EHT?

johannes
Ben Greear Oct. 4, 2023, 8:36 p.m. UTC | #2 
On 10/4/23 12:38, Johannes Berg wrote:
> On Tue, 2023-10-03 at 09:43 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Check if user has configured STA to be AX mode, and if so,
>> skip the check for MLD elements (as they would not be needed
>> in AX mode).
> 
> Eh, no, I think this is wrong.
> 
>>
>>   		if (ieee80211_vif_is_mld(&sdata->vif)) {
> 
> If you get past this if, you've (locally) committed to doing EHT
> already, not just HE (née 11ax), so should have an EHT connection?
> 
> Though the change is hard to read - but why are you telling the
> supplicant to connect with MLO if you wanted to not use EHT?

It is mostly white-space changes, but the diff is ugly.

I caused the DISABLE_EHT flag to be set in mac80211, and that generally
seems to work in my kernel, but something is still trying to do MLO I guess.

I did not specifically tell supplicant to do MLO, and in fact I told
it to disable_be, but maybe that is not fully implemented.  My supplicant
is a few months old vs upstream at this point.

Thanks,
Ben
Ben Greear Oct. 5, 2023, 11:38 p.m. UTC | #3 
On 10/4/23 13:36, Ben Greear wrote:
> On 10/4/23 12:38, Johannes Berg wrote:
>> On Tue, 2023-10-03 at 09:43 -0700, greearb@candelatech.com wrote:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> Check if user has configured STA to be AX mode, and if so,
>>> skip the check for MLD elements (as they would not be needed
>>> in AX mode).
>>
>> Eh, no, I think this is wrong.
>>
>>>
>>>           if (ieee80211_vif_is_mld(&sdata->vif)) {
>>
>> If you get past this if, you've (locally) committed to doing EHT
>> already, not just HE (née 11ax), so should have an EHT connection?
>>
>> Though the change is hard to read - but why are you telling the
>> supplicant to connect with MLO if you wanted to not use EHT?
> 
> It is mostly white-space changes, but the diff is ugly.
> 
> I caused the DISABLE_EHT flag to be set in mac80211, and that generally
> seems to work in my kernel, but something is still trying to do MLO I guess.
> 
> I did not specifically tell supplicant to do MLO, and in fact I told
> it to disable_be, but maybe that is not fully implemented.  My supplicant
> is a few months old vs upstream at this point.

I created a patch to supplicant where it will disable MLO if disable_eht
is true, and that seems to have properly associated a non-MLO station
in AX mode.

So I agree that my patch to the kernel is not needed.

Thanks for the patch review and comments.

Thanks,
Ben
diff mbox series

Patch

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e7d42545851f..5fb0bdad88f4 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5550,34 +5550,39 @@  static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
 		}
 
 		if (ieee80211_vif_is_mld(&sdata->vif)) {
-			if (!elems->ml_basic) {
-				sdata_info(sdata,
-					   "MLO association with %pM but no multi-link element in response!\n",
-					   assoc_data->ap_addr);
-				goto abandon_assoc;
-			}
-
-			if (le16_get_bits(elems->ml_basic->control,
-					  IEEE80211_ML_CONTROL_TYPE) !=
-					IEEE80211_ML_CONTROL_TYPE_BASIC) {
-				sdata_info(sdata,
-					   "bad multi-link element (control=0x%x)\n",
-					   le16_to_cpu(elems->ml_basic->control));
-				goto abandon_assoc;
-			} else {
-				struct ieee80211_mle_basic_common_info *common;
-
-				common = (void *)elems->ml_basic->variable;
+			struct ieee80211_link_data *link;
 
-				if (memcmp(assoc_data->ap_addr,
-					   common->mld_mac_addr, ETH_ALEN)) {
+			link = sdata_dereference(sdata->link[0], sdata);
+			if (!(link && (link->u.mgd.conn_flags & IEEE80211_CONN_DISABLE_EHT))) {
+				if (!elems->ml_basic) {
 					sdata_info(sdata,
-						   "AP MLD MAC address mismatch: got %pM expected %pM\n",
-						   common->mld_mac_addr,
+						   "MLO association with %pM but no multi-link element in response!\n",
 						   assoc_data->ap_addr);
 					goto abandon_assoc;
 				}
-			}
+
+				if (le16_get_bits(elems->ml_basic->control,
+						  IEEE80211_ML_CONTROL_TYPE) !=
+				    IEEE80211_ML_CONTROL_TYPE_BASIC) {
+					sdata_info(sdata,
+						   "bad multi-link element (control=0x%x)\n",
+						   le16_to_cpu(elems->ml_basic->control));
+					goto abandon_assoc;
+				} else {
+					struct ieee80211_mle_basic_common_info *common;
+
+					common = (void *)elems->ml_basic->variable;
+
+					if (memcmp(assoc_data->ap_addr,
+						   common->mld_mac_addr, ETH_ALEN)) {
+						sdata_info(sdata,
+							   "AP MLD MAC address mismatch: got %pM expected %pM\n",
+							   common->mld_mac_addr,
+							   assoc_data->ap_addr);
+						goto abandon_assoc;
+					}
+				}
+			} /* if we are not configured to disable EHT */
 		}
 
 		sdata->vif.cfg.aid = aid;