| Message ID | 1619696874-30072-2-git-send-email-mkenna@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support to configure beacon tx mode | expand |
On 2021-04-29 04:47, Maharaja Kennadyrajan wrote: [..snip..] > +/** > + * 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_STAGGERED_MODE = 1, > + NL80211_BEACON_BURST_MODE = 2, > +}; > + [..snip..] > @@ -5330,6 +5331,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]); > + Note that in the case where NL80211_ATTR_BEACON_TX_MODE is not specified that beacon_tx_mode will be zero, which is not a valid nl80211_beacon_tx_mode enumeration. Should you renumber the nl80211_beacon_tx_mode enumerations so that the default mode has a value of 0? Or add NL80211_BEACON_DEFAULT_MODE = 0 and allow the driver to select a default mode?
On 2021-05-03 22:54, jjohnson@codeaurora.org wrote: > On 2021-04-29 04:47, Maharaja Kennadyrajan wrote: > [..snip..] >> +/** >> + * 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_STAGGERED_MODE = 1, >> + NL80211_BEACON_BURST_MODE = 2, >> +}; >> + > [..snip..] >> @@ -5330,6 +5331,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]); >> + > > Note that in the case where NL80211_ATTR_BEACON_TX_MODE is not > specified that > beacon_tx_mode will be zero, which is not a valid > nl80211_beacon_tx_mode enumeration. > > Should you renumber the nl80211_beacon_tx_mode enumerations so that the > default > mode has a value of 0? Or add NL80211_BEACON_DEFAULT_MODE = 0 and > allow the driver > to select a default mode? [Maha]: Yes, it will select the default mode as STAGGERED mode when the user set beacon_tx_mode as 0 in the hostapd conf file. Also, it will select the default mode as STAGGERED mode when the user didn't add beacon_tx_mode config in the hostapd conf file. This is how it is handled here already. Regards, Maha
On 2021-05-05 00:18, Maharaja Kennadyrajan wrote: > On 2021-05-03 22:54, jjohnson@codeaurora.org wrote: >> On 2021-04-29 04:47, Maharaja Kennadyrajan wrote: >> [..snip..] >>> +/** >>> + * 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_STAGGERED_MODE = 1, >>> + NL80211_BEACON_BURST_MODE = 2, >>> +}; >>> + >> [..snip..] >>> @@ -5330,6 +5331,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]); >>> + >> >> Note that in the case where NL80211_ATTR_BEACON_TX_MODE is not >> specified that >> beacon_tx_mode will be zero, which is not a valid >> nl80211_beacon_tx_mode enumeration. >> >> Should you renumber the nl80211_beacon_tx_mode enumerations so that >> the default >> mode has a value of 0? Or add NL80211_BEACON_DEFAULT_MODE = 0 and >> allow the driver >> to select a default mode? > > [Maha]: Yes, it will select the default mode as STAGGERED mode when the > user set > beacon_tx_mode as 0 in the hostapd conf file. > Also, it will select the default mode as STAGGERED mode when the user > didn't add > beacon_tx_mode config in the hostapd conf file. > This is how it is handled here already. Regardless of how it is handled, I still assert that there is a coding/logic error here since in the case the attribute is not present you send an invalid enumeration (0) to the driver. I'm suggesting to fix that logic/coding error either by renumbering the existing enumerations or by adding a new enumeration so that 0 is a valid enumeration.
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 5224f88..88ed048 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..c9a447a 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,13 @@ 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_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 7e811a3..edf6640 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 */ @@ -5330,6 +5331,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) @@ -11884,6 +11889,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> --- V2: Addressed Johannes's comment on v1 patch. Updated the commit log. include/net/cfg80211.h | 4 ++++ include/uapi/linux/nl80211.h | 15 +++++++++++++++ net/wireless/nl80211.c | 9 +++++++++ 3 files changed, 28 insertions(+)