linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] nl80211: Add HE UL MU fixed rate setting
@ 2021-07-29 19:41 Muna Sinada
  2021-08-17 13:48 ` Johannes Berg
  0 siblings, 1 reply; 4+ messages in thread
From: Muna Sinada @ 2021-07-29 19:41 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Muna Sinada

This patch adds nl80211 definitions, policies and parsing code required
to pass HE UL MU fixed rate settings.

Signed-off-by: Muna Sinada <msinada@codeaurora.org>
---
 include/net/cfg80211.h       | 1 +
 include/uapi/linux/nl80211.h | 2 ++
 net/wireless/nl80211.c       | 9 +++++++++
 3 files changed, 12 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 161cdf7df1a0..6aa10479a2ce 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -694,6 +694,7 @@ struct cfg80211_bitrate_mask {
 		enum nl80211_txrate_gi gi;
 		enum nl80211_he_gi he_gi;
 		enum nl80211_he_ltf he_ltf;
+		u16 he_ul_mcs[NL80211_HE_NSS_MAX];
 	} control[NUM_NL80211_BANDS];
 };
 
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index db474994fa73..db40b34eec06 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4839,6 +4839,7 @@ enum nl80211_key_attributes {
  *	see &struct nl80211_txrate_he
  * @NL80211_TXRATE_HE_GI: configure HE GI, 0.8us, 1.6us and 3.2us.
  * @NL80211_TXRATE_HE_LTF: configure HE LTF, 1XLTF, 2XLTF and 4XLTF.
+ * @NL80211_TXRATE_HE_UL: HE MCS rates of connected HE STA for uplink traffic.
  * @__NL80211_TXRATE_AFTER_LAST: internal
  * @NL80211_TXRATE_MAX: highest TX rate attribute
  */
@@ -4851,6 +4852,7 @@ enum nl80211_tx_rate_attributes {
 	NL80211_TXRATE_HE,
 	NL80211_TXRATE_HE_GI,
 	NL80211_TXRATE_HE_LTF,
+	NL80211_TXRATE_HE_UL,
 
 	/* keep last */
 	__NL80211_TXRATE_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 50eb405b0690..e3ed33940185 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -384,6 +384,7 @@ static const struct nla_policy nl80211_txattr_policy[NL80211_TXRATE_MAX + 1] = {
 	[NL80211_TXRATE_HE_LTF] = NLA_POLICY_RANGE(NLA_U8,
 						   NL80211_RATE_INFO_HE_1XLTF,
 						   NL80211_RATE_INFO_HE_4XLTF),
+	[NL80211_TXRATE_HE_UL] = NLA_POLICY_EXACT_LEN(sizeof(struct nl80211_txrate_he)),
 };
 
 static const struct nla_policy
@@ -4869,6 +4870,14 @@ static int nl80211_parse_tx_bitrate_mask(struct genl_info *info,
 		if (tb[NL80211_TXRATE_HE_LTF])
 			mask->control[band].he_ltf =
 				nla_get_u8(tb[NL80211_TXRATE_HE_LTF]);
+		if (tb[NL80211_TXRATE_HE_UL] &&
+		    !he_set_mcs_mask(info, wdev, sband,
+				     nla_data(tb[NL80211_TXRATE_HE_UL]),
+				     mask->control[band].he_ul_mcs)) {
+			NL_SET_ERR_MSG(info->extack,
+					    "Unable to build HE mcs mask");
+			return -EINVAL;
+		}
 
 		if (mask->control[band].legacy == 0) {
 			/* don't allow empty legacy rates if HT, VHT or HE
-- 
2.7.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nl80211: Add HE UL MU fixed rate setting
  2021-07-29 19:41 [PATCH v2] nl80211: Add HE UL MU fixed rate setting Muna Sinada
@ 2021-08-17 13:48 ` Johannes Berg
  2021-08-26 23:08   ` Muna Sinada
  0 siblings, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2021-08-17 13:48 UTC (permalink / raw)
  To: Muna Sinada; +Cc: linux-wireless

Hi,

On Thu, 2021-07-29 at 12:41 -0700, Muna Sinada wrote:
> This patch adds nl80211 definitions, policies and parsing code required
> to pass HE UL MU fixed rate settings.
> 

I don't understand how this is sufficient?

>  		enum nl80211_txrate_gi gi;
>  		enum nl80211_he_gi he_gi;
>  		enum nl80211_he_ltf he_ltf;

Previously, for HE rates, we had configurations for:
 * HE MCS
 * HE guard interval
 * HE LTF

I guess I can sort of follow that uplink traffic is a bit different and
not already configured by the setting for rate control we have today,
but why does it not need all these parameters?

Also, why is this not a per-station parameter? OK, maybe we don't really
want it to be a per-station parameter, or maybe the firmware/algorithm
that's selecting things there can't deal with that, but it feels odd to
combine it with the "rate control fixed rate" parameters you have here,
and do that without even any explanation of how this is supposed to
work.

This is going to need some work.

johannes


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v2] nl80211: Add HE UL MU fixed rate setting
  2021-08-17 13:48 ` Johannes Berg
@ 2021-08-26 23:08   ` Muna Sinada
  2021-09-14  5:57     ` Kalle Valo
  0 siblings, 1 reply; 4+ messages in thread
From: Muna Sinada @ 2021-08-26 23:08 UTC (permalink / raw)
  To: 'Johannes Berg'; +Cc: linux-wireless

Hello Johannes,

I agree that it is odd to combine this new attribute with the existing parameters. I will be removing "he_ul_mcs" out of cfg80211_bitrate_mask and passing it as a separate attribute in next version.

Thank you,
Muna 

-----Original Message-----
From: Johannes Berg <johannes@sipsolutions.net> 
Sent: Tuesday, August 17, 2021 6:49 AM
To: Muna Sinada <msinada@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2] nl80211: Add HE UL MU fixed rate setting

Hi,

On Thu, 2021-07-29 at 12:41 -0700, Muna Sinada wrote:
> This patch adds nl80211 definitions, policies and parsing code 
> required to pass HE UL MU fixed rate settings.
> 

I don't understand how this is sufficient?

>  		enum nl80211_txrate_gi gi;
>  		enum nl80211_he_gi he_gi;
>  		enum nl80211_he_ltf he_ltf;

Previously, for HE rates, we had configurations for:
 * HE MCS
 * HE guard interval
 * HE LTF

I guess I can sort of follow that uplink traffic is a bit different and not already configured by the setting for rate control we have today, but why does it not need all these parameters?

Also, why is this not a per-station parameter? OK, maybe we don't really want it to be a per-station parameter, or maybe the firmware/algorithm that's selecting things there can't deal with that, but it feels odd to combine it with the "rate control fixed rate" parameters you have here, and do that without even any explanation of how this is supposed to work.

This is going to need some work.

johannes



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nl80211: Add HE UL MU fixed rate setting
  2021-08-26 23:08   ` Muna Sinada
@ 2021-09-14  5:57     ` Kalle Valo
  0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2021-09-14  5:57 UTC (permalink / raw)
  To: Muna Sinada; +Cc: 'Johannes Berg', linux-wireless

"Muna Sinada" <msinada@codeaurora.org> writes:

> I agree that it is odd to combine this new attribute with the existing
> parameters. I will be removing "he_ul_mcs" out of
> cfg80211_bitrate_mask and passing it as a separate attribute in next
> version.

Please do not top post.

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

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

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-14  5:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 19:41 [PATCH v2] nl80211: Add HE UL MU fixed rate setting Muna Sinada
2021-08-17 13:48 ` Johannes Berg
2021-08-26 23:08   ` Muna Sinada
2021-09-14  5:57     ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).