| Message ID | 1623164855-15910-2-git-send-email-mkenna@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support to configure beacon tx mode | expand |
First, I'd like to see some input from other vendors - is this useful? I have no objections to it and it comes with an in-kernel driver, so I'm inclined to accept it, but maybe we should have other modes, etc.? > + * @NL80211_ATTR_BEACON_TX_MODE: used to configure the beacon tx mode as > + * staggered mode = 1 or burst mode = 2 in %NL80211_CMD_START_AP or > + * %NL80211_CMD_JOIN_MESH from user-space. This should refer to the enum, not the hard-coded constants. > > +/** > + * enum nl80211_beacon_tx_mode - Beacon Tx Mode enum. > + * Used to configure beacon staggered mode or beacon burst mode. > + */ > +enum nl80211_beacon_tx_mode { That's missing kernel-doc, and you also don't specify what default means. Why is it even here? > + [NL80211_ATTR_BEACON_TX_MODE] = NLA_POLICY_RANGE(NLA_U32, 1, 2), That really also needs to use the enum, not hard-coded constants... johannes
On 2021-06-23 13:16, Johannes Berg wrote: > First, I'd like to see some input from other vendors - is this useful? > > I have no objections to it and it comes with an in-kernel driver, so I'm > inclined to accept it, but maybe we should have other modes, etc.? I haven't seen any config option like this outside of QCA stuff. It seems to me that before getting merged, the documentation should at least cover when and why the *user* would configure this implementation detail, instead of having the driver/fw make a reasonable decision on its own. - Felix
On 2021-06-23 16:46, Johannes Berg wrote: > First, I'd like to see some input from other vendors - is this useful? > > I have no objections to it and it comes with an in-kernel driver, so > I'm > inclined to accept it, but maybe we should have other modes, etc.? > > >> + * @NL80211_ATTR_BEACON_TX_MODE: used to configure the beacon tx mode >> as >> + * staggered mode = 1 or burst mode = 2 in %NL80211_CMD_START_AP >> or >> + * %NL80211_CMD_JOIN_MESH from user-space. > [Maha]: will replace the hard-coded values with enum here. > > This should refer to the enum, not the hard-coded constants. >> >> +/** >> + * enum nl80211_beacon_tx_mode - Beacon Tx Mode enum. >> + * Used to configure beacon staggered mode or beacon burst mode. >> + */ >> +enum nl80211_beacon_tx_mode { > > > That's missing kernel-doc, and you also don't specify what default > means. Why is it even here? > [Maha]: sure, I will add the same in the kernel-doc. >> + [NL80211_ATTR_BEACON_TX_MODE] = NLA_POLICY_RANGE(NLA_U32, 1, > 2), > > That really also needs to use the enum, not hard-coded constants... [Maha]: will replace the hard-coded values with enum also here. > > johannes
On 2021-06-29 23:14, Felix Fietkau wrote: > On 2021-06-23 13:16, Johannes Berg wrote: >> First, I'd like to see some input from other vendors - is this useful? >> >> I have no objections to it and it comes with an in-kernel driver, so >> I'm >> inclined to accept it, but maybe we should have other modes, etc.? > I haven't seen any config option like this outside of QCA stuff. > It seems to me that before getting merged, the documentation should at > least cover when and why the *user* would configure this implementation > detail, instead of having the driver/fw make a reasonable decision on > its own. > > - Felix [Maha]: I will do the documentation for this in the next version patchset.
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 58c2cd4..65161fd 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -1182,6 +1182,7 @@ enum cfg80211_ap_settings_flags { * @he_oper: HE operation IE (or %NULL if HE isn't enabled) * @fils_discovery: FILS discovery transmission parameters * @unsol_bcast_probe_resp: Unsolicited broadcast probe response parameters + * @beacon_tx_mode: Beacon Tx Mode setting */ struct cfg80211_ap_settings { struct cfg80211_chan_def chandef; @@ -1214,6 +1215,7 @@ struct cfg80211_ap_settings { struct cfg80211_he_bss_color he_bss_color; struct cfg80211_fils_discovery fils_discovery; struct cfg80211_unsol_bcast_probe_resp unsol_bcast_probe_resp; + enum nl80211_beacon_tx_mode beacon_tx_mode; }; /** @@ -2061,6 +2063,7 @@ struct mesh_config { * to operate on DFS channels. * @control_port_over_nl80211: TRUE if userspace expects to exchange control * port frames over NL80211 instead of the network interface. + * @beacon_tx_mode: Beacon Tx Mode setting. * * These parameters are fixed when the mesh is created. */ @@ -2084,6 +2087,7 @@ struct mesh_setup { struct cfg80211_bitrate_mask beacon_rate; bool userspace_handles_dfs; bool control_port_over_nl80211; + enum nl80211_beacon_tx_mode beacon_tx_mode; }; /** diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index f962c06..a86b32e 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -2560,6 +2560,10 @@ enum nl80211_commands { * disassoc events to indicate that an immediate reconnect to the AP * is desired. * + * @NL80211_ATTR_BEACON_TX_MODE: used to configure the beacon tx mode as + * staggered mode = 1 or burst mode = 2 in %NL80211_CMD_START_AP or + * %NL80211_CMD_JOIN_MESH from user-space. + * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use @@ -3057,6 +3061,8 @@ enum nl80211_attrs { NL80211_ATTR_DISABLE_HE, + NL80211_ATTR_BEACON_TX_MODE, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, @@ -7299,4 +7305,14 @@ enum nl80211_sar_specs_attrs { NL80211_SAR_ATTR_SPECS_MAX = __NL80211_SAR_ATTR_SPECS_LAST - 1, }; +/** + * enum nl80211_beacon_tx_mode - Beacon Tx Mode enum. + * Used to configure beacon staggered mode or beacon burst mode. + */ +enum nl80211_beacon_tx_mode { + NL80211_BEACON_DEFAULT_MODE = 0, + NL80211_BEACON_STAGGERED_MODE = 1, + NL80211_BEACON_BURST_MODE = 2, +}; + #endif /* __LINUX_NL80211_H */ diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index fc9286a..6076d00 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -759,6 +759,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { [NL80211_ATTR_RECONNECT_REQUESTED] = { .type = NLA_REJECT }, [NL80211_ATTR_SAR_SPEC] = NLA_POLICY_NESTED(sar_policy), [NL80211_ATTR_DISABLE_HE] = { .type = NLA_FLAG }, + [NL80211_ATTR_BEACON_TX_MODE] = NLA_POLICY_RANGE(NLA_U32, 1, 2), }; /* policy for the key attributes */ @@ -5342,6 +5343,10 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info) params.dtim_period = nla_get_u32(info->attrs[NL80211_ATTR_DTIM_PERIOD]); + if (info->attrs[NL80211_ATTR_BEACON_TX_MODE]) + params.beacon_tx_mode = + nla_get_u32(info->attrs[NL80211_ATTR_BEACON_TX_MODE]); + err = cfg80211_validate_beacon_int(rdev, dev->ieee80211_ptr->iftype, params.beacon_interval); if (err) @@ -11896,6 +11901,10 @@ static int nl80211_join_mesh(struct sk_buff *skb, struct genl_info *info) return -EINVAL; } + if (info->attrs[NL80211_ATTR_BEACON_TX_MODE]) + setup.beacon_tx_mode = + nla_get_u32(info->attrs[NL80211_ATTR_BEACON_TX_MODE]); + if (info->attrs[NL80211_ATTR_MESH_SETUP]) { /* parse additional setup parameters if given */ err = nl80211_parse_mesh_setup(info, &setup);
User can configure the below beacon tx mode 1. Staggered mode and 2. Burst mode while bring-up the AP or MESH. Beacons can be sent out in burst(continuously in a single shot one after another) or staggered (equally spread out over beacon interval) mode. Set the beacon_tx_mode as 1 for Staggered mode and 2 for burst mode. Hence, added the support in the nl80211/cfg80211 layer to honour the beacon tx mode configuration and pass this value to the driver. Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-00630-QCAHKSWPL_SILICONZ-2 Signed-off-by: Maharaja Kennadyrajan <mkenna@codeaurora.org> --- V3: Addressed Johnson's comment on v2 patch. V2: Addressed Johannes's comment on v1 patch. include/net/cfg80211.h | 4 ++++ include/uapi/linux/nl80211.h | 16 ++++++++++++++++ net/wireless/nl80211.c | 9 +++++++++ 3 files changed, 29 insertions(+)