| Message ID | 20211026204144.29250-1-ps.report@gmx.net (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [RFC,v1] mac80211: minstrel_ht: respect RTS threshold setting | expand |
On Tue, 26 Oct 2021 22:41:44 +0200, Peter Seiderer <ps.report@gmx.net> wrote: > Despite the 'RTS thr:off' setting a wireshark trace of IBSS > traffic with HT40 mode enabled between two ath9k cards revealed > some RTS/CTS traffic. > > Debug and code analysis showed that most places setting > IEEE80211_TX_RC_USE_RTS_CTS respect the RTS strategy by > evaluating rts_threshold, e.g. net/mac80211/tx.c: > > 698 /* set up RTS protection if desired */ > 699 if (len > tx->local->hw.wiphy->rts_threshold) { > 700 txrc.rts = true; > 701 } > 702 > 703 info->control.use_rts = txrc.rts; > > or drivers/net/wireless/ath/ath9k/xmit.c > > 1238 /* > 1239 * Handle RTS threshold for unaggregated HT frames. > 1240 */ > 1241 if (bf_isampdu(bf) && !bf_isaggr(bf) && > 1242 (rates[i].flags & IEEE80211_TX_RC_MCS) && > 1243 unlikely(rts_thresh != (u32) -1)) { > 1244 if (!rts_thresh || (len > rts_thresh)) > 1245 rts = true; > 1246 } > > The only place setting IEEE80211_TX_RC_USE_RTS_CTS unconditionally > was found in net/mac80211/rc80211_minstrel_ht.c. > > Fix this by propagating the calculated use_rts value to the > minstrel_ht_set_rate() function and evaluate it accordingly > before setting IEEE80211_TX_RC_USE_RTS_CTS. Despite my believe after code review the use_rts value is only calculated after hitting the minstrel_ht code, so my preferred alternative solution would be to NOT requesting RTS/CTS from the minstrel_ht code (for the fallback rates case)...., updated patch will follow... Regards, Peter > > Signed-off-by: Peter Seiderer <ps.report@gmx.net> > --- > net/mac80211/rc80211_minstrel_ht.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c > index 72b44d4c42d0..f52edef443fa 100644 > --- a/net/mac80211/rc80211_minstrel_ht.c > +++ b/net/mac80211/rc80211_minstrel_ht.c > @@ -276,7 +276,8 @@ static const u8 minstrel_sample_seq[] = { > }; > > static void > -minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi); > +minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, > + bool use_rts); > > /* > * Some VHT MCSes are invalid (when Ndbps / Nes is not an integer) > @@ -1254,7 +1255,7 @@ minstrel_ht_tx_status(void *priv, struct ieee80211_supported_band *sband, > } > > if (update) > - minstrel_ht_update_rates(mp, mi); > + minstrel_ht_update_rates(mp, mi, info->control.use_rts); > } > > static void > @@ -1319,7 +1320,8 @@ minstrel_calc_retransmit(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, > > static void > minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, > - struct ieee80211_sta_rates *ratetbl, int offset, int index) > + struct ieee80211_sta_rates *ratetbl, int offset, int index, > + bool use_rts) > { > int group_idx = MI_RATE_GROUP(index); > const struct mcs_group *group = &minstrel_mcs_groups[group_idx]; > @@ -1357,7 +1359,7 @@ minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, > * - if station is in dynamic SMPS (and streams > 1) > * - for fallback rates, to increase chances of getting through > */ > - if (offset > 0 || > + if ((offset > 0 && use_rts) || > (mi->sta->smps_mode == IEEE80211_SMPS_DYNAMIC && > group->streams > 1)) { > ratetbl->rate[offset].count = ratetbl->rate[offset].count_rts; > @@ -1426,7 +1428,8 @@ minstrel_ht_get_max_amsdu_len(struct minstrel_ht_sta *mi) > } > > static void > -minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi) > +minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, > + bool use_rts) > { > struct ieee80211_sta_rates *rates; > int i = 0; > @@ -1436,15 +1439,15 @@ minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi) > return; > > /* Start with max_tp_rate[0] */ > - minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_tp_rate[0]); > + minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_tp_rate[0], use_rts); > > if (mp->hw->max_rates >= 3) { > /* At least 3 tx rates supported, use max_tp_rate[1] next */ > - minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_tp_rate[1]); > + minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_tp_rate[1], use_rts); > } > > if (mp->hw->max_rates >= 2) { > - minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_prob_rate); > + minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_prob_rate, use_rts); > } > > mi->sta->max_rc_amsdu_len = minstrel_ht_get_max_amsdu_len(mi); > @@ -1705,7 +1708,7 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband, > > /* create an initial rate table with the lowest supported rates */ > minstrel_ht_update_stats(mp, mi); > - minstrel_ht_update_rates(mp, mi); > + minstrel_ht_update_rates(mp, mi, false); > } > > static void
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c index 72b44d4c42d0..f52edef443fa 100644 --- a/net/mac80211/rc80211_minstrel_ht.c +++ b/net/mac80211/rc80211_minstrel_ht.c @@ -276,7 +276,8 @@ static const u8 minstrel_sample_seq[] = { }; static void -minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi); +minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, + bool use_rts); /* * Some VHT MCSes are invalid (when Ndbps / Nes is not an integer) @@ -1254,7 +1255,7 @@ minstrel_ht_tx_status(void *priv, struct ieee80211_supported_band *sband, } if (update) - minstrel_ht_update_rates(mp, mi); + minstrel_ht_update_rates(mp, mi, info->control.use_rts); } static void @@ -1319,7 +1320,8 @@ minstrel_calc_retransmit(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, static void minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, - struct ieee80211_sta_rates *ratetbl, int offset, int index) + struct ieee80211_sta_rates *ratetbl, int offset, int index, + bool use_rts) { int group_idx = MI_RATE_GROUP(index); const struct mcs_group *group = &minstrel_mcs_groups[group_idx]; @@ -1357,7 +1359,7 @@ minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, * - if station is in dynamic SMPS (and streams > 1) * - for fallback rates, to increase chances of getting through */ - if (offset > 0 || + if ((offset > 0 && use_rts) || (mi->sta->smps_mode == IEEE80211_SMPS_DYNAMIC && group->streams > 1)) { ratetbl->rate[offset].count = ratetbl->rate[offset].count_rts; @@ -1426,7 +1428,8 @@ minstrel_ht_get_max_amsdu_len(struct minstrel_ht_sta *mi) } static void -minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi) +minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, + bool use_rts) { struct ieee80211_sta_rates *rates; int i = 0; @@ -1436,15 +1439,15 @@ minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi) return; /* Start with max_tp_rate[0] */ - minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_tp_rate[0]); + minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_tp_rate[0], use_rts); if (mp->hw->max_rates >= 3) { /* At least 3 tx rates supported, use max_tp_rate[1] next */ - minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_tp_rate[1]); + minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_tp_rate[1], use_rts); } if (mp->hw->max_rates >= 2) { - minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_prob_rate); + minstrel_ht_set_rate(mp, mi, rates, i++, mi->max_prob_rate, use_rts); } mi->sta->max_rc_amsdu_len = minstrel_ht_get_max_amsdu_len(mi); @@ -1705,7 +1708,7 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband, /* create an initial rate table with the lowest supported rates */ minstrel_ht_update_stats(mp, mi); - minstrel_ht_update_rates(mp, mi); + minstrel_ht_update_rates(mp, mi, false); } static void
Despite the 'RTS thr:off' setting a wireshark trace of IBSS traffic with HT40 mode enabled between two ath9k cards revealed some RTS/CTS traffic. Debug and code analysis showed that most places setting IEEE80211_TX_RC_USE_RTS_CTS respect the RTS strategy by evaluating rts_threshold, e.g. net/mac80211/tx.c: 698 /* set up RTS protection if desired */ 699 if (len > tx->local->hw.wiphy->rts_threshold) { 700 txrc.rts = true; 701 } 702 703 info->control.use_rts = txrc.rts; or drivers/net/wireless/ath/ath9k/xmit.c 1238 /* 1239 * Handle RTS threshold for unaggregated HT frames. 1240 */ 1241 if (bf_isampdu(bf) && !bf_isaggr(bf) && 1242 (rates[i].flags & IEEE80211_TX_RC_MCS) && 1243 unlikely(rts_thresh != (u32) -1)) { 1244 if (!rts_thresh || (len > rts_thresh)) 1245 rts = true; 1246 } The only place setting IEEE80211_TX_RC_USE_RTS_CTS unconditionally was found in net/mac80211/rc80211_minstrel_ht.c. Fix this by propagating the calculated use_rts value to the minstrel_ht_set_rate() function and evaluate it accordingly before setting IEEE80211_TX_RC_USE_RTS_CTS. Signed-off-by: Peter Seiderer <ps.report@gmx.net> --- net/mac80211/rc80211_minstrel_ht.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)