[PATCH 1/2] cfg80211: Handling driver updated MU-EDCA params

[PATCH 1/2] cfg80211: Handling driver updated MU-EDCA params

[Muna Sinada]

Added necessary functions and attributes to be able to pass driver
updated MU-EDCA parameters from mac80211 to user space.

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

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ab249ca5d5d1..83e6101ad681 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -8038,4 +8038,16 @@ void cfg80211_update_owe_info_event(struct net_device *netdev,
  */
 void cfg80211_bss_flush(struct wiphy *wiphy);
 
+/**
+ * cfg80211_update_muedca_params_event - Notify the updated MU-EDCA parameters
+ *	to user space.
+ *
+ * @wiphy: the wiphy
+ * @params: Updated MU-EDCA parameters
+ * @gfp: allocation flags
+ */
+void cfg80211_update_muedca_params_event(struct wiphy *wiphy,
+					 struct ieee80211_mu_edca_param_set
+					 *params, gfp_t gfp);
+
 #endif /* __NET_CFG80211_H */
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 3e0d4a038ab6..0cd2e33f3df8 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1178,6 +1178,11 @@
  *	includes the contents of the frame. %NL80211_ATTR_ACK flag is included
  *	if the recipient acknowledged the frame.
  *
+ * @NL80211_CMD_UPDATE_HE_MUEDCA_PARAMS: Updated MU-EDCA parameters from
+ *	driver. This event is used to update dynamic MU-EDCA parameters in
+ *	Beacon frame, coming from driver and now need to be reflected in
+ *	Beacon frame.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1408,6 +1413,7 @@ enum nl80211_commands {
 
 	NL80211_CMD_CONTROL_PORT_FRAME_TX_STATUS,
 
+	NL80211_CMD_UPDATE_HE_MUEDCA_PARAMS,
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -2534,6 +2540,9 @@ enum nl80211_commands {
  *	This is a u8 attribute that encapsulates one of the values from
  *	&enum nl80211_sae_pwe_mechanism.
  *
+ * @NL80211_ATTR_HE_MUEDCA_PARAMS: MU-EDCA AC parameters for the
+ *	%NL80211_CMD_UPDATE_HE_MUEDCA_PARAMS command.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3025,6 +3034,7 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_SAE_PWE,
 
+	NL80211_ATTR_HE_MUEDCA_PARAMS,
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8811a4b69f21..75493758ec8e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -17806,6 +17806,42 @@ void cfg80211_update_owe_info_event(struct net_device *netdev,
 }
 EXPORT_SYMBOL(cfg80211_update_owe_info_event);
 
+void cfg80211_update_muedca_params_event(struct wiphy *wiphy,
+					 struct ieee80211_mu_edca_param_set
+					 *params, gfp_t gfp)
+{
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
+	struct sk_buff *msg;
+	void *hdr;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp);
+	if (!msg)
+		return;
+
+	hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_UPDATE_HE_MUEDCA_PARAMS);
+	if (!hdr)
+		goto nla_put_failure;
+
+	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx))
+		goto nla_put_failure;
+
+	if (nla_put(msg, NL80211_ATTR_HE_MUEDCA_PARAMS,
+		    sizeof(struct ieee80211_mu_edca_param_set),
+		    (const void *)params))
+		goto nla_put_failure;
+
+	genlmsg_end(msg, hdr);
+
+	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
+				NL80211_MCGRP_MLME, gfp);
+	return;
+
+nla_put_failure:
+	genlmsg_cancel(msg, hdr);
+	nlmsg_free(msg);
+}
+EXPORT_SYMBOL(cfg80211_update_muedca_params_event);
+
 /* initialisation/exit functions */
 
 int __init nl80211_init(void)
-- 
2.7.4

[PATCH 2/2] mac80211: Handling driver updated MU-EDCA params

[Muna Sinada]

Added necessary function to pass through driver updated MU-EDCA
parameters to user space.

Signed-off-by: Muna Sinada <msinada@codeaurora.org>
---
 include/net/mac80211.h | 15 +++++++++++++++
 net/mac80211/mlme.c    | 12 ++++++++++++
 net/mac80211/trace.h   | 20 ++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 05c7524bab26..eeadddaf1f6a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6695,4 +6695,19 @@ struct sk_buff *ieee80211_get_fils_discovery_tmpl(struct ieee80211_hw *hw,
 struct sk_buff *
 ieee80211_get_unsol_bcast_probe_resp_tmpl(struct ieee80211_hw *hw,
 					  struct ieee80211_vif *vif);
+
+/**
+ * ieee80211_update_muedca_params - update MU-EDCA parameters.
+ *
+ * This function is used to pass dynamically updated MU-EDCA parameters
+ * from driver to user space in order for parameters to be updated in beacon.
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @params: updated MU-EDCA parameters
+ * @gfp: allocation flags
+ */
+void ieee80211_update_muedca_params(struct ieee80211_hw *hw,
+				    struct ieee80211_mu_edca_param_set
+				    *params, gfp_t gfp);
+
 #endif /* MAC80211_H */
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 6adfcb9c06dc..aba8fecf8284 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5934,3 +5934,15 @@ void ieee80211_cqm_beacon_loss_notify(struct ieee80211_vif *vif, gfp_t gfp)
 	cfg80211_cqm_beacon_loss_notify(sdata->dev, gfp);
 }
 EXPORT_SYMBOL(ieee80211_cqm_beacon_loss_notify);
+
+void ieee80211_update_muedca_params(struct ieee80211_hw *hw,
+				    struct ieee80211_mu_edca_param_set
+				    *params, gfp_t gfp)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+
+	trace_api_update_muedca_params(local, params);
+
+	cfg80211_update_muedca_params_event(local->hw.wiphy, params, gfp);
+}
+EXPORT_SYMBOL(ieee80211_update_muedca_params);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 89723907a094..4ede71bea435 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -2767,6 +2767,26 @@ TRACE_EVENT(drv_sta_set_4addr,
 	)
 );
 
+TRACE_EVENT(api_update_muedca_params,
+	TP_PROTO(struct ieee80211_local *local,
+		 struct ieee80211_mu_edca_param_set *params),
+
+	TP_ARGS(local, params),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT " updated MU-EDCA parameters",
+		LOCAL_PR_ARG
+	)
+);
+
 #endif /* !__MAC80211_DRIVER_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.7.4

Re: [PATCH 1/2] cfg80211: Handling driver updated MU-EDCA params

[Johannes Berg]

On Tue, 2020-12-08 at 14:54 -0800, Muna Sinada wrote:
> Added necessary functions and attributes to be able to pass driver
> updated MU-EDCA parameters from mac80211 to user space.

Please write a proper commit message, in imperative voice.

> +/**
> + * cfg80211_update_muedca_params_event - Notify the updated MU-EDCA parameters
> + *	to user space.

"Notify the ... to" sounds really odd to me - "notify userspace about
updated ..." or something?

Also this is missing a lot of commit message to explain what this does
and when it's needed.

> + *
> + * @wiphy: the wiphy
> + * @params: Updated MU-EDCA parameters
> + * @gfp: allocation flags
> + */
> +void cfg80211_update_muedca_params_event(struct wiphy *wiphy,
> +					 struct ieee80211_mu_edca_param_set
> +					 *params, gfp_t gfp);

that line breaking is awful

> @@ -1408,6 +1413,7 @@ enum nl80211_commands {
>  
>  	NL80211_CMD_CONTROL_PORT_FRAME_TX_STATUS,
>  
> +	NL80211_CMD_UPDATE_HE_MUEDCA_PARAMS,
>  	/* add new commands above here */

a blank line would be nice

>   *	This is a u8 attribute that encapsulates one of the values from
>   *	&enum nl80211_sae_pwe_mechanism.
>   *
> + * @NL80211_ATTR_HE_MUEDCA_PARAMS: MU-EDCA AC parameters for the
> + *	%NL80211_CMD_UPDATE_HE_MUEDCA_PARAMS command.

That should explain the data type.

>   * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>   * @NL80211_ATTR_MAX: highest attribute number currently defined
>   * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -3025,6 +3034,7 @@ enum nl80211_attrs {
>  
>  	NL80211_ATTR_SAE_PWE,
>  
> +	NL80211_ATTR_HE_MUEDCA_PARAMS,
>  	/* add attributes here, update the policy in nl80211.c */

blank line again

> +void cfg80211_update_muedca_params_event(struct wiphy *wiphy,
> +					 struct ieee80211_mu_edca_param_set
> +					 *params, gfp_t gfp)

line breaks are awful

probably should have tracing.

johannes

Re: [PATCH 2/2] mac80211: Handling driver updated MU-EDCA params

[Johannes Berg]

> +void ieee80211_update_muedca_params(struct ieee80211_hw *hw,
> +				    struct ieee80211_mu_edca_param_set
> +				    *params, gfp_t gfp)
> +{
> +	struct ieee80211_local *local = hw_to_local(hw);
> +
> +	trace_api_update_muedca_params(local, params);
> +
> +	cfg80211_update_muedca_params_event(local->hw.wiphy, params, gfp);
> +}
> +EXPORT_SYMBOL(ieee80211_update_muedca_params);
> 

I see no reason to have this trivial wrapper.

johannes

RE: [PATCH 2/2] mac80211: Handling driver updated MU-EDCA params

[Muna Sinada]

Hello Johannes,

I saw on your review comment that this wrapper is not needed. I wanted to confirm with you if it would be ok to call a cfg80211 API from a mac80211 based driver, since that is what would be done if this wrapper is removed. Additionally, another reason I have this wrapper is so I can place a tracepoint. What are your thoughts about this?

Thank you,
Muna  

-----Original Message-----
From: Johannes Berg <johannes@sipsolutions.net> 
Sent: Friday, February 12, 2021 12:49 AM
To: Muna Sinada <msinada@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH 2/2] mac80211: Handling driver updated MU-EDCA params


> +void ieee80211_update_muedca_params(struct ieee80211_hw *hw,
> +				    struct ieee80211_mu_edca_param_set
> +				    *params, gfp_t gfp)
> +{
> +	struct ieee80211_local *local = hw_to_local(hw);
> +
> +	trace_api_update_muedca_params(local, params);
> +
> +	cfg80211_update_muedca_params_event(local->hw.wiphy, params, gfp); } 
> +EXPORT_SYMBOL(ieee80211_update_muedca_params);
> 

I see no reason to have this trivial wrapper.

johannes

Re: [PATCH 2/2] mac80211: Handling driver updated MU-EDCA params

[Johannes Berg]

On Tue, 2021-08-10 at 19:48 -0700, Muna Sinada wrote:
> Hello Johannes,
> 
> I saw on your review comment that this wrapper is not needed. I wanted
> to confirm with you if it would be ok to call a cfg80211 API from a
> mac80211 based driver, since that is what would be done if this
> wrapper is removed.

Yes, that's fine.

>  Additionally, another reason I have this wrapper is so I can place a
> tracepoint. What are your thoughts about this?
You already have a tracepoint on cfg80211, seems sufficient?

johannes

RE: [PATCH 2/2] mac80211: Handling driver updated MU-EDCA params

[Muna Sinada]

Hello Johannes,

Please drop this patch.

Thank you,
Muna

-----Original Message-----
From: Johannes Berg <johannes@sipsolutions.net> 
Sent: Wednesday, August 11, 2021 3:07 AM
To: Muna Sinada <msinada@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH 2/2] mac80211: Handling driver updated MU-EDCA params

On Tue, 2021-08-10 at 19:48 -0700, Muna Sinada wrote:
> Hello Johannes,
> 
> I saw on your review comment that this wrapper is not needed. I wanted 
> to confirm with you if it would be ok to call a cfg80211 API from a
> mac80211 based driver, since that is what would be done if this 
> wrapper is removed.

Yes, that's fine.

>  Additionally, another reason I have this wrapper is so I can place a 
> tracepoint. What are your thoughts about this?
You already have a tracepoint on cfg80211, seems sufficient?

johannes