| Message ID | 1601717108-1030-1-git-send-email-rmanohar@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | nl80211: clear VHT and HE MCS rates in legacy beacon tx rate | expand |
On Sat, 2020-10-03 at 02:25 -0700, Rajkumar Manoharan wrote: > While configuring legacy beacon rate, VHT and HE MCS rates must be cleared. > Otherwise the default MCS rates is filled by cfg80211 while validating beacon > tx rate netlink attribute. This issue is exposed by ap_beacon_rate_legacy2 > test case while validating HE MCS beacon tx rate support. As I said with the kernel patch, I don't think that the kernel's side is approaching this the right way - better to default to no rate if the attribute isn't present, at least for the purposes of beacon rate settings. Maybe not for other purposes. johannes
On 2020-10-03 04:01, Johannes Berg wrote: > On Sat, 2020-10-03 at 02:25 -0700, Rajkumar Manoharan wrote: >> While configuring legacy beacon rate, VHT and HE MCS rates must be >> cleared. >> Otherwise the default MCS rates is filled by cfg80211 while validating >> beacon >> tx rate netlink attribute. This issue is exposed by >> ap_beacon_rate_legacy2 >> test case while validating HE MCS beacon tx rate support. > > As I said with the kernel patch, I don't think that the kernel's side > is > approaching this the right way - better to default to no rate if the > attribute isn't present, at least for the purposes of beacon rate > settings. Maybe not for other purposes. > Understood. I submit the patch for cfg80211. Shall we retain this patch or drop it? Rajkumar
On Sat, 2020-10-03 at 14:33 -0700, Rajkumar Manoharan wrote: > Understood. I submit the patch for cfg80211. Thanks. > Shall we retain this patch > or drop it? I guess it's no longer needed then, but you'll want a patch that makes it actually possible to _use_ HE MCSes? johannes
On Sat, Oct 03, 2020 at 02:25:08AM -0700, Rajkumar Manoharan wrote: > While configuring legacy beacon rate, VHT and HE MCS rates must be cleared. > Otherwise the default MCS rates is filled by cfg80211 while validating beacon > tx rate netlink attribute. This issue is exposed by ap_beacon_rate_legacy2 > test case while validating HE MCS beacon tx rate support. I'm dropping this based on the discussion on where this should be fixed (in kernel instead of hostapd).
On Mon, 2021-02-08 at 00:42 +0200, Jouni Malinen wrote: > On Sat, Oct 03, 2020 at 02:25:08AM -0700, Rajkumar Manoharan wrote: > > While configuring legacy beacon rate, VHT and HE MCS rates must be cleared. > > Otherwise the default MCS rates is filled by cfg80211 while validating beacon > > tx rate netlink attribute. This issue is exposed by ap_beacon_rate_legacy2 > > test case while validating HE MCS beacon tx rate support. > > I'm dropping this based on the discussion on where this should be fixed > (in kernel instead of hostapd). And indeed has been. johannes
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index 2ee34d11d64e..c435d54b5168 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -4165,6 +4165,7 @@ static int nl80211_put_beacon_rate(struct nl_msg *msg, const u64 flags, { struct nlattr *bands, *band; struct nl80211_txrate_vht vht_rate; + struct nl80211_txrate_he he_rate; if (!params->freq || (params->beacon_rate == 0 && @@ -4194,6 +4195,7 @@ static int nl80211_put_beacon_rate(struct nl_msg *msg, const u64 flags, return -1; os_memset(&vht_rate, 0, sizeof(vht_rate)); + os_memset(&he_rate, 0, sizeof(he_rate)); switch (params->rate_type) { case BEACON_RATE_LEGACY: @@ -4206,9 +4208,9 @@ static int nl80211_put_beacon_rate(struct nl_msg *msg, const u64 flags, if (nla_put_u8(msg, NL80211_TXRATE_LEGACY, (u8) params->beacon_rate / 5) || nla_put(msg, NL80211_TXRATE_HT, 0, NULL) || - (params->freq->vht_enabled && - nla_put(msg, NL80211_TXRATE_VHT, sizeof(vht_rate), - &vht_rate))) + nla_put(msg, NL80211_TXRATE_VHT, sizeof(vht_rate), + &vht_rate) || + nla_put(msg, NL80211_TXRATE_HE, sizeof(he_rate), &he_rate)) return -1; wpa_printf(MSG_DEBUG, " * beacon_rate = legacy:%u (* 100 kbps)", @@ -4222,9 +4224,9 @@ static int nl80211_put_beacon_rate(struct nl_msg *msg, const u64 flags, } if (nla_put(msg, NL80211_TXRATE_LEGACY, 0, NULL) || nla_put_u8(msg, NL80211_TXRATE_HT, params->beacon_rate) || - (params->freq->vht_enabled && - nla_put(msg, NL80211_TXRATE_VHT, sizeof(vht_rate), - &vht_rate))) + nla_put(msg, NL80211_TXRATE_VHT, sizeof(vht_rate), + &vht_rate) || + nla_put(msg, NL80211_TXRATE_HE, sizeof(he_rate), &he_rate)) return -1; wpa_printf(MSG_DEBUG, " * beacon_rate = HT-MCS %u", params->beacon_rate); @@ -4243,6 +4245,8 @@ static int nl80211_put_beacon_rate(struct nl_msg *msg, const u64 flags, if (nla_put(msg, NL80211_TXRATE_VHT, sizeof(vht_rate), &vht_rate)) return -1; + if (nla_put(msg, NL80211_TXRATE_HE, sizeof(he_rate), &he_rate)) + return -1; wpa_printf(MSG_DEBUG, " * beacon_rate = VHT-MCS %u", params->beacon_rate); break;
While configuring legacy beacon rate, VHT and HE MCS rates must be cleared. Otherwise the default MCS rates is filled by cfg80211 while validating beacon tx rate netlink attribute. This issue is exposed by ap_beacon_rate_legacy2 test case while validating HE MCS beacon tx rate support. Reported-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: Rajkumar Manoharan <rmanohar@codeaurora.org> --- src/drivers/driver_nl80211.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)