| Message ID | 20211215215042.637-1-jelonek.jonas@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ath5k: switch to rate table based lookup | expand |
Jonas Jelonek <jelonek.jonas@gmail.com> writes: > Switching from legacy usage of ieee80211_get_tx_rates() lookup to direct > rate table lookup in struct ieee80211_sta->rates. > > The current rate control API allows drivers to directly get rates from > ieee80211_sta->rates. ath5k is currently one of the legacy drivers that > perform translation/merge with the internal rate table via > ieee80211_get_tx_rates provided by rate control API. > For our upcoming changes to rate control API and the implementation of > transmit power control, this patch changes the behaviour. The call to > ieee80211_get_tx_rates and subsequent calls are also avoided. ath5k now > directly reads rates from sta->rates into its internal rate table. Cause > ath5k does not rely on the rate array in SKB->CB, this is not considered > anymore except for the first entry (used for probing). > > Tested this on a PCEngines ALIX with CMP9-GP miniPCI wifi card (Atheros > AR5213A). Generated traffic between AP and multiple STAs before and > after applying the patch and simultaneously measured throughput and > captured rc_stats. Comparison resulted in same rate selection and no > performance loss between both runs. Good that you explained how you tested this. > @@ -753,8 +790,11 @@ ath5k_txbuf_setup(struct ath5k_hw *ah, struct ath5k_buf *bf, > if (dma_mapping_error(ah->dev, bf->skbaddr)) > return -ENOSPC; > > - ieee80211_get_tx_rates(info->control.vif, (control) ? control->sta : NULL, skb, bf->rates, > - ARRAY_SIZE(bf->rates)); > + if (!ath5k_merge_ratetbl((control) ? control->sta : NULL, bf, info)) { > + ieee80211_get_tx_rates(info->control.vif, > + (control) ? control->sta : NULL, skb, bf->rates, > + ARRAY_SIZE(bf->rates)); > + } I'm not really a fan of ? operator, so in the pending branch I changed this to use a normal if statement. Please double check that I didn't break anything. https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=9e7c92df342f884434ccc8635d2606c1d1b11ef9 diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c index 647b3b76495c..66d123f48085 100644 --- a/drivers/net/wireless/ath/ath5k/base.c +++ b/drivers/net/wireless/ath/ath5k/base.c @@ -774,6 +774,7 @@ ath5k_txbuf_setup(struct ath5k_hw *ah, struct ath5k_buf *bf, struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); unsigned int pktlen, flags, keyidx = AR5K_TXKEYIX_INVALID; struct ieee80211_rate *rate; + struct ieee80211_sta *sta; unsigned int mrr_rate[3], mrr_tries[3]; int i, ret; u16 hw_rate; @@ -790,10 +791,15 @@ ath5k_txbuf_setup(struct ath5k_hw *ah, struct ath5k_buf *bf, if (dma_mapping_error(ah->dev, bf->skbaddr)) return -ENOSPC; - if (!ath5k_merge_ratetbl((control) ? control->sta : NULL, bf, info)) { + if (control) + sta = control->sta; + else + sta = NULL; + + if (!ath5k_merge_ratetbl(sta, bf, info)) { ieee80211_get_tx_rates(info->control.vif, - (control) ? control->sta : NULL, skb, bf->rates, - ARRAY_SIZE(bf->rates)); + sta, skb, bf->rates, + ARRAY_SIZE(bf->rates)); } rate = ath5k_get_rate(ah->hw, info, bf, 0);
Thanks for your review! Kalle Valo <kvalo@kernel.org> writes: > I'm not really a fan of ? operator, so in the pending branch I changed > this to use a normal if statement. Please double check that I didn't > break anything. > > https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=9e7c92df342f884434ccc8635d2606c1d1b11ef9 Checked this just a few moments ago, still works fine.
Jonas Jelonek <jelonek.jonas@gmail.com> wrote: > Switching from legacy usage of ieee80211_get_tx_rates() lookup to direct > rate table lookup in struct ieee80211_sta->rates. > > The current rate control API allows drivers to directly get rates from > ieee80211_sta->rates. ath5k is currently one of the legacy drivers that > perform translation/merge with the internal rate table via > ieee80211_get_tx_rates provided by rate control API. > For our upcoming changes to rate control API and the implementation of > transmit power control, this patch changes the behaviour. The call to > ieee80211_get_tx_rates and subsequent calls are also avoided. ath5k now > directly reads rates from sta->rates into its internal rate table. Cause > ath5k does not rely on the rate array in SKB->CB, this is not considered > anymore except for the first entry (used for probing). > > Tested this on a PCEngines ALIX with CMP9-GP miniPCI wifi card (Atheros > AR5213A). Generated traffic between AP and multiple STAs before and > after applying the patch and simultaneously measured throughput and > captured rc_stats. Comparison resulted in same rate selection and no > performance loss between both runs. > > Co-developed-by: Thomas Huehn <thomas.huehn@hs-nordhausen.de> > Signed-off-by: Thomas Huehn <thomas.huehn@hs-nordhausen.de> > Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> Patch applied to ath-next branch of ath.git, thanks. a5d862da9105 ath5k: switch to rate table based lookup
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c index cef17f33c69e..647b3b76495c 100644 --- a/drivers/net/wireless/ath/ath5k/base.c +++ b/drivers/net/wireless/ath/ath5k/base.c @@ -727,6 +727,43 @@ ath5k_get_rate_hw_value(const struct ieee80211_hw *hw, return hw_rate; } +static bool ath5k_merge_ratetbl(struct ieee80211_sta *sta, + struct ath5k_buf *bf, + struct ieee80211_tx_info *tx_info) +{ + struct ieee80211_sta_rates *ratetbl; + u8 i; + + if (!sta) + return false; + + ratetbl = rcu_dereference(sta->rates); + if (!ratetbl) + return false; + + if (tx_info->control.rates[0].idx < 0 || + tx_info->control.rates[0].count == 0) + { + i = 0; + } else { + bf->rates[0] = tx_info->control.rates[0]; + i = 1; + } + + for ( ; i < IEEE80211_TX_MAX_RATES; i++) { + bf->rates[i].idx = ratetbl->rate[i].idx; + bf->rates[i].flags = ratetbl->rate[i].flags; + if (tx_info->control.use_rts) + bf->rates[i].count = ratetbl->rate[i].count_rts; + else if (tx_info->control.use_cts_prot) + bf->rates[i].count = ratetbl->rate[i].count_cts; + else + bf->rates[i].count = ratetbl->rate[i].count; + } + + return true; +} + static int ath5k_txbuf_setup(struct ath5k_hw *ah, struct ath5k_buf *bf, struct ath5k_txq *txq, int padsize, @@ -753,8 +790,11 @@ ath5k_txbuf_setup(struct ath5k_hw *ah, struct ath5k_buf *bf, if (dma_mapping_error(ah->dev, bf->skbaddr)) return -ENOSPC; - ieee80211_get_tx_rates(info->control.vif, (control) ? control->sta : NULL, skb, bf->rates, - ARRAY_SIZE(bf->rates)); + if (!ath5k_merge_ratetbl((control) ? control->sta : NULL, bf, info)) { + ieee80211_get_tx_rates(info->control.vif, + (control) ? control->sta : NULL, skb, bf->rates, + ARRAY_SIZE(bf->rates)); + } rate = ath5k_get_rate(ah->hw, info, bf, 0);