diff mbox series

wifi: mac80211: handle link ID during management Tx

Message ID 20240326045242.3190611-1-quic_adisi@quicinc.com
State Superseded
Headers show
Series wifi: mac80211: handle link ID during management Tx | expand

Commit Message

Aditya Kumar Singh March 26, 2024, 4:52 a.m. UTC 
From: Sriram R <quic_srirrama@quicinc.com>

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 <quic_srirrama@quicinc.com>
Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 net/mac80211/offchannel.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Aditya Kumar Singh April 8, 2024, 3:45 a.m. UTC | #1 
On 3/26/24 10:22, Aditya Kumar Singh wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> 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 <quic_srirrama@quicinc.com>
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>


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
Johannes Berg April 8, 2024, 6:31 p.m. UTC | #2 
On Tue, 2024-03-26 at 10:22 +0530, Aditya Kumar Singh wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> 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
Aditya Kumar Singh April 9, 2024, 6:02 a.m. UTC | #3 
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 <quic_srirrama@quicinc.com>
>>
>> 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.
diff mbox series

Patch

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;
 		}