Re: [PATCH v2 1/3] nl80211: Add support for beacon tx mode
From: <hidden>
Date: 2021-05-05 14:27:45
Also in:
ath11k
On 2021-05-05 00:18, Maharaja Kennadyrajan wrote:
On 2021-05-03 22:54, jjohnson@codeaurora.org wrote:quoted
On 2021-04-29 04:47, Maharaja Kennadyrajan wrote: [..snip..]quoted
+/** + * 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..]quoted
@@ -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.