| Message ID | 1607468044-31789-1-git-send-email-msinada@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] cfg80211: Handling driver updated MU-EDCA params | expand |
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
> +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
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
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
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
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)
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(+)