| Message ID | 20211116212828.27613-1-ps.report@gmx.net (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [RFC,v2] mac80211: minstrel_ht: do not set RTS/CTS flag for fallback rates | expand |
On 2021-11-16 22:28, Peter Seiderer 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. > > As the use_rts value is only calculated after hitting the minstrel_ht code > preferre to not set IEEE80211_TX_RC_USE_RTS_CTS (and overruling the > RTS threshold setting) for the fallback rates case. The idea behind the this part of minstrel_ht code is to avoid the overhead of RTS/CTS for transmissions using the primary rate and to increase the reliability of retransmissions by adding it for fallback rates. This is completely unrelated to the RTS threshold. If you don't want this behavior, I'm fine with adding a way to explicitly disable it. However, I do think leaving it on by default makes sense. - Felix
Hello Felix, On Fri, 26 Nov 2021 13:47:07 +0100, Felix Fietkau <nbd@nbd.name> wrote: > On 2021-11-16 22:28, Peter Seiderer 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. > > > > As the use_rts value is only calculated after hitting the minstrel_ht code > > preferre to not set IEEE80211_TX_RC_USE_RTS_CTS (and overruling the > > RTS threshold setting) for the fallback rates case. > The idea behind the this part of minstrel_ht code is to avoid the > overhead of RTS/CTS for transmissions using the primary rate and to > increase the reliability of retransmissions by adding it for fallback > rates. This is completely unrelated to the RTS threshold. How does it avoid RTS/CTS (if it is set independent by RTS threshold evaluation mac80211 and/or hardware driver)? > > If you don't want this behavior, I'm fine with adding a way to > explicitly disable it. However, I do think leaving it on by default > makes sense. I expected this (as otherwise the flag setting would not be there) ;-) Any hint how to implement an additional RTS/CTS on/off feature despite the RTS threshold one for use explicit by minstrel_ht? Configure option, module option, ...? Regards, Peter > > - Felix
On Fri, 2021-11-26 at 15:25 +0100, Peter Seiderer wrote: > > > > > If you don't want this behavior, I'm fine with adding a way to > > explicitly disable it. However, I do think leaving it on by default > > makes sense. > > I expected this (as otherwise the flag setting would not be there) ;-) > To be fair, that setting (RTS threshold) has been there for 20 years or more, and comes from a much simpler time when the reasoning for RTS/CTS was mostly about hidden stations, not about protecting the transmissions from older clients that don't understand the newer PHY protocols, etc. johannes
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c index 72b44d4c42d0..f151acbe7bf5 100644 --- a/net/mac80211/rc80211_minstrel_ht.c +++ b/net/mac80211/rc80211_minstrel_ht.c @@ -1355,11 +1355,9 @@ minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi, /* enable RTS/CTS if needed: * - if station is in dynamic SMPS (and streams > 1) - * - for fallback rates, to increase chances of getting through */ - if (offset > 0 || - (mi->sta->smps_mode == IEEE80211_SMPS_DYNAMIC && - group->streams > 1)) { + if (mi->sta->smps_mode == IEEE80211_SMPS_DYNAMIC && + group->streams > 1) { ratetbl->rate[offset].count = ratetbl->rate[offset].count_rts; flags |= IEEE80211_TX_RC_USE_RTS_CTS; }
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. As the use_rts value is only calculated after hitting the minstrel_ht code preferre to not set IEEE80211_TX_RC_USE_RTS_CTS (and overruling the RTS threshold setting) for the fallback rates case. Signed-off-by: Peter Seiderer <ps.report@gmx.net> --- Changes v1 -> v2: - change from 'respect RTS threshold setting' to 'do not set RTS/CTS flag for fallback rates' (see commit message for reasoning) --- net/mac80211/rc80211_minstrel_ht.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)