2023-10-10 02:11:05

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH v2 6/6] wifi: rtw89: add EHT radiotap in monitor mode

Add IEEE80211_RADIOTAP_EHT and IEEE80211_RADIOTAP_EHT_USIG radiotap to
fill basic EHT NSS, MCS, GI and bandwidth.

Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw89/core.c | 66 +++++++++++++++++++++++
drivers/net/wireless/realtek/rtw89/core.h | 9 +++-
2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index 2742e6646cf1..8cb1715d049a 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -1907,6 +1907,70 @@ static void rtw89_core_hw_to_sband_rate(struct ieee80211_rx_status *rx_status)
rx_status->rate_idx -= 4;
}

+static u8 rx_status_bw_to_radiotap_eht_usig[] = {
+ [RATE_INFO_BW_20] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_20MHZ,
+ [RATE_INFO_BW_5] = U8_MAX,
+ [RATE_INFO_BW_10] = U8_MAX,
+ [RATE_INFO_BW_40] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_40MHZ,
+ [RATE_INFO_BW_80] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_80MHZ,
+ [RATE_INFO_BW_160] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_160MHZ,
+ [RATE_INFO_BW_HE_RU] = U8_MAX,
+ [RATE_INFO_BW_320] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_320MHZ_1,
+ [RATE_INFO_BW_EHT_RU] = U8_MAX,
+};
+
+static void rtw89_core_update_radiotap_eht(struct rtw89_dev *rtwdev,
+ struct sk_buff *skb,
+ struct ieee80211_rx_status *rx_status)
+{
+ struct ieee80211_radiotap_eht_usig *usig;
+ struct ieee80211_radiotap_eht *eht;
+ struct ieee80211_radiotap_tlv *tlv;
+ int eht_len = struct_size(eht, user_info, 1);
+ int usig_len = sizeof(*usig);
+ int len;
+ u8 bw;
+
+ len = sizeof(*tlv) + ALIGN(eht_len, 4) +
+ sizeof(*tlv) + ALIGN(usig_len, 4);
+
+ rx_status->flag |= RX_FLAG_RADIOTAP_TLV_AT_END;
+ skb_reset_mac_header(skb);
+
+ /* EHT */
+ tlv = skb_push(skb, len);
+ memset(tlv, 0, len);
+ tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT);
+ tlv->len = cpu_to_le16(eht_len);
+
+ eht = (struct ieee80211_radiotap_eht *)tlv->data;
+ eht->known = cpu_to_le32(IEEE80211_RADIOTAP_EHT_KNOWN_GI);
+ eht->data[0] =
+ le32_encode_bits(rx_status->eht.gi, IEEE80211_RADIOTAP_EHT_DATA0_GI);
+
+ eht->user_info[0] =
+ cpu_to_le32(IEEE80211_RADIOTAP_EHT_USER_INFO_MCS_KNOWN |
+ IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_KNOWN_O);
+ eht->user_info[0] |=
+ le32_encode_bits(rx_status->rate_idx, IEEE80211_RADIOTAP_EHT_USER_INFO_MCS) |
+ le32_encode_bits(rx_status->nss, IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_O);
+
+ /* U-SIG */
+ tlv = (void *)tlv + sizeof(*tlv) + ALIGN(eht_len, 4);
+ tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT_USIG);
+ tlv->len = cpu_to_le16(usig_len);
+
+ bw = rx_status->bw < ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig) ?
+ rx_status_bw_to_radiotap_eht_usig[rx_status->bw] : U8_MAX;
+ if (bw == U8_MAX)
+ return;
+
+ usig = (struct ieee80211_radiotap_eht_usig *)tlv->data;
+ usig->common =
+ le32_encode_bits(1, IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_KNOWN) |
+ le32_encode_bits(bw, IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW);
+}
+
static void rtw89_core_update_radiotap(struct rtw89_dev *rtwdev,
struct sk_buff *skb,
struct ieee80211_rx_status *rx_status)
@@ -1925,6 +1989,8 @@ static void rtw89_core_update_radiotap(struct rtw89_dev *rtwdev,
rx_status->flag |= RX_FLAG_RADIOTAP_HE;
he = skb_push(skb, sizeof(*he));
*he = known_he;
+ } else if (rx_status->encoding == RX_ENC_EHT) {
+ rtw89_core_update_radiotap_eht(rtwdev, skb, rx_status);
}
}

diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index 8c0dfd73031e..d5272a82ff8b 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -37,7 +37,14 @@ extern const struct ieee80211_ops rtw89_ops;
#define RSSI_FACTOR 1
#define RTW89_RSSI_RAW_TO_DBM(rssi) ((s8)((rssi) >> RSSI_FACTOR) - MAX_RSSI)
#define RTW89_TX_DIV_RSSI_RAW_TH (2 << RSSI_FACTOR)
-#define RTW89_RADIOTAP_ROOM ALIGN(sizeof(struct ieee80211_radiotap_he), 64)
+#define RTW89_RADIOTAP_ROOM_HE sizeof(struct ieee80211_radiotap_he)
+#define RTW89_RADIOTAP_ROOM_EHT \
+ (sizeof(struct ieee80211_radiotap_tlv) + \
+ ALIGN(struct_size((struct ieee80211_radiotap_eht *)0, user_info, 1), 4) + \
+ sizeof(struct ieee80211_radiotap_tlv) + \
+ ALIGN(sizeof(struct ieee80211_radiotap_eht_usig), 4))
+#define RTW89_RADIOTAP_ROOM \
+ ALIGN(max(RTW89_RADIOTAP_ROOM_HE, RTW89_RADIOTAP_ROOM_EHT), 64)

#define RTW89_HTC_MASK_VARIANT GENMASK(1, 0)
#define RTW89_HTC_VARIANT_HE 3
--
2.25.1


2023-10-11 09:13:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] wifi: rtw89: add EHT radiotap in monitor mode

Ping-Ke Shih <[email protected]> writes:

> Add IEEE80211_RADIOTAP_EHT and IEEE80211_RADIOTAP_EHT_USIG radiotap to
> fill basic EHT NSS, MCS, GI and bandwidth.
>
> Signed-off-by: Ping-Ke Shih <[email protected]>
> ---
> drivers/net/wireless/realtek/rtw89/core.c | 66 +++++++++++++++++++++++
> drivers/net/wireless/realtek/rtw89/core.h | 9 +++-
> 2 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
> index 2742e6646cf1..8cb1715d049a 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.c
> +++ b/drivers/net/wireless/realtek/rtw89/core.c
> @@ -1907,6 +1907,70 @@ static void rtw89_core_hw_to_sband_rate(struct ieee80211_rx_status *rx_status)
> rx_status->rate_idx -= 4;
> }
>
> +static u8 rx_status_bw_to_radiotap_eht_usig[] = {
> + [RATE_INFO_BW_20] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_20MHZ,
> + [RATE_INFO_BW_5] = U8_MAX,
> + [RATE_INFO_BW_10] = U8_MAX,
> + [RATE_INFO_BW_40] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_40MHZ,
> + [RATE_INFO_BW_80] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_80MHZ,
> + [RATE_INFO_BW_160] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_160MHZ,
> + [RATE_INFO_BW_HE_RU] = U8_MAX,
> + [RATE_INFO_BW_320] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_320MHZ_1,
> + [RATE_INFO_BW_EHT_RU] = U8_MAX,
> +};

Sorry, I noticed this only when I was abot to commit this. Should this
be static const?

> +static void rtw89_core_update_radiotap_eht(struct rtw89_dev *rtwdev,
> + struct sk_buff *skb,
> + struct ieee80211_rx_status *rx_status)
> +{
> + struct ieee80211_radiotap_eht_usig *usig;
> + struct ieee80211_radiotap_eht *eht;
> + struct ieee80211_radiotap_tlv *tlv;
> + int eht_len = struct_size(eht, user_info, 1);
> + int usig_len = sizeof(*usig);
> + int len;
> + u8 bw;
> +
> + len = sizeof(*tlv) + ALIGN(eht_len, 4) +
> + sizeof(*tlv) + ALIGN(usig_len, 4);
> +
> + rx_status->flag |= RX_FLAG_RADIOTAP_TLV_AT_END;
> + skb_reset_mac_header(skb);
> +
> + /* EHT */
> + tlv = skb_push(skb, len);
> + memset(tlv, 0, len);
> + tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT);
> + tlv->len = cpu_to_le16(eht_len);
> +
> + eht = (struct ieee80211_radiotap_eht *)tlv->data;
> + eht->known = cpu_to_le32(IEEE80211_RADIOTAP_EHT_KNOWN_GI);
> + eht->data[0] =
> + le32_encode_bits(rx_status->eht.gi, IEEE80211_RADIOTAP_EHT_DATA0_GI);
> +
> + eht->user_info[0] =
> + cpu_to_le32(IEEE80211_RADIOTAP_EHT_USER_INFO_MCS_KNOWN |
> + IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_KNOWN_O);
> + eht->user_info[0] |=
> + le32_encode_bits(rx_status->rate_idx, IEEE80211_RADIOTAP_EHT_USER_INFO_MCS) |
> + le32_encode_bits(rx_status->nss, IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_O);
> +
> + /* U-SIG */
> + tlv = (void *)tlv + sizeof(*tlv) + ALIGN(eht_len, 4);
> + tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT_USIG);
> + tlv->len = cpu_to_le16(usig_len);
> +
> + bw = rx_status->bw < ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig) ?
> + rx_status_bw_to_radiotap_eht_usig[rx_status->bw] : U8_MAX;
> + if (bw == U8_MAX)
> + return;

This is cosmetics but I feel that 'if' statement is more readable than
':' operator:

if (rx_status->bw >= ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig)
return;

bw = rx_status_bw_to_radiotap_eht_usig[rx_status->bw];

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2023-10-11 09:19:53

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH v2 6/6] wifi: rtw89: add EHT radiotap in monitor mode



> -----Original Message-----
> From: Kalle Valo <[email protected]>
> Sent: Wednesday, October 11, 2023 5:14 PM
> To: Ping-Ke Shih <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH v2 6/6] wifi: rtw89: add EHT radiotap in monitor mode
>
> Ping-Ke Shih <[email protected]> writes:
>
> > Add IEEE80211_RADIOTAP_EHT and IEEE80211_RADIOTAP_EHT_USIG radiotap to
> > fill basic EHT NSS, MCS, GI and bandwidth.
> >
> > Signed-off-by: Ping-Ke Shih <[email protected]>
> > ---
> > drivers/net/wireless/realtek/rtw89/core.c | 66 +++++++++++++++++++++++
> > drivers/net/wireless/realtek/rtw89/core.h | 9 +++-
> > 2 files changed, 74 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
> > index 2742e6646cf1..8cb1715d049a 100644
> > --- a/drivers/net/wireless/realtek/rtw89/core.c
> > +++ b/drivers/net/wireless/realtek/rtw89/core.c
> > @@ -1907,6 +1907,70 @@ static void rtw89_core_hw_to_sband_rate(struct ieee80211_rx_status *rx_status)
> > rx_status->rate_idx -= 4;
> > }
> >
> > +static u8 rx_status_bw_to_radiotap_eht_usig[] = {
> > + [RATE_INFO_BW_20] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_20MHZ,
> > + [RATE_INFO_BW_5] = U8_MAX,
> > + [RATE_INFO_BW_10] = U8_MAX,
> > + [RATE_INFO_BW_40] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_40MHZ,
> > + [RATE_INFO_BW_80] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_80MHZ,
> > + [RATE_INFO_BW_160] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_160MHZ,
> > + [RATE_INFO_BW_HE_RU] = U8_MAX,
> > + [RATE_INFO_BW_320] = IEEE80211_RADIOTAP_EHT_USIG_COMMON_BW_320MHZ_1,
> > + [RATE_INFO_BW_EHT_RU] = U8_MAX,
> > +};
>
> Sorry, I noticed this only when I was abot to commit this. Should this
> be static const?

Yes, I miss it.

>
> > +static void rtw89_core_update_radiotap_eht(struct rtw89_dev *rtwdev,
> > + struct sk_buff *skb,
> > + struct ieee80211_rx_status *rx_status)
> > +{
> > + struct ieee80211_radiotap_eht_usig *usig;
> > + struct ieee80211_radiotap_eht *eht;
> > + struct ieee80211_radiotap_tlv *tlv;
> > + int eht_len = struct_size(eht, user_info, 1);
> > + int usig_len = sizeof(*usig);
> > + int len;
> > + u8 bw;
> > +
> > + len = sizeof(*tlv) + ALIGN(eht_len, 4) +
> > + sizeof(*tlv) + ALIGN(usig_len, 4);
> > +
> > + rx_status->flag |= RX_FLAG_RADIOTAP_TLV_AT_END;
> > + skb_reset_mac_header(skb);
> > +
> > + /* EHT */
> > + tlv = skb_push(skb, len);
> > + memset(tlv, 0, len);
> > + tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT);
> > + tlv->len = cpu_to_le16(eht_len);
> > +
> > + eht = (struct ieee80211_radiotap_eht *)tlv->data;
> > + eht->known = cpu_to_le32(IEEE80211_RADIOTAP_EHT_KNOWN_GI);
> > + eht->data[0] =
> > + le32_encode_bits(rx_status->eht.gi, IEEE80211_RADIOTAP_EHT_DATA0_GI);
> > +
> > + eht->user_info[0] =
> > + cpu_to_le32(IEEE80211_RADIOTAP_EHT_USER_INFO_MCS_KNOWN |
> > + IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_KNOWN_O);
> > + eht->user_info[0] |=
> > + le32_encode_bits(rx_status->rate_idx, IEEE80211_RADIOTAP_EHT_USER_INFO_MCS) |
> > + le32_encode_bits(rx_status->nss, IEEE80211_RADIOTAP_EHT_USER_INFO_NSS_O);
> > +
> > + /* U-SIG */
> > + tlv = (void *)tlv + sizeof(*tlv) + ALIGN(eht_len, 4);
> > + tlv->type = cpu_to_le16(IEEE80211_RADIOTAP_EHT_USIG);
> > + tlv->len = cpu_to_le16(usig_len);
> > +
> > + bw = rx_status->bw < ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig) ?
> > + rx_status_bw_to_radiotap_eht_usig[rx_status->bw] : U8_MAX;
> > + if (bw == U8_MAX)
> > + return;
>
> This is cosmetics but I feel that 'if' statement is more readable than
> ':' operator:
>
> if (rx_status->bw >= ARRAY_SIZE(rx_status_bw_to_radiotap_eht_usig)
> return;
>
> bw = rx_status_bw_to_radiotap_eht_usig[rx_status->bw];
>

Got it. I will prepare v3 for them.

Thank you.