2024-03-26 04:53:10

by Aditya Kumar Singh

[permalink] [raw]
Subject: [PATCH] wifi: mac80211: handle link ID during management Tx

From: Sriram R <[email protected]>

During management Tx, when source address is same as the link conf's
address and even when userspace requested Tx on a specific link, link ID
is not set which leads to dropping of the frame later.

Add changes to use the same link id and set it if the link bss is matching
the requested channel.

Signed-off-by: Sriram R <[email protected]>
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
net/mac80211/offchannel.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 221695d841fd..65e1e9e971fd 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -897,8 +897,18 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
break;
}

- if (ether_addr_equal(conf->addr, mgmt->sa))
+ if (ether_addr_equal(conf->addr, mgmt->sa)) {
+ /* If userspace requested Tx on a specific link
+ * use the same link id if the link bss is matching
+ * the requested chan.
+ */
+ if (sdata->vif.valid_links &&
+ params->link_id >= 0 && params->link_id == i &&
+ params->chan == chanctx_conf->def.chan)
+ link_id = i;
+
break;
+ }

chanctx_conf = NULL;
}
--
2.25.1



2024-04-08 03:45:38

by Aditya Kumar Singh

[permalink] [raw]
Subject: Re: [PATCH] wifi: mac80211: handle link ID during management Tx

On 3/26/24 10:22, Aditya Kumar Singh wrote:
> From: Sriram R <[email protected]>
>
> During management Tx, when source address is same as the link conf's
> address and even when userspace requested Tx on a specific link, link ID
> is not set which leads to dropping of the frame later.
>
> Add changes to use the same link id and set it if the link bss is matching
> the requested channel.
>
> Signed-off-by: Sriram R <[email protected]>
> Signed-off-by: Aditya Kumar Singh <[email protected]>


Hi Johannes,

Any further inputs?

This is a dependency for the hostapd series change -

[PATCH 00/22] hostapd: add cohosted MLO support
(https://patchwork.ozlabs.org/project/hostap/list/?series=400904)

The newly added test cases needs this kernel change.

- Aditya


2024-04-08 18:32:07

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wifi: mac80211: handle link ID during management Tx

On Tue, 2024-03-26 at 10:22 +0530, Aditya Kumar Singh wrote:
> From: Sriram R <[email protected]>
>
> During management Tx, when source address is same as the link conf's
> address and even when userspace requested Tx on a specific link, link ID
> is not set which leads to dropping of the frame later.

Sorry, don't understand. Please be more precise about the scenario and
why it's dropped.

Also, client mode, so wpa_s is actually sending a link-specific action
frame?

johannes


2024-04-09 06:02:28

by Aditya Kumar Singh

[permalink] [raw]
Subject: Re: [PATCH] wifi: mac80211: handle link ID during management Tx

On 4/9/24 00:01, Johannes Berg wrote:
> On Tue, 2024-03-26 at 10:22 +0530, Aditya Kumar Singh wrote:
>> From: Sriram R <[email protected]>
>>
>> During management Tx, when source address is same as the link conf's
>> address and even when userspace requested Tx on a specific link, link ID
>> is not set which leads to dropping of the frame later.
>
> Sorry, don't understand. Please be more precise about the scenario and
> why it's dropped.

So here, as per existing code, link_id would be -1 still even when user
space requested it and there is a channel to Tx on.

This -1 would be sent forward like this -

* ieee80211_tx_skb_tid()
* __ieee80211_tx_skb_tid_band() // band = 0
Now, if your link address happen to be same as your MLD
address, then mac80211 would put IEEE80211_LINK_UNSPECIFIED.
Looks like this results in mac80211 unable to set certain
sta related fields in tx_control.

As a result when the packet arrives in driver, with various condition
checks it fails and goes to either wrong path or dropped completely.

For example in mac80211_hw_sim driver,

Since link is unspecified, it goes in mac80211_hwsim_select_tx_link()
but there this condition is hit -

if (WARN_ON_ONCE(!sta || !sta->valid_links))
return &vif->bss_conf;

Following is the trace -
------------[ cut here ]------------
WARNING: CPU: 0 PID: 518 at
drivers/net/wireless/virtual/mac80211_hwsim.c:1889
mac80211_hwsim_tx+0x23e/0x7df
CPU: 0 PID: 518 Comm: hostapd Tainted: G W
6.8.0-rc7-g43e63eea1b51-dirty #140
Stack:
6060f8e1 604af0ce 6003ba75 00000001
6060f8e1 604c74ef 00000000 604cf3b0
00000000 00000000 00000009 60044d6a
Call Trace:
[<6003bcc9>] ? os_is_signal_stack+0x1b/0x27
[<604c74ef>] ? _printk+0x0/0x9f
[<60025b3b>] ? show_stack+0x13a/0x147
[<604af0ce>] ? dump_stack_print_info+0xe4/0xf0
[<6003ba75>] ? um_set_signals+0x0/0x3c
[<604c74ef>] ? _printk+0x0/0x9f
[<604cf3b0>] ? dump_stack_lvl+0x47/0x52
[<60044d6a>] ? __warn+0xd5/0x107
[<604c64ac>] ? warn_slowpath_fmt+0xd6/0xe2
[<604c7586>] ? _printk+0x97/0x9f
[<604c63d6>] ? warn_slowpath_fmt+0x0/0xe2
[<60078561>] ? find_held_lock+0x3b/0x82
[<6046b953>] ? ieee80211_tx_frags+0x1d8/0x27f
[<6007aa1d>] ? lock_release+0x239/0x248
[<60241801>] ? mac80211_hwsim_tx+0x23e/0x7df
[<6046b9a0>] ? ieee80211_tx_frags+0x225/0x27f
[<6046cd82>] ? invoke_tx_handlers_late+0x2da/0x7a3
[<604c74ef>] ? _printk+0x0/0x9f
[<604bf08b>] ? memcmp+0x0/0x21
[<6046baf9>] ? __ieee80211_tx+0xff/0x147
[<6046f658>] ? ieee80211_tx+0x11a/0x128
[<60471742>] ? __ieee80211_tx_skb_tid_band+0x20d/0x227
[<60471888>] ? ieee80211_tx_skb_tid+0x12c/0x148
[<6044439c>] ? ieee80211_mgmt_tx+0x528/0x5c7


Later in mac80211_hwsim_tx(), channel is selected as NULL ultimately due
to this and hence this condition is hit -

if (WARN(!channel, "TX w/o channel - queue = %d\n", txi->hw_queue)) {
ieee80211_free_txskb(hw, skb);
return;
}

Trace-
------------[ cut here ]------------
WARNING: CPU: 0 PID: 518 at
drivers/net/wireless/virtual/mac80211_hwsim.c:2005
mac80211_hwsim_tx+0x54e/0x7df
TX w/o channel - queue = 0
CPU: 0 PID: 518 Comm: hostapd Tainted: G W
6.8.0-rc7-g43e63eea1b51-dirty #140
Stack:
6060f8e1 604af0ce 6003ba75 00000001
6060f8e1 604c74ef 00000000 604cf3b0
00000000 826d7560 00000009 60044d6a
Call Trace:
[<6003bcc9>] ? os_is_signal_stack+0x1b/0x27
[<604c74ef>] ? _printk+0x0/0x9f
[<60025b3b>] ? show_stack+0x13a/0x147
[<604af0ce>] ? dump_stack_print_info+0xe4/0xf0
[<6003ba75>] ? um_set_signals+0x0/0x3c
[<604c74ef>] ? _printk+0x0/0x9f
[<604cf3b0>] ? dump_stack_lvl+0x47/0x52
[<60044d6a>] ? __warn+0xd5/0x107
[<604c64ac>] ? warn_slowpath_fmt+0xd6/0xe2
[<604c63d6>] ? warn_slowpath_fmt+0x0/0xe2
[<60078561>] ? find_held_lock+0x3b/0x82
[<6046b953>] ? ieee80211_tx_frags+0x1d8/0x27f
[<6007aa1d>] ? lock_release+0x239/0x248
[<60241b11>] ? mac80211_hwsim_tx+0x54e/0x7df
[<6046b9a0>] ? ieee80211_tx_frags+0x225/0x27f
[<6046cd82>] ? invoke_tx_handlers_late+0x2da/0x7a3
[<604c74ef>] ? _printk+0x0/0x9f
[<604bf08b>] ? memcmp+0x0/0x21
[<6046baf9>] ? __ieee80211_tx+0xff/0x147
[<6046f658>] ? ieee80211_tx+0x11a/0x128
[<60471742>] ? __ieee80211_tx_skb_tid_band+0x20d/0x227
[<60471888>] ? ieee80211_tx_skb_tid+0x12c/0x148
[<6044439c>] ? ieee80211_mgmt_tx+0x528/0x5c7
[<6041bf86>] ? nl80211_tx_mgmt+0x298/0x319



>
> Also, client mode, so wpa_s is actually sending a link-specific action
> frame?

As per current supplicant code I don't see it sending. This change is
agnostic to supplicant or hostapd sending link id or not in action frame.

If user space has requested in a particular link and if channel is set
in that link then use that link. This is what this change is trying to
do. And all this for management frames only.