Subject: [RFC 0/2] wifi: mac80211: extend rx API with link_id for MLO connection

In MLO, the frames can be received on any of the affiliated links.
When the address translation for rx frames are done in fw/hw, it
is very important to have an explicit link information reported
for every rx frame to mac80211. Per-link processing includes
stats update, GTK/IGTK/BIGTK retrieval and so on. This patch
set only tries to use the link at the top level APIs, deep
rx handlers are yet to be changed to use the respective
link accordingly.

This series is prepared on top of the latest mld branch.

Base mld commit

commit f69d4554386b4d2b56ca883fb97c92d64e188615
Author: Shaul Triebitz <[email protected]>
Date: Sun Jul 24 11:07:32 2022 +0300

wifi: mac80211: properly set old_links when removing a link

In ieee80211_sta_remove_link, valid_links is set to
the new_links before calling drv_change_sta_links, but
is used for the old_links.

Change-Id: I7f74b5d818c6154bc544a75a2933ab924b0c8937
Fixes: cb71f1d136a6 ("wifi: mac80211: add sta link addition/removal")
Signed-off-by: Shaul Triebitz <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>


Vasanthakumar Thiagarajan (2):
wifi: mac80211: add a new field in ieee80211_rx_status for link id
wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx
link

include/net/mac80211.h | 4 ++
net/mac80211/rx.c | 133 ++++++++++++++++++++++++++++++++++++++---
2 files changed, 128 insertions(+), 9 deletions(-)

--
2.17.1



Subject: [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link

Fill rx.link with respective data_link from the reported link_id
in rx_status. Any link_id > 15 is invalid. Non-MLO connections
can use either 0 or 15 as the link_id. Please note that link_id
0 is used with non-MLO connections to avoid changes in the
drivers not supporting MLO. For a 802.11 MLD address translated
frame, driver must report the right link_id for the
frame to get processed. When processing 802.3 frame format,
link_id is not that critical, used only with stats update.
In such case, all the stats will be updated for the deflink.

Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
net/mac80211/rx.c | 133 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 124 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 57df21e2170a..87aa81bc6595 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -4508,6 +4508,16 @@ void ieee80211_check_fast_rx_iface(struct ieee80211_sub_if_data *sdata)
mutex_unlock(&local->sta_mtx);
}

+static bool
+ieee80211_rx_is_valid_sta_link_id(struct ieee80211_sta *sta, u8 link_id)
+{
+ if (!sta->mlo && (link_id && link_id != IEEE80211_LINK_UNSPECIFIED))
+ return false;
+
+ return !(link_id != IEEE80211_LINK_UNSPECIFIED &&
+ (sta->mlo && !(sta->valid_links & BIT(link_id))));
+}
+
static void ieee80211_rx_8023(struct ieee80211_rx_data *rx,
struct ieee80211_fast_rx *fast_rx,
int orig_len)
@@ -4515,19 +4525,30 @@ static void ieee80211_rx_8023(struct ieee80211_rx_data *rx,
struct ieee80211_sta_rx_stats *stats;
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(rx->skb);
struct sta_info *sta = rx->sta;
+ struct link_sta_info *link_sta;
struct sk_buff *skb = rx->skb;
void *sa = skb->data + ETH_ALEN;
void *da = skb->data;

- stats = &sta->deflink.rx_stats;
+ if (rx->link_id >= 0) {
+ link_sta = rcu_dereference(sta->link[rx->link_id]);
+ if (WARN_ON_ONCE(!link_sta)) {
+ dev_kfree_skb(rx->skb);
+ return;
+ }
+ } else {
+ link_sta = &sta->deflink;
+ }
+
+ stats = &link_sta->rx_stats;
if (fast_rx->uses_rss)
- stats = this_cpu_ptr(sta->deflink.pcpu_rx_stats);
+ stats = this_cpu_ptr(link_sta->pcpu_rx_stats);

/* statistics part of ieee80211_rx_h_sta_process() */
if (!(status->flag & RX_FLAG_NO_SIGNAL_VAL)) {
stats->last_signal = status->signal;
if (!fast_rx->uses_rss)
- ewma_signal_add(&sta->deflink.rx_stats_avg.signal,
+ ewma_signal_add(&link_sta->rx_stats_avg.signal,
-status->signal);
}

@@ -4543,7 +4564,7 @@ static void ieee80211_rx_8023(struct ieee80211_rx_data *rx,

stats->chain_signal_last[i] = signal;
if (!fast_rx->uses_rss)
- ewma_signal_add(&sta->deflink.rx_stats_avg.chain_signal[i],
+ ewma_signal_add(&link_sta->rx_stats_avg.chain_signal[i],
-signal);
}
}
@@ -4619,7 +4640,8 @@ static bool ieee80211_invoke_fast_rx(struct ieee80211_rx_data *rx,
u8 da[ETH_ALEN];
u8 sa[ETH_ALEN];
} addrs __aligned(2);
- struct ieee80211_sta_rx_stats *stats = &sta->deflink.rx_stats;
+ struct link_sta_info *link_sta;
+ struct ieee80211_sta_rx_stats *stats;

/* for parallel-rx, we need to have DUP_VALIDATED, otherwise we write
* to a common data structure; drivers can implement that per queue
@@ -4720,8 +4742,19 @@ static bool ieee80211_invoke_fast_rx(struct ieee80211_rx_data *rx,
return true;
drop:
dev_kfree_skb(skb);
+
+ if (rx->link_id >= 0) {
+ link_sta = rcu_dereference(sta->link[rx->link_id]);
+ if (!link_sta)
+ return true;
+ } else {
+ link_sta = &sta->deflink;
+ }
+
if (fast_rx->uses_rss)
- stats = this_cpu_ptr(sta->deflink.pcpu_rx_stats);
+ stats = this_cpu_ptr(link_sta->pcpu_rx_stats);
+ else
+ stats = &link_sta->rx_stats;

stats->dropped++;
return true;
@@ -4763,8 +4796,7 @@ static bool ieee80211_prepare_and_rx_handle(struct ieee80211_rx_data *rx,
return false;

if (rx->link_id >= 0) {
- link = rcu_dereference(rx->sdata->link[rx->link_id]);
-
+ link = rcu_dereference(sdata->link[rx->link_id]);
/* we might race link removal */
if (!link)
return true;
@@ -4827,6 +4859,7 @@ static void __ieee80211_rx_handle_8023(struct ieee80211_hw *hw,
struct list_head *list)
{
struct ieee80211_local *local = hw_to_local(hw);
+ struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_fast_rx *fast_rx;
struct ieee80211_rx_data rx;

@@ -4847,7 +4880,30 @@ static void __ieee80211_rx_handle_8023(struct ieee80211_hw *hw,

rx.sta = container_of(pubsta, struct sta_info, sta);
rx.sdata = rx.sta->sdata;
- rx.link = &rx.sdata->deflink;
+
+ if (!ieee80211_rx_is_valid_sta_link_id(pubsta, status->link_id))
+ goto drop;
+
+ /*
+ * TODO: In MLO, should the frame be dropped if the right link_id is not
+ * available? Or may be it is fine in the current form to proceed with
+ * the frame processing because with frame being in 802.3 format,
+ * link_id is used only for stats purpose and updating the stats on
+ * the deflink is fine?
+ */
+ if (pubsta->mlo && status->link_id != IEEE80211_LINK_UNSPECIFIED)
+ rx.link_id = status->link_id;
+
+ if (rx.link_id >= 0) {
+ struct ieee80211_link_data *link;
+
+ link = rcu_dereference(rx.sdata->link[rx.link_id]);
+ if (!link)
+ goto drop;
+ rx.link = link;
+ } else {
+ rx.link = &rx.sdata->deflink;
+ }

fast_rx = rcu_dereference(rx.sta->fast_rx);
if (!fast_rx)
@@ -4877,7 +4933,18 @@ static bool ieee80211_rx_for_interface(struct ieee80211_rx_data *rx,
rx->sta = link_sta->sta;
rx->link_id = link_sta->link_id;
} else {
+ struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
+
rx->sta = sta_info_get_bss(rx->sdata, hdr->addr2);
+ if (rx->sta) {
+ if ((status->link_id == IEEE80211_LINK_UNSPECIFIED &&
+ rx->sta->sta.mlo) ||
+ !ieee80211_rx_is_valid_sta_link_id(&rx->sta->sta,
+ status->link_id))
+ return false;
+
+ rx->link_id = rx->sta->sta.mlo ? status->link_id : -1;
+ }
}

return ieee80211_prepare_and_rx_handle(rx, skb, consume);
@@ -4893,6 +4960,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
struct list_head *list)
{
struct ieee80211_local *local = hw_to_local(hw);
+ struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_sub_if_data *sdata;
struct ieee80211_hdr *hdr;
__le16 fc;
@@ -4941,6 +5009,36 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
if (pubsta) {
rx.sta = container_of(pubsta, struct sta_info, sta);
rx.sdata = rx.sta->sdata;
+
+ if (!ieee80211_rx_is_valid_sta_link_id(pubsta,
+ status->link_id))
+ goto out;
+
+ if (pubsta->mlo &&
+ status->link_id != IEEE80211_LINK_UNSPECIFIED)
+ rx.link_id = status->link_id;
+
+ /*
+ * In MLO connection, fetch the link_id using addr2
+ * when the driver passes unspecified link_id in status.
+ * When the address translation is already performed by
+ * driver/hw, the right link_id must be passed in
+ * status.
+ */
+
+ if (status->link_id == IEEE80211_LINK_UNSPECIFIED &&
+ pubsta->mlo) {
+ struct ieee80211_hdr *hdr = (void *)skb->data;
+ struct link_sta_info *link_sta;
+
+ link_sta = link_sta_info_get_bss(rx.sdata,
+ hdr->addr2);
+ if (!link_sta)
+ goto out;
+
+ rx.link_id = link_sta->link_id;
+ }
+
if (ieee80211_prepare_and_rx_handle(&rx, skb, true))
return;
goto out;
@@ -4954,6 +5052,13 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
continue;
}

+ if ((status->link_id == IEEE80211_LINK_UNSPECIFIED &&
+ sta->sta.mlo) ||
+ !ieee80211_rx_is_valid_sta_link_id(&sta->sta,
+ status->link_id))
+ continue;
+
+ rx.link_id = sta->sta.mlo ? status->link_id : -1;
rx.sta = prev_sta;
rx.sdata = prev_sta->sdata;
ieee80211_prepare_and_rx_handle(&rx, skb, false);
@@ -4962,6 +5067,13 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
}

if (prev_sta) {
+ if ((status->link_id == IEEE80211_LINK_UNSPECIFIED &&
+ prev_sta->sta.mlo) ||
+ !ieee80211_rx_is_valid_sta_link_id(&prev_sta->sta,
+ status->link_id))
+ goto out;
+
+ rx.link_id = sta->sta.mlo ? status->link_id : -1;
rx.sta = prev_sta;
rx.sdata = prev_sta->sdata;

@@ -5104,6 +5216,9 @@ void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
}
}

+ if (WARN_ON_ONCE(status->link_id > IEEE80211_LINK_UNSPECIFIED))
+ goto drop;
+
status->rx_flags = 0;

kcov_remote_start_common(skb_get_kcov_handle(skb));
--
2.17.1


Subject: [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id

In MLO, when the address translation from link to MLD is done
in fw/hw, it is necessary to be able to have some information
on the link on which the frame has been received. Extend the
rx API to include link_id in ieee80211_rx_status.

Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
include/net/mac80211.h | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f198af600b5e..23bc34657b16 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1480,6 +1480,9 @@ enum mac80211_rx_encoding {
* each A-MPDU but the same for each subframe within one A-MPDU
* @ampdu_delimiter_crc: A-MPDU delimiter CRC
* @zero_length_psdu_type: radiotap type of the 0-length PSDU
+ * @link_id: id of the link used to receive the packet. Applicable only with
+ * MLO connection, valid link ids are in 0-14, link_id 15 means either the
+ * link id is not known or it is a non-MLO connection.
*/
struct ieee80211_rx_status {
u64 mactime;
@@ -1504,6 +1507,7 @@ struct ieee80211_rx_status {
s8 chain_signal[IEEE80211_MAX_CHAINS];
u8 ampdu_delimiter_crc;
u8 zero_length_psdu_type;
+ u8 link_id;
};

static inline u32
--
2.17.1


2022-08-02 15:36:21

by Jeff Johnson

[permalink] [raw]
Subject: Re: [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id

On 8/1/2022 11:50 PM, Vasanthakumar Thiagarajan wrote:
> In MLO, when the address translation from link to MLD is done
> in fw/hw, it is necessary to be able to have some information
> on the link on which the frame has been received. Extend the
> rx API to include link_id in ieee80211_rx_status.
>
> Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
> ---
> include/net/mac80211.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index f198af600b5e..23bc34657b16 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1480,6 +1480,9 @@ enum mac80211_rx_encoding {
> * each A-MPDU but the same for each subframe within one A-MPDU
> * @ampdu_delimiter_crc: A-MPDU delimiter CRC
> * @zero_length_psdu_type: radiotap type of the 0-length PSDU
> + * @link_id: id of the link used to receive the packet. Applicable only with
> + * MLO connection, valid link ids are in 0-14, link_id 15 means either the
> + * link id is not known or it is a non-MLO connection.
> */
> struct ieee80211_rx_status {
> u64 mactime;
> @@ -1504,6 +1507,7 @@ struct ieee80211_rx_status {
> s8 chain_signal[IEEE80211_MAX_CHAINS];
> u8 ampdu_delimiter_crc;
> u8 zero_length_psdu_type;
> + u8 link_id;
> };
>
> static inline u32

in other parts of the MLO code the link_id is defined as int and a value
of -1 is used for a non-MLO link. but I don't know if that is currently
universally true.

if that is curently universally true, do we want to now have divergent
definitions of a link_id?


Subject: Re: [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id

On 8/2/2022 8:58 PM, Jeff Johnson wrote:
> On 8/1/2022 11:50 PM, Vasanthakumar Thiagarajan wrote:
>> In MLO, when the address translation from link to MLD is done
>> in fw/hw, it is necessary to be able to have some information
>> on the link on which the frame has been received. Extend the
>> rx API to include link_id in ieee80211_rx_status.
>>
>> Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
>> ---
>>   include/net/mac80211.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>> index f198af600b5e..23bc34657b16 100644
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -1480,6 +1480,9 @@ enum mac80211_rx_encoding {
>>    *    each A-MPDU but the same for each subframe within one A-MPDU
>>    * @ampdu_delimiter_crc: A-MPDU delimiter CRC
>>    * @zero_length_psdu_type: radiotap type of the 0-length PSDU
>> + * @link_id: id of the link used to receive the packet. Applicable
>> only with
>> + *    MLO connection, valid link ids are in 0-14, link_id 15 means
>> either the
>> + *    link id is not known or it is a non-MLO connection.
>>    */
>>   struct ieee80211_rx_status {
>>       u64 mactime;
>> @@ -1504,6 +1507,7 @@ struct ieee80211_rx_status {
>>       s8 chain_signal[IEEE80211_MAX_CHAINS];
>>       u8 ampdu_delimiter_crc;
>>       u8 zero_length_psdu_type;
>> +    u8 link_id;
>>   };
>>   static inline u32
>
> in other parts of the MLO code the link_id is defined as int and a value
> of -1 is used for a non-MLO link. but I don't know if that is currently
> universally true.
>
> if that is curently universally true, do we want to now have divergent
> definitions of a link_id?

Good point, i see link_id is used both as unsigned and int based on
their usage. The reason I preferred unsigned is that we can make use of
the remaining 4-bits for some other purpose in future,
ieee80211_rx_status has size limitation.


Vasanth