* [RFC 0/2] wifi: mac80211: extend rx API with link_id for MLO connection
@ 2022-08-02 6:50 Vasanthakumar Thiagarajan
2022-08-02 6:50 ` [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id Vasanthakumar Thiagarajan
2022-08-02 6:50 ` [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link Vasanthakumar Thiagarajan
0 siblings, 2 replies; 10+ messages in thread
From: Vasanthakumar Thiagarajan @ 2022-08-02 6:50 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
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 <shaul.triebitz@intel.com>
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 <shaul.triebitz@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id
2022-08-02 6:50 [RFC 0/2] wifi: mac80211: extend rx API with link_id for MLO connection Vasanthakumar Thiagarajan
@ 2022-08-02 6:50 ` Vasanthakumar Thiagarajan
2022-08-02 15:28 ` Jeff Johnson
2022-08-02 6:50 ` [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link Vasanthakumar Thiagarajan
1 sibling, 1 reply; 10+ messages in thread
From: Vasanthakumar Thiagarajan @ 2022-08-02 6:50 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
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 <quic_vthiagar@quicinc.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link
2022-08-02 6:50 [RFC 0/2] wifi: mac80211: extend rx API with link_id for MLO connection Vasanthakumar Thiagarajan
2022-08-02 6:50 ` [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id Vasanthakumar Thiagarajan
@ 2022-08-02 6:50 ` Vasanthakumar Thiagarajan
2022-08-09 18:15 ` Johannes Berg
1 sibling, 1 reply; 10+ messages in thread
From: Vasanthakumar Thiagarajan @ 2022-08-02 6:50 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
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 <quic_vthiagar@quicinc.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id
2022-08-02 6:50 ` [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id Vasanthakumar Thiagarajan
@ 2022-08-02 15:28 ` Jeff Johnson
2022-08-03 16:46 ` Vasanthakumar Thiagarajan
0 siblings, 1 reply; 10+ messages in thread
From: Jeff Johnson @ 2022-08-02 15:28 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan, johannes; +Cc: linux-wireless
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 <quic_vthiagar@quicinc.com>
> ---
> 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?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id
2022-08-02 15:28 ` Jeff Johnson
@ 2022-08-03 16:46 ` Vasanthakumar Thiagarajan
2022-08-09 18:12 ` Johannes Berg
0 siblings, 1 reply; 10+ messages in thread
From: Vasanthakumar Thiagarajan @ 2022-08-03 16:46 UTC (permalink / raw)
To: Jeff Johnson, johannes; +Cc: linux-wireless
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 <quic_vthiagar@quicinc.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id
2022-08-03 16:46 ` Vasanthakumar Thiagarajan
@ 2022-08-09 18:12 ` Johannes Berg
2022-08-11 9:31 ` Vasanthakumar Thiagarajan
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2022-08-09 18:12 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan, Jeff Johnson; +Cc: linux-wireless
On Wed, 2022-08-03 at 22:16 +0530, Vasanthakumar Thiagarajan wrote:
>
> > > + 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.
>
It's a bit tricky though - do we want to have 0 for all the drivers that
don't support MLO? Might not really be an issue, but OTOH not
initializing it should probably not result in a valid value otherwise
you might test something, think it's fine, and it really isn't.
I think we should spend a bit to have a validity indication in this
case. Even -1 wouldn't address it since you'd have to initialize to
that.
It works better the other way around in APIs etc. where we have a few
producers and many consumers (cfg80211 down to drivers), but less well
this way where we'd have to change drivers...
johannes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link
2022-08-02 6:50 ` [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link Vasanthakumar Thiagarajan
@ 2022-08-09 18:15 ` Johannes Berg
2022-08-11 10:28 ` Vasanthakumar Thiagarajan
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2022-08-09 18:15 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan; +Cc: linux-wireless
On Tue, 2022-08-02 at 12:20 +0530, Vasanthakumar Thiagarajan wrote:
> 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.
I think it might be worth splitting this patch a bit different - putting
some parts into the first patch already (that fill rx.link_id), and
keeping some others (statistics etc.) in this patch, which is then more
related to statistics?
>
> if (rx->link_id >= 0) {
> - link = rcu_dereference(rx->sdata->link[rx->link_id]);
> -
> + link = rcu_dereference(sdata->link[rx->link_id]);
that has some spurious whitespace changes
> + /*
> + * 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 the driver *does* give a link ID, it better be valid?
OTOH, could there be races, e.g. while disabling a link?
johannes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id
2022-08-09 18:12 ` Johannes Berg
@ 2022-08-11 9:31 ` Vasanthakumar Thiagarajan
0 siblings, 0 replies; 10+ messages in thread
From: Vasanthakumar Thiagarajan @ 2022-08-11 9:31 UTC (permalink / raw)
To: Johannes Berg, Jeff Johnson; +Cc: linux-wireless
On 8/9/2022 11:42 PM, Johannes Berg wrote:
> On Wed, 2022-08-03 at 22:16 +0530, Vasanthakumar Thiagarajan wrote:
>>
>>>> + 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.
>>
>
> It's a bit tricky though - do we want to have 0 for all the drivers that
> don't support MLO? Might not really be an issue, but OTOH not
> initializing it should probably not result in a valid value otherwise
> you might test something, think it's fine, and it really isn't. >
> I think we should spend a bit to have a validity indication in this
> case. Even -1 wouldn't address it since you'd have to initialize to
> that.
>
Sure, a new valid bit looks cleaner. Ill make the changes.
> It works better the other way around in APIs etc. where we have a few
> producers and many consumers (cfg80211 down to drivers), but less well
> this way where we'd have to change drivers...
I agree.
Thanks!
Vasanth
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link
2022-08-09 18:15 ` Johannes Berg
@ 2022-08-11 10:28 ` Vasanthakumar Thiagarajan
2023-01-19 20:57 ` Johannes Berg
0 siblings, 1 reply; 10+ messages in thread
From: Vasanthakumar Thiagarajan @ 2022-08-11 10:28 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On 8/9/2022 11:45 PM, Johannes Berg wrote:
> On Tue, 2022-08-02 at 12:20 +0530, Vasanthakumar Thiagarajan wrote:
>> 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.
>
> I think it might be worth splitting this patch a bit different - putting
> some parts into the first patch already (that fill rx.link_id), and
> keeping some others (statistics etc.) in this patch, which is then more
> related to statistics?
Sure.
>
>>
>> if (rx->link_id >= 0) {
>> - link = rcu_dereference(rx->sdata->link[rx->link_id]);
>> -
>> + link = rcu_dereference(sdata->link[rx->link_id]);
>
> that has some spurious whitespace changes
>
>> + /*
>> + * 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 the driver *does* give a link ID, it better be valid?
Sure, we can remove IEEE80211_LINK_UNSPECIFIED. If now valid
link_id is set for an MLO link, we still try to retrieve it from
the addr2 in case the MLD address translation is not done in the
lower layers but when this happens with already translated packets
then we may need to drop the packet?
>
> OTOH, could there be races, e.g. while disabling a link?
I assume this is for the packets where the received on a
link which is disabled? May be driver sets that link but
that link will not be valid in mac80211 while processing
this packet and the packet will be dropped? Or may be
a different race you are mentioning?
Vasanth
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link
2022-08-11 10:28 ` Vasanthakumar Thiagarajan
@ 2023-01-19 20:57 ` Johannes Berg
0 siblings, 0 replies; 10+ messages in thread
From: Johannes Berg @ 2023-01-19 20:57 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan; +Cc: linux-wireless
So I'm digging through old email before I declare myself bankrupt on
that ...
> >
> > OTOH, could there be races, e.g. while disabling a link?
>
> I assume this is for the packets where the received on a
> link which is disabled? May be driver sets that link but
> that link will not be valid in mac80211 while processing
> this packet and the packet will be dropped? Or may be
> a different race you are mentioning?
>
Yes that's what I was thinking of.
johannes
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-01-19 21:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 6:50 [RFC 0/2] wifi: mac80211: extend rx API with link_id for MLO connection Vasanthakumar Thiagarajan
2022-08-02 6:50 ` [RFC 1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id Vasanthakumar Thiagarajan
2022-08-02 15:28 ` Jeff Johnson
2022-08-03 16:46 ` Vasanthakumar Thiagarajan
2022-08-09 18:12 ` Johannes Berg
2022-08-11 9:31 ` Vasanthakumar Thiagarajan
2022-08-02 6:50 ` [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status to retrieve rx link Vasanthakumar Thiagarajan
2022-08-09 18:15 ` Johannes Berg
2022-08-11 10:28 ` Vasanthakumar Thiagarajan
2023-01-19 20:57 ` Johannes Berg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.