2014-07-07 09:31:51

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 00/10] add support for ack timeout estimation in ath9k driver

This patchset adds support for estimation of the ack timeout (dynack) in ath9k
driver. Ath9k dynack computes the ack timeout based on ack frame RX timestamp,
TX frame timestamp and frame duration.

Ath9k dynack has been tested in indoor environment using AR9223 chipset
(running 3.15.3 kernel) configured as AP and using AR9280 chipset
(running 3.16.0-rc3) configured as STA. Ath9k dynack needs to be tested on a
long distance link in outdoor environment.

Lorenzo Bianconi (10):
ath9k: export methods related to ack timeout estimation
ath9k: add duration field to ath_tx_status
ath9k: add dynamic ack timeout estimation
ath9k: add config for (en|dis)abling ack timeout estimation
ath9k: do not overwrite ack timeout estimation
ath9k: add sampling methods for (tx|rx) timestamp
ath9k: enable control frame reception
ath9k: add debugfs support for dynack
ath9k: disable dynack algorithm when coverage class is set
ath9k: add ath_node linked list

drivers/net/wireless/ath/ath.h | 2 +
drivers/net/wireless/ath/ath9k/Kconfig | 7 +
drivers/net/wireless/ath/ath9k/Makefile | 3 +
drivers/net/wireless/ath/ath9k/ar9002_mac.c | 7 +
drivers/net/wireless/ath/ath9k/ar9003_mac.c | 9 +
drivers/net/wireless/ath/ath9k/ath9k.h | 3 +
drivers/net/wireless/ath/ath9k/debug.c | 82 ++++++++
drivers/net/wireless/ath/ath9k/dynack.c | 293 ++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath9k/dynack.h | 81 ++++++++
drivers/net/wireless/ath/ath9k/hw.c | 16 +-
drivers/net/wireless/ath/ath9k/hw.h | 7 +
drivers/net/wireless/ath/ath9k/mac.h | 1 +
drivers/net/wireless/ath/ath9k/main.c | 11 ++
drivers/net/wireless/ath/ath9k/recv.c | 7 +-
drivers/net/wireless/ath/ath9k/xmit.c | 5 +
15 files changed, 530 insertions(+), 4 deletions(-)
create mode 100644 drivers/net/wireless/ath/ath9k/dynack.c
create mode 100644 drivers/net/wireless/ath/ath9k/dynack.h

--
1.9.1



2014-07-07 09:31:57

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 09/10] ath9k: disable dynack algorithm when coverage class is set

Disable ack timeout estimation algorithm if the coverage class has been
configured

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/main.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e6ac8d2..5d8af08 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1969,6 +1969,7 @@ static void ath9k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)

mutex_lock(&sc->mutex);
ah->coverage_class = coverage_class;
+ ah->dynack.fix_to = true;

ath9k_ps_wakeup(sc);
ath9k_hw_init_global_settings(ah);
--
1.9.1


2014-07-09 15:29:37

by Sujith Manoharan

[permalink] [raw]
Subject: Re: [RFC 03/10] ath9k: add dynamic ack timeout estimation

Lorenzo Bianconi wrote:
> +/*
> + * Copyright 2014, Lorenzo Bianconi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */

I have not reviewed the patches, but the ath9k driver is licensed
under ISC. Introducing a file with GPLv2 license is bound to cause
complications and I think might require legal clearance from a
QCA employee maintaining ath9k.

Right now, I am not sure if there is an official maintainer for
ath9k, but is there any specific reason why you have chosen
GPLv2 ?

Sujith

2014-07-07 09:31:51

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 02/10] ath9k: add duration field to ath_tx_status

Add duration field to ath_tx_status in order to report frame duration for each
entry in multi-retry chain. These fields will be used in dynamic ack timeout
processing

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/ar9002_mac.c | 7 +++++++
drivers/net/wireless/ath/ath9k/ar9003_mac.c | 9 +++++++++
drivers/net/wireless/ath/ath9k/mac.h | 1 +
3 files changed, 17 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/ar9002_mac.c b/drivers/net/wireless/ath/ath9k/ar9002_mac.c
index 741b38d..b1d8d0e 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_mac.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_mac.c
@@ -377,6 +377,13 @@ static int ar9002_hw_proc_txdesc(struct ath_hw *ah, void *ds,
ts->evm1 = ads->AR_TxEVM1;
ts->evm2 = ads->AR_TxEVM2;

+ status = ACCESS_ONCE(ads->ds_ctl4);
+ ts->duration[0] = MS(status, AR_PacketDur0);
+ ts->duration[1] = MS(status, AR_PacketDur1);
+ status = ACCESS_ONCE(ads->ds_ctl5);
+ ts->duration[2] = MS(status, AR_PacketDur2);
+ ts->duration[3] = MS(status, AR_PacketDur3);
+
return 0;
}

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_mac.c b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
index 729ffbf..d261c6e 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_mac.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
@@ -351,9 +351,11 @@ static int ar9003_hw_proc_txdesc(struct ath_hw *ah, void *ds,
struct ath_tx_status *ts)
{
struct ar9003_txs *ads;
+ struct ar9003_txc *adc;
u32 status;

ads = &ah->ts_ring[ah->ts_tail];
+ adc = (struct ar9003_txc *)ads;

status = ACCESS_ONCE(ads->status8);
if ((status & AR_TxDone) == 0)
@@ -422,6 +424,13 @@ static int ar9003_hw_proc_txdesc(struct ath_hw *ah, void *ds,
ts->ts_rssi_ext1 = MS(status, AR_TxRSSIAnt11);
ts->ts_rssi_ext2 = MS(status, AR_TxRSSIAnt12);

+ status = ACCESS_ONCE(adc->ctl15);
+ ts->duration[0] = MS(status, AR_PacketDur0);
+ ts->duration[1] = MS(status, AR_PacketDur1);
+ status = ACCESS_ONCE(adc->ctl16);
+ ts->duration[2] = MS(status, AR_PacketDur2);
+ ts->duration[3] = MS(status, AR_PacketDur3);
+
memset(ads, 0, sizeof(*ads));

return 0;
diff --git a/drivers/net/wireless/ath/ath9k/mac.h b/drivers/net/wireless/ath/ath9k/mac.h
index da76867..06efbe8 100644
--- a/drivers/net/wireless/ath/ath9k/mac.h
+++ b/drivers/net/wireless/ath/ath9k/mac.h
@@ -121,6 +121,7 @@ struct ath_tx_status {
u32 evm0;
u32 evm1;
u32 evm2;
+ u32 duration[4];
};

struct ath_rx_status {
--
1.9.1


2014-07-07 09:31:55

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 07/10] ath9k: enable control frame reception

Enable control frame reception if dynamic ack processing is enabled

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/recv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index ad317a4..5d02467 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -398,7 +398,7 @@ u32 ath_calcrxfilter(struct ath_softc *sc)
if (sc->sc_ah->is_monitoring)
rfilt |= ATH9K_RX_FILTER_PROM;

- if (sc->rx.rxfilter & FIF_CONTROL)
+ if ((sc->rx.rxfilter & FIF_CONTROL) || !sc->sc_ah->dynack.fix_to)
rfilt |= ATH9K_RX_FILTER_CONTROL;

if ((sc->sc_ah->opmode == NL80211_IFTYPE_STATION) &&
--
1.9.1


2014-07-07 09:31:53

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 03/10] ath9k: add dynamic ack timeout estimation

Add dynamic ack timeout estimation algorithm based on ack frame RX timestamp,
TX frame timestamp and frame duration.

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath.h | 2 +
drivers/net/wireless/ath/ath9k/ath9k.h | 3 +
drivers/net/wireless/ath/ath9k/dynack.c | 293 ++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath9k/dynack.h | 81 +++++++++
drivers/net/wireless/ath/ath9k/hw.c | 2 +
drivers/net/wireless/ath/ath9k/hw.h | 3 +
6 files changed, 384 insertions(+)
create mode 100644 drivers/net/wireless/ath/ath9k/dynack.c
create mode 100644 drivers/net/wireless/ath/ath9k/dynack.h

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index fd9e530..4e51072 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -234,6 +234,7 @@ void ath_printk(const char *level, const struct ath_common *common,
* AR9462.
* @ATH_DBG_DFS: radar datection
* @ATH_DBG_WOW: Wake on Wireless
+ * @ATH_DBG_DYNACK: dynack handling
* @ATH_DBG_ANY: enable all debugging
*
* The debug level is used to control the amount and type of debugging output
@@ -261,6 +262,7 @@ enum ATH_DEBUG {
ATH_DBG_MCI = 0x00008000,
ATH_DBG_DFS = 0x00010000,
ATH_DBG_WOW = 0x00020000,
+ ATH_DBG_DYNACK = 0x00040000,
ATH_DBG_ANY = 0xffffffff
};

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 11b5e4d..65a2587 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -272,6 +272,9 @@ struct ath_node {
struct ath_rx_rate_stats rx_rate_stats;
#endif
u8 key_idx[4];
+
+ u32 ackto;
+ struct list_head list;
};

struct ath_tx_control {
diff --git a/drivers/net/wireless/ath/ath9k/dynack.c b/drivers/net/wireless/ath/ath9k/dynack.c
new file mode 100644
index 0000000..50297e7
--- /dev/null
+++ b/drivers/net/wireless/ath/ath9k/dynack.c
@@ -0,0 +1,293 @@
+/*
+ * Copyright 2014, Lorenzo Bianconi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include "ath9k.h"
+#include "hw.h"
+#include "dynack.h"
+
+#define COMPUTE_TO (5 * HZ)
+#define LATEACK_DELAY (10 * HZ)
+#define LATEACK_TO 256
+#define MAX_DELAY 300
+#define EWMA_LEVEL 75
+#define DYNACK_EWMA(old, new) \
+ (((new) * (100 - EWMA_LEVEL) + (old) * EWMA_LEVEL) / 100)
+
+/**
+ * ath_dynack_get_sifs - get sifs time based on phy used
+ * @ah: ath hw
+ * @phy: phy used
+ */
+static inline u32 ath_dynack_get_sifs(struct ath_hw *ah, int phy)
+{
+ u32 sifs = CCK_SIFS_TIME;
+
+ if (phy == WLAN_RC_PHY_OFDM) {
+ if (IS_CHAN_QUARTER_RATE(ah->curchan))
+ sifs = OFDM_SIFS_TIME_QUARTER;
+ else if (IS_CHAN_HALF_RATE(ah->curchan))
+ sifs = OFDM_SIFS_TIME_HALF;
+ else
+ sifs = OFDM_SIFS_TIME;
+ }
+ return sifs;
+}
+
+static inline bool ath_dynack_bssidmask(struct ath_hw *ah, const u8 *mac)
+{
+ int i;
+ struct ath_common *common = ath9k_hw_common(ah);
+
+ for (i = 0; i < ETH_ALEN; i++) {
+ if ((common->macaddr[i] & common->bssidmask[i]) !=
+ (mac[i] & common->bssidmask[i]))
+ return false;
+ }
+
+ return true;
+}
+
+/**
+ * ath_dynack_set_ackto - compute ack timeout based on sta timeout
+ * @ah: ath hw
+ *
+ * should be called while holding qlock
+ */
+static void ath_dynack_compute_ackto(struct ath_hw *ah)
+{
+ struct ath_node *an;
+ u32 to = 0;
+ struct ath_dynack *da = &ah->dynack;
+ struct ath_common *common = ath9k_hw_common(ah);
+
+ list_for_each_entry(an, &da->nodes, list)
+ if (an->ackto > to)
+ to = an->ackto;
+
+ if (to && da->ackto != to) {
+ u32 slottime;
+
+ slottime = (to - 3) / 2;
+ da->ackto = to;
+ ath_dbg(common, DYNACK, "ack timeout %u slottime %u\n",
+ da->ackto, slottime);
+ ath9k_hw_setslottime(ah, slottime);
+ ath9k_hw_set_ack_timeout(ah, da->ackto);
+ ath9k_hw_set_cts_timeout(ah, da->ackto);
+ }
+}
+
+/**
+ * ath_dynack_compute_to - compute ack timeout
+ * @ah: ath hw
+ *
+ * should be called while holding qlock
+ */
+static void ath_dynack_compute_to(struct ath_hw *ah)
+{
+ u32 ackto, ack_ts;
+ u8 *dst, *src;
+ struct ieee80211_sta *sta;
+ struct ath_node *an;
+ struct ts_info *st_ts;
+ struct ath_dynack *da = &ah->dynack;
+
+ rcu_read_lock();
+
+ while (da->st_rbf.h_rb != da->st_rbf.t_rb &&
+ da->ack_rbf.h_rb != da->ack_rbf.t_rb) {
+ ack_ts = da->ack_rbf.tstamp[da->ack_rbf.h_rb];
+ st_ts = &da->st_rbf.ts[da->st_rbf.h_rb];
+ dst = da->st_rbf.addr[da->st_rbf.h_rb].h_dest;
+ src = da->st_rbf.addr[da->st_rbf.h_rb].h_src;
+
+ ath_dbg(ath9k_hw_common(ah), DYNACK,
+ "ack_ts %u st_ts %u st_dur %u [%u-%u]\n",
+ ack_ts, st_ts->tstamp, st_ts->dur,
+ da->ack_rbf.h_rb, da->st_rbf.h_rb);
+
+ if (ack_ts > st_ts->tstamp + st_ts->dur) {
+ ackto = ack_ts - st_ts->tstamp - st_ts->dur;
+
+ if (ackto < MAX_DELAY) {
+ sta = ieee80211_find_sta_by_ifaddr(ah->hw, dst,
+ src);
+ if (sta) {
+ an = (struct ath_node *)sta->drv_priv;
+ an->ackto = DYNACK_EWMA((u32)ackto,
+ an->ackto);
+ ath_dbg(ath9k_hw_common(ah), DYNACK,
+ "%pM to %u\n", dst, an->ackto);
+ if (time_is_before_jiffies(da->lto)) {
+ ath_dynack_compute_ackto(ah);
+ da->lto = jiffies + COMPUTE_TO;
+ }
+ }
+ INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
+ }
+ INCR(da->st_rbf.h_rb, ATH_DYN_BUF);
+ } else {
+ INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
+ }
+ }
+
+ rcu_read_unlock();
+}
+
+/**
+ * ath_dynack_sample_tx_ts - status ts sampling method
+ * @ah: ath hw
+ * @skb: socket buffer
+ * @ts: tx status info
+ *
+ */
+void ath_dynack_sample_tx_ts(struct ath_hw *ah, struct sk_buff *skb,
+ struct ath_tx_status *ts)
+{
+ u8 ridx;
+ struct ieee80211_hdr *hdr;
+ struct ath_dynack *da = &ah->dynack;
+ struct ath_common *common = ath9k_hw_common(ah);
+ struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+
+ if ((info->flags & IEEE80211_TX_CTL_NO_ACK) || da->fix_to)
+ return;
+
+ spin_lock_bh(&da->qlock);
+
+ hdr = (struct ieee80211_hdr *)skb->data;
+
+ /* late ack */
+ if (ts->ts_status & ATH9K_TXERR_XRETRY) {
+ if (ieee80211_is_assoc_req(hdr->frame_control) ||
+ ieee80211_is_assoc_resp(hdr->frame_control)) {
+ ath_dbg(common, DYNACK, "late ack\n");
+ ath9k_hw_setslottime(ah, (LATEACK_TO - 3) / 2);
+ ath9k_hw_set_ack_timeout(ah, LATEACK_TO);
+ ath9k_hw_set_cts_timeout(ah, LATEACK_TO);
+ da->lto = jiffies + LATEACK_DELAY;
+ }
+
+ spin_unlock_bh(&da->qlock);
+ return;
+ }
+
+ ridx = ts->ts_rateindex;
+
+ da->st_rbf.ts[da->st_rbf.t_rb].tstamp = ts->ts_tstamp;
+ da->st_rbf.ts[da->st_rbf.t_rb].dur = ts->duration[ts->ts_rateindex];
+ ether_addr_copy(da->st_rbf.addr[da->st_rbf.t_rb].h_dest, hdr->addr1);
+ ether_addr_copy(da->st_rbf.addr[da->st_rbf.t_rb].h_src, hdr->addr2);
+
+ if (!(info->status.rates[ridx].flags & IEEE80211_TX_RC_MCS)) {
+ u32 phy, sifs;
+ const struct ieee80211_rate *rate;
+ struct ieee80211_tx_rate *rates = info->status.rates;
+
+ rate = &common->sbands[info->band].bitrates[rates[ridx].idx];
+ if (info->band == IEEE80211_BAND_2GHZ &&
+ !(rate->flags & IEEE80211_RATE_ERP_G))
+ phy = WLAN_RC_PHY_CCK;
+ else
+ phy = WLAN_RC_PHY_OFDM;
+
+ sifs = ath_dynack_get_sifs(ah, phy);
+ da->st_rbf.ts[da->st_rbf.t_rb].dur -= sifs;
+ }
+
+ ath_dbg(common, DYNACK, "{%pM} tx sample %u [dur %u][h %u-t %u]\n",
+ hdr->addr1, da->st_rbf.ts[da->st_rbf.t_rb].tstamp,
+ da->st_rbf.ts[da->st_rbf.t_rb].dur, da->st_rbf.h_rb,
+ (da->st_rbf.t_rb + 1) % ATH_DYN_BUF);
+
+ INCR(da->st_rbf.t_rb, ATH_DYN_BUF);
+ if (da->st_rbf.t_rb == da->st_rbf.h_rb)
+ INCR(da->st_rbf.h_rb, ATH_DYN_BUF);
+
+ ath_dynack_compute_to(ah);
+
+ spin_unlock_bh(&da->qlock);
+}
+EXPORT_SYMBOL(ath_dynack_sample_tx_ts);
+
+/**
+ * ath_dynack_sample_ack_ts - ack ts sampling method
+ * @ah: ath hw
+ * @skb: socket buffer
+ * @ts: rx timestamp
+ *
+ */
+void ath_dynack_sample_ack_ts(struct ath_hw *ah, struct sk_buff *skb,
+ u32 ts)
+{
+ struct ath_dynack *da = &ah->dynack;
+ struct ath_common *common = ath9k_hw_common(ah);
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+
+ if (!ath_dynack_bssidmask(ah, hdr->addr1) || da->fix_to)
+ return;
+
+ spin_lock_bh(&da->qlock);
+ da->ack_rbf.tstamp[da->ack_rbf.t_rb] = ts;
+
+ ath_dbg(common, DYNACK, "rx sample %u [h %u-t %u]\n",
+ da->ack_rbf.tstamp[da->ack_rbf.t_rb],
+ da->ack_rbf.h_rb, (da->ack_rbf.t_rb + 1) % ATH_DYN_BUF);
+
+ INCR(da->ack_rbf.t_rb, ATH_DYN_BUF);
+ if (da->ack_rbf.t_rb == da->ack_rbf.h_rb)
+ INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
+
+ ath_dynack_compute_to(ah);
+
+ spin_unlock_bh(&da->qlock);
+}
+EXPORT_SYMBOL(ath_dynack_sample_ack_ts);
+
+/**
+ * ath_dynack_reset - reset dynack processing
+ * @ah: ath hw
+ *
+ */
+void ath_dynack_reset(struct ath_hw *ah)
+{
+ /* ackto = slottime + sifs + air delay */
+ u32 ackto = ATH9K_SLOT_TIME_9 + 16 + 64;
+ struct ath_dynack *da = &ah->dynack;
+
+ da->lto = jiffies;
+ da->ackto = ackto;
+
+ da->st_rbf.t_rb = 0;
+ da->st_rbf.h_rb = 0;
+ da->ack_rbf.t_rb = 0;
+ da->ack_rbf.h_rb = 0;
+
+ /* init acktimeout */
+ ath9k_hw_setslottime(ah, (ackto - 3) / 2);
+ ath9k_hw_set_ack_timeout(ah, ackto);
+ ath9k_hw_set_cts_timeout(ah, ackto);
+}
+EXPORT_SYMBOL(ath_dynack_reset);
+
+/**
+ * ath_dynack_init - init dynack data structure
+ * @ah: ath hw
+ *
+ */
+void ath_dynack_init(struct ath_hw *ah)
+{
+ struct ath_dynack *da = &ah->dynack;
+
+ memset(da, 0, sizeof(struct ath_dynack));
+
+ spin_lock_init(&da->qlock);
+ INIT_LIST_HEAD(&da->nodes);
+
+ ath_dynack_reset(ah);
+}
+
diff --git a/drivers/net/wireless/ath/ath9k/dynack.h b/drivers/net/wireless/ath/ath9k/dynack.h
new file mode 100644
index 0000000..386f176
--- /dev/null
+++ b/drivers/net/wireless/ath/ath9k/dynack.h
@@ -0,0 +1,81 @@
+/*
+ * Copyright 2014, Lorenzo Bianconi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef DYNACK_H
+#define DYNACK_H
+
+#define ATH_DYN_BUF 64
+
+struct ath_hw;
+
+/**
+ * ath_dyn_rxbuf - ack frame ring buffer
+ */
+struct ath_dyn_rxbuf {
+ u16 h_rb, t_rb;
+ u32 tstamp[ATH_DYN_BUF];
+};
+
+struct ts_info {
+ u32 tstamp;
+ u32 dur;
+};
+
+struct haddr_pair {
+ u8 h_dest[ETH_ALEN];
+ u8 h_src[ETH_ALEN];
+};
+/**
+ * ath_dyn_txbuf - tx frame ring buffer
+ */
+struct ath_dyn_txbuf {
+ u16 h_rb, t_rb;
+ struct haddr_pair addr[ATH_DYN_BUF];
+ struct ts_info ts[ATH_DYN_BUF];
+};
+
+/**
+ * ath_dynack - dyn ack processing info
+ * @fix_to: use static ack timeout
+ * @ackto: current ack timeout
+ * @lto: last ack timeout computation
+ * @nodes: ath_node linked list
+ * @qlock: ts queue spinlock
+ * @ack_rbf: ack ts ring buffer
+ * @st_rbf: status ts ring buffer
+ */
+struct ath_dynack {
+ bool fix_to;
+ int ackto;
+ unsigned long lto;
+
+ struct list_head nodes;
+
+ /* protect timestamp queue access */
+ spinlock_t qlock;
+ struct ath_dyn_rxbuf ack_rbf;
+ struct ath_dyn_txbuf st_rbf;
+};
+
+#if defined(CONFIG_ATH9K_DYNACK)
+void ath_dynack_reset(struct ath_hw *ah);
+void ath_dynack_init(struct ath_hw *ah);
+void ath_dynack_sample_ack_ts(struct ath_hw *ah, struct sk_buff *skb, u32 ts);
+void ath_dynack_sample_tx_ts(struct ath_hw *ah, struct sk_buff *skb,
+ struct ath_tx_status *ts);
+#else
+static inline void ath_dynack_reset(struct ath_hw *ah) {}
+static inline void ath_dynack_init(struct ath_hw *ah) {}
+static inline void ath_dynack_sample_ack_ts(struct ath_hw *ah,
+ struct sk_buff *skb, u32 ts) {}
+static inline void ath_dynack_sample_tx_ts(struct ath_hw *ah,
+ struct sk_buff *skb,
+ struct ath_tx_status *ts) {}
+#endif
+
+#endif /* DYNACK_H */
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index e88896c..9a6b113 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -647,6 +647,8 @@ int ath9k_hw_init(struct ath_hw *ah)
return ret;
}

+ ath_dynack_init(ah);
+
return 0;
}
EXPORT_SYMBOL(ath9k_hw_init);
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index f36d971..b9eef33 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -29,6 +29,7 @@
#include "reg.h"
#include "phy.h"
#include "btcoex.h"
+#include "dynack.h"

#include "../regd.h"

@@ -924,6 +925,8 @@ struct ath_hw {
int (*external_reset)(void);

const struct firmware *eeprom_blob;
+
+ struct ath_dynack dynack;
};

struct ath_bus_ops {
--
1.9.1


2014-07-11 22:16:21

by Philippe DUCHEIN

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC 00/10] add support for ack timeout estimation in ath9k driver

Hi Lorenzo,
i upgraded last night a point to pint of 9kms with dynack (it?s bullets 5M)
It works perfectly
I have better bandwidth ! yes, about 10% better!
next week, i?ll upgrade a point to multipoint with 20 STA. STA are around 6kms from the base station. Base station is a rocket 5M and CPE are Ubnt nanobridge.

Regards,
Philippe


Le 7 juil. 2014 ? 11:31, Lorenzo Bianconi <[email protected]> a ?crit :

> This patchset adds support for estimation of the ack timeout (dynack) in ath9k
> driver. Ath9k dynack computes the ack timeout based on ack frame RX timestamp,
> TX frame timestamp and frame duration.
>
> Ath9k dynack has been tested in indoor environment using AR9223 chipset
> (running 3.15.3 kernel) configured as AP and using AR9280 chipset
> (running 3.16.0-rc3) configured as STA. Ath9k dynack needs to be tested on a
> long distance link in outdoor environment.
>
> Lorenzo Bianconi (10):
> ath9k: export methods related to ack timeout estimation
> ath9k: add duration field to ath_tx_status
> ath9k: add dynamic ack timeout estimation
> ath9k: add config for (en|dis)abling ack timeout estimation
> ath9k: do not overwrite ack timeout estimation
> ath9k: add sampling methods for (tx|rx) timestamp
> ath9k: enable control frame reception
> ath9k: add debugfs support for dynack
> ath9k: disable dynack algorithm when coverage class is set
> ath9k: add ath_node linked list
>
> drivers/net/wireless/ath/ath.h | 2 +
> drivers/net/wireless/ath/ath9k/Kconfig | 7 +
> drivers/net/wireless/ath/ath9k/Makefile | 3 +
> drivers/net/wireless/ath/ath9k/ar9002_mac.c | 7 +
> drivers/net/wireless/ath/ath9k/ar9003_mac.c | 9 +
> drivers/net/wireless/ath/ath9k/ath9k.h | 3 +
> drivers/net/wireless/ath/ath9k/debug.c | 82 ++++++++
> drivers/net/wireless/ath/ath9k/dynack.c | 293 ++++++++++++++++++++++++++++
> drivers/net/wireless/ath/ath9k/dynack.h | 81 ++++++++
> drivers/net/wireless/ath/ath9k/hw.c | 16 +-
> drivers/net/wireless/ath/ath9k/hw.h | 7 +
> drivers/net/wireless/ath/ath9k/mac.h | 1 +
> drivers/net/wireless/ath/ath9k/main.c | 11 ++
> drivers/net/wireless/ath/ath9k/recv.c | 7 +-
> drivers/net/wireless/ath/ath9k/xmit.c | 5 +
> 15 files changed, 530 insertions(+), 4 deletions(-)
> create mode 100644 drivers/net/wireless/ath/ath9k/dynack.c
> create mode 100644 drivers/net/wireless/ath/ath9k/dynack.h
>
> --
> 1.9.1
>
> _______________________________________________
> ath9k-devel mailing list
> [email protected]
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel


2014-07-07 09:31:55

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 08/10] ath9k: add debugfs support for dynack

Add ack_to entry to debugfs in order to dump current ack timeout value.
Add dynack entry to enable/disable ack timeout estimation algorithm:

echo (1|0) > /sys/kernel/debug/ieee80211/phy0/ath9k/dynack

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/debug.c | 82 ++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index ce073e9..2f8354f 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1169,6 +1169,81 @@ static const struct file_operations fops_btcoex = {
};
#endif

+#ifdef CONFIG_ATH9K_DYNACK
+/* enable/disable dynack processing */
+static ssize_t read_file_dynack(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ struct ath_hw *ah = sc->sc_ah;
+ char buf[32];
+ unsigned int len;
+
+ len = sprintf(buf, "%u\n", !ah->dynack.fix_to);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_file_dynack(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ struct ath_hw *ah = sc->sc_ah;
+ char buf[32];
+ unsigned long dynack;
+ ssize_t len;
+ u32 rfilt;
+
+ len = min(count, sizeof(buf) - 1);
+ if (copy_from_user(buf, user_buf, len))
+ return -EFAULT;
+
+ buf[len] = '\0';
+ if (kstrtoul(buf, 0, &dynack))
+ return -EINVAL;
+
+ ah->dynack.fix_to = !dynack;
+
+ rfilt = ath_calcrxfilter(sc);
+ ath9k_hw_setrxfilter(ah, rfilt);
+
+ if (ah->dynack.fix_to)
+ ath9k_hw_init_global_settings(ah);
+ else
+ ath_dynack_reset(ah);
+
+ return count;
+}
+
+static const struct file_operations fops_dynack = {
+ .read = read_file_dynack,
+ .write = write_file_dynack,
+ .open = simple_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
+static ssize_t read_file_ackto(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ struct ath_hw *ah = sc->sc_ah;
+ char buf[32];
+ unsigned int len;
+
+ len = sprintf(buf, "%u %c\n", ah->dynack.ackto,
+ (ah->dynack.fix_to) ? 'S' : 'A');
+
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static const struct file_operations fops_ackto = {
+ .read = read_file_ackto,
+ .open = simple_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+#endif
+
/* Ethtool support for get-stats */

#define AMKSTR(nm) #nm "_BE", #nm "_BK", #nm "_VI", #nm "_VO"
@@ -1374,5 +1449,12 @@ int ath9k_init_debug(struct ath_hw *ah)
&fops_btcoex);
#endif

+#ifdef CONFIG_ATH9K_DYNACK
+ debugfs_create_file("dynack", S_IRUSR | S_IWUSR,
+ sc->debug.debugfs_phy, sc, &fops_dynack);
+ debugfs_create_file("ack_to", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
+ sc, &fops_ackto);
+#endif
+
return 0;
}
--
1.9.1


2014-07-07 09:31:57

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 10/10] ath9k: add ath_node linked list

Add neighbor linked list used by estimation algorithm to compute maximum sta
ack timeout

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/main.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 5d8af08..ee07776 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -350,6 +350,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta,
struct ieee80211_vif *vif)
{
struct ath_node *an;
+ struct ath_dynack *da = &sc->sc_ah->dynack;
an = (struct ath_node *)sta->drv_priv;

an->sc = sc;
@@ -358,12 +359,21 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta,
memset(&an->key_idx, 0, sizeof(an->key_idx));

ath_tx_node_init(sc, an);
+
+ spin_lock(&da->qlock);
+ list_add_tail(&an->list, &da->nodes);
+ spin_unlock(&da->qlock);
}

static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta)
{
struct ath_node *an = (struct ath_node *)sta->drv_priv;
+ struct ath_dynack *da = &sc->sc_ah->dynack;
ath_tx_node_cleanup(sc, an);
+
+ spin_lock(&da->qlock);
+ list_del(&an->list);
+ spin_unlock(&da->qlock);
}

void ath9k_tasklet(unsigned long data)
--
1.9.1


2014-07-07 18:36:47

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC 03/10] ath9k: add dynamic ack timeout estimation

> Hi Lorenzo,
>

Hi Thomas,

> My comments are inline.
>
>
>> Add dynamic ack timeout estimation algorithm based on ack frame RX timestamp,
>> TX frame timestamp and frame duration.
>>
>> Signed-off-by: Lorenzo Bianconi <[email protected]>
>> ---
>> drivers/net/wireless/ath/ath.h | 2 +
>> drivers/net/wireless/ath/ath9k/ath9k.h | 3 +
>> drivers/net/wireless/ath/ath9k/dynack.c | 293 ++++++++++++++++++++++++++++++++
>> drivers/net/wireless/ath/ath9k/dynack.h | 81 +++++++++
>> drivers/net/wireless/ath/ath9k/hw.c | 2 +
>> drivers/net/wireless/ath/ath9k/hw.h | 3 +
>> 6 files changed, 384 insertions(+)
>> create mode 100644 drivers/net/wireless/ath/ath9k/dynack.c
>> create mode 100644 drivers/net/wireless/ath/ath9k/dynack.h
>>
>> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
>> index fd9e530..4e51072 100644
>> --- a/drivers/net/wireless/ath/ath.h
>> +++ b/drivers/net/wireless/ath/ath.h
>> @@ -234,6 +234,7 @@ void ath_printk(const char *level, const struct ath_common *common,
>> * AR9462.
>> * @ATH_DBG_DFS: radar datection
>> * @ATH_DBG_WOW: Wake on Wireless
>> + * @ATH_DBG_DYNACK: dynack handling
>> * @ATH_DBG_ANY: enable all debugging
>> *
>> * The debug level is used to control the amount and type of debugging output
>> @@ -261,6 +262,7 @@ enum ATH_DEBUG {
>> ATH_DBG_MCI = 0x00008000,
>> ATH_DBG_DFS = 0x00010000,
>> ATH_DBG_WOW = 0x00020000,
>> + ATH_DBG_DYNACK = 0x00040000,
>> ATH_DBG_ANY = 0xffffffff
>> };
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> index 11b5e4d..65a2587 100644
>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> @@ -272,6 +272,9 @@ struct ath_node {
>> struct ath_rx_rate_stats rx_rate_stats;
>> #endif
>> u8 key_idx[4];
>> +
>> + u32 ackto;
>> + struct list_head list;
>> };
>>
>> struct ath_tx_control {
>> diff --git a/drivers/net/wireless/ath/ath9k/dynack.c b/drivers/net/wireless/ath/ath9k/dynack.c
>> new file mode 100644
>> index 0000000..50297e7
>> --- /dev/null
>> +++ b/drivers/net/wireless/ath/ath9k/dynack.c
>> @@ -0,0 +1,293 @@
>> +/*
>> + * Copyright 2014, Lorenzo Bianconi <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +#include "ath9k.h"
>> +#include "hw.h"
>> +#include "dynack.h"
>> +
>> +#define COMPUTE_TO (5 * HZ)
>> +#define LATEACK_DELAY (10 * HZ)
>> +#define LATEACK_TO 256
>> +#define MAX_DELAY 300
>> +#define EWMA_LEVEL 75
>> +#define DYNACK_EWMA(old, new) \
>> + (((new) * (100 - EWMA_LEVEL) + (old) * EWMA_LEVEL) / 100)
>> +
>
> You could change this EWMA calculation as we did it in Minstrel to use powers of two as the calculation speed increased.
> This would change :
> - EWMA_DIV from 100 to 2^7
> - EWMA_LEVEL from 75 (/EWMA_DIV=100) to 2^6 + 2^5 (/EWMA_DIV=128)
>
> Note that this changes EWMA_DIV - EWMA_LEVEL from 25 to 2^5 and keeps
> EWMA_LEVEL / EWMA_DIV == 0.75.
>
> Something like this:
>
> +#define EWMA_LEVEL 96 /* ewma weighting factor [/EWMA_DIV] */
> +#define EWMA_DIV 128
>
> static inline int
> dynack_ewma(int old, int new, int weight)
> {
> + return (new * (EWMA_DIV - weight) + old * weight) / EWMA_DIV;
> }
>

Ack

>
>> +/**
>> + * ath_dynack_get_sifs - get sifs time based on phy used
>> + * @ah: ath hw
>> + * @phy: phy used
>> + */
>> +static inline u32 ath_dynack_get_sifs(struct ath_hw *ah, int phy)
>> +{
>> + u32 sifs = CCK_SIFS_TIME;
>> +
>> + if (phy == WLAN_RC_PHY_OFDM) {
>> + if (IS_CHAN_QUARTER_RATE(ah->curchan))
>> + sifs = OFDM_SIFS_TIME_QUARTER;
>> + else if (IS_CHAN_HALF_RATE(ah->curchan))
>> + sifs = OFDM_SIFS_TIME_HALF;
>> + else
>> + sifs = OFDM_SIFS_TIME;
>> + }
>> + return sifs;
>> +}
>> +
>> +static inline bool ath_dynack_bssidmask(struct ath_hw *ah, const u8 *mac)
>> +{
>> + int i;
>> + struct ath_common *common = ath9k_hw_common(ah);
>> +
>> + for (i = 0; i < ETH_ALEN; i++) {
>> + if ((common->macaddr[i] & common->bssidmask[i]) !=
>> + (mac[i] & common->bssidmask[i]))
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +/**
>> + * ath_dynack_set_ackto - compute ack timeout based on sta timeout
>> + * @ah: ath hw
>> + *
>> + * should be called while holding qlock
>> + */
>> +static void ath_dynack_compute_ackto(struct ath_hw *ah)
>
> The function name does not match the name in the comment above (ath_dynack_set_ackto != ath_dynack_compute_ackto).
>

Sorry, just a typo

>> +{
>> + struct ath_node *an;
>> + u32 to = 0;
>> + struct ath_dynack *da = &ah->dynack;
>> + struct ath_common *common = ath9k_hw_common(ah);
>> +
>
>> + list_for_each_entry(an, &da->nodes, list)
>> + if (an->ackto > to)
>> + to = an->ackto;
>> +
>
> This list parsing would probably need rcu protection like:
>
> rcu_read_lock();
> list_for_each_entry(an, &da->nodes, list)
> if (an->ackto > to)
> to = an->ackto;
> rcu_read_unlock();
>
> I am not sure that you need to call the entire function with spin_lock as you do it now.
>

ath_dynack_compute_ackto() is called while holding qlock, so need to
use RCU locking

>> + if (to && da->ackto != to) {
>> + u32 slottime;
>> +
>> + slottime = (to - 3) / 2;
>
> Should the case to < 3 be covered or is it safe to have potentially slottime = 0 ?

slottime should be at least 9us, it cannot be 0

>
>> + da->ackto = to;
>> + ath_dbg(common, DYNACK, "ack timeout %u slottime %u\n",
>> + da->ackto, slottime);
>> + ath9k_hw_setslottime(ah, slottime);
>> + ath9k_hw_set_ack_timeout(ah, da->ackto);
>> + ath9k_hw_set_cts_timeout(ah, da->ackto);
>> + }
>> +}
>> +
>> +/**
>> + * ath_dynack_compute_to - compute ack timeout
>> + * @ah: ath hw
>> + *
>> + * should be called while holding qlock
>> + */
>> +static void ath_dynack_compute_to(struct ath_hw *ah)
>> +{
>> + u32 ackto, ack_ts;
>> + u8 *dst, *src;
>> + struct ieee80211_sta *sta;
>> + struct ath_node *an;
>> + struct ts_info *st_ts;
>> + struct ath_dynack *da = &ah->dynack;
>> +
>> + rcu_read_lock();
>> +
>> + while (da->st_rbf.h_rb != da->st_rbf.t_rb &&
>> + da->ack_rbf.h_rb != da->ack_rbf.t_rb) {
>> + ack_ts = da->ack_rbf.tstamp[da->ack_rbf.h_rb];
>> + st_ts = &da->st_rbf.ts[da->st_rbf.h_rb];
>> + dst = da->st_rbf.addr[da->st_rbf.h_rb].h_dest;
>> + src = da->st_rbf.addr[da->st_rbf.h_rb].h_src;
>> +
>> + ath_dbg(ath9k_hw_common(ah), DYNACK,
>> + "ack_ts %u st_ts %u st_dur %u [%u-%u]\n",
>> + ack_ts, st_ts->tstamp, st_ts->dur,
>> + da->ack_rbf.h_rb, da->st_rbf.h_rb);
>> +
>> + if (ack_ts > st_ts->tstamp + st_ts->dur) {
>> + ackto = ack_ts - st_ts->tstamp - st_ts->dur;
>> +
>> + if (ackto < MAX_DELAY) {
>> + sta = ieee80211_find_sta_by_ifaddr(ah->hw, dst,
>> + src);
>> + if (sta) {
>> + an = (struct ath_node *)sta->drv_priv;
>> + an->ackto = DYNACK_EWMA((u32)ackto,
>> + an->ackto);
>> + ath_dbg(ath9k_hw_common(ah), DYNACK,
>> + "%pM to %u\n", dst, an->ackto);
>> + if (time_is_before_jiffies(da->lto)) {
>> + ath_dynack_compute_ackto(ah);
>> + da->lto = jiffies + COMPUTE_TO;
>> + }
>> + }
>> + INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
>> + }
>> + INCR(da->st_rbf.h_rb, ATH_DYN_BUF);
>> + } else {
>> + INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
>> + }
>> + }
>> +
>> + rcu_read_unlock();
>
> I think it is sufficient to have the rcu_read_unlock just around ieee80211_find_sta_by_ifaddr(). So the lock does not need to
> include the whole while loop under lock.
>
>
> Greetings Thomas
>
>> +}
>> +
>> +/**
>> + * ath_dynack_sample_tx_ts - status ts sampling method
>> + * @ah: ath hw
>> + * @skb: socket buffer
>> + * @ts: tx status info
>> + *
>> + */
>> +void ath_dynack_sample_tx_ts(struct ath_hw *ah, struct sk_buff *skb,
>> + struct ath_tx_status *ts)
>> +{
>> + u8 ridx;
>> + struct ieee80211_hdr *hdr;
>> + struct ath_dynack *da = &ah->dynack;
>> + struct ath_common *common = ath9k_hw_common(ah);
>> + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>> +
>> + if ((info->flags & IEEE80211_TX_CTL_NO_ACK) || da->fix_to)
>> + return;
>> +
>> + spin_lock_bh(&da->qlock);
>> +
>> + hdr = (struct ieee80211_hdr *)skb->data;
>> +
>> + /* late ack */
>> + if (ts->ts_status & ATH9K_TXERR_XRETRY) {
>> + if (ieee80211_is_assoc_req(hdr->frame_control) ||
>> + ieee80211_is_assoc_resp(hdr->frame_control)) {
>> + ath_dbg(common, DYNACK, "late ack\n");
>> + ath9k_hw_setslottime(ah, (LATEACK_TO - 3) / 2);
>> + ath9k_hw_set_ack_timeout(ah, LATEACK_TO);
>> + ath9k_hw_set_cts_timeout(ah, LATEACK_TO);
>> + da->lto = jiffies + LATEACK_DELAY;
>> + }
>> +
>> + spin_unlock_bh(&da->qlock);
>> + return;
>> + }
>> +
>> + ridx = ts->ts_rateindex;
>> +
>> + da->st_rbf.ts[da->st_rbf.t_rb].tstamp = ts->ts_tstamp;
>> + da->st_rbf.ts[da->st_rbf.t_rb].dur = ts->duration[ts->ts_rateindex];
>> + ether_addr_copy(da->st_rbf.addr[da->st_rbf.t_rb].h_dest, hdr->addr1);
>> + ether_addr_copy(da->st_rbf.addr[da->st_rbf.t_rb].h_src, hdr->addr2);
>> +
>> + if (!(info->status.rates[ridx].flags & IEEE80211_TX_RC_MCS)) {
>> + u32 phy, sifs;
>> + const struct ieee80211_rate *rate;
>> + struct ieee80211_tx_rate *rates = info->status.rates;
>> +
>> + rate = &common->sbands[info->band].bitrates[rates[ridx].idx];
>> + if (info->band == IEEE80211_BAND_2GHZ &&
>> + !(rate->flags & IEEE80211_RATE_ERP_G))
>> + phy = WLAN_RC_PHY_CCK;
>> + else
>> + phy = WLAN_RC_PHY_OFDM;
>> +
>> + sifs = ath_dynack_get_sifs(ah, phy);
>> + da->st_rbf.ts[da->st_rbf.t_rb].dur -= sifs;
>> + }
>> +
>> + ath_dbg(common, DYNACK, "{%pM} tx sample %u [dur %u][h %u-t %u]\n",
>> + hdr->addr1, da->st_rbf.ts[da->st_rbf.t_rb].tstamp,
>> + da->st_rbf.ts[da->st_rbf.t_rb].dur, da->st_rbf.h_rb,
>> + (da->st_rbf.t_rb + 1) % ATH_DYN_BUF);
>> +
>> + INCR(da->st_rbf.t_rb, ATH_DYN_BUF);
>> + if (da->st_rbf.t_rb == da->st_rbf.h_rb)
>> + INCR(da->st_rbf.h_rb, ATH_DYN_BUF);
>> +
>> + ath_dynack_compute_to(ah);
>> +
>> + spin_unlock_bh(&da->qlock);
>> +}
>> +EXPORT_SYMBOL(ath_dynack_sample_tx_ts);
>> +
>> +/**
>> + * ath_dynack_sample_ack_ts - ack ts sampling method
>> + * @ah: ath hw
>> + * @skb: socket buffer
>> + * @ts: rx timestamp
>> + *
>> + */
>> +void ath_dynack_sample_ack_ts(struct ath_hw *ah, struct sk_buff *skb,
>> + u32 ts)
>> +{
>> + struct ath_dynack *da = &ah->dynack;
>> + struct ath_common *common = ath9k_hw_common(ah);
>> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>> +
>> + if (!ath_dynack_bssidmask(ah, hdr->addr1) || da->fix_to)
>> + return;
>> +
>> + spin_lock_bh(&da->qlock);
>> + da->ack_rbf.tstamp[da->ack_rbf.t_rb] = ts;
>> +
>> + ath_dbg(common, DYNACK, "rx sample %u [h %u-t %u]\n",
>> + da->ack_rbf.tstamp[da->ack_rbf.t_rb],
>> + da->ack_rbf.h_rb, (da->ack_rbf.t_rb + 1) % ATH_DYN_BUF);
>> +
>> + INCR(da->ack_rbf.t_rb, ATH_DYN_BUF);
>> + if (da->ack_rbf.t_rb == da->ack_rbf.h_rb)
>> + INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
>> +
>> + ath_dynack_compute_to(ah);
>> +
>> + spin_unlock_bh(&da->qlock);
>> +}
>> +EXPORT_SYMBOL(ath_dynack_sample_ack_ts);
>> +
>> +/**
>> + * ath_dynack_reset - reset dynack processing
>> + * @ah: ath hw
>> + *
>> + */
>> +void ath_dynack_reset(struct ath_hw *ah)
>> +{
>> + /* ackto = slottime + sifs + air delay */
>> + u32 ackto = ATH9K_SLOT_TIME_9 + 16 + 64;
>> + struct ath_dynack *da = &ah->dynack;
>> +
>> + da->lto = jiffies;
>> + da->ackto = ackto;
>> +
>> + da->st_rbf.t_rb = 0;
>> + da->st_rbf.h_rb = 0;
>> + da->ack_rbf.t_rb = 0;
>> + da->ack_rbf.h_rb = 0;
>> +
>> + /* init acktimeout */
>> + ath9k_hw_setslottime(ah, (ackto - 3) / 2);
>> + ath9k_hw_set_ack_timeout(ah, ackto);
>> + ath9k_hw_set_cts_timeout(ah, ackto);
>> +}
>> +EXPORT_SYMBOL(ath_dynack_reset);
>> +
>> +/**
>> + * ath_dynack_init - init dynack data structure
>> + * @ah: ath hw
>> + *
>> + */
>> +void ath_dynack_init(struct ath_hw *ah)
>> +{
>> + struct ath_dynack *da = &ah->dynack;
>> +
>> + memset(da, 0, sizeof(struct ath_dynack));
>> +
>> + spin_lock_init(&da->qlock);
>> + INIT_LIST_HEAD(&da->nodes);
>> +
>> + ath_dynack_reset(ah);
>> +}
>> +
>> diff --git a/drivers/net/wireless/ath/ath9k/dynack.h b/drivers/net/wireless/ath/ath9k/dynack.h
>> new file mode 100644
>> index 0000000..386f176
>> --- /dev/null
>> +++ b/drivers/net/wireless/ath/ath9k/dynack.h
>> @@ -0,0 +1,81 @@
>> +/*
>> + * Copyright 2014, Lorenzo Bianconi <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef DYNACK_H
>> +#define DYNACK_H
>> +
>> +#define ATH_DYN_BUF 64
>> +
>> +struct ath_hw;
>> +
>> +/**
>> + * ath_dyn_rxbuf - ack frame ring buffer
>> + */
>> +struct ath_dyn_rxbuf {
>> + u16 h_rb, t_rb;
>> + u32 tstamp[ATH_DYN_BUF];
>> +};
>> +
>> +struct ts_info {
>> + u32 tstamp;
>> + u32 dur;
>> +};
>> +
>> +struct haddr_pair {
>> + u8 h_dest[ETH_ALEN];
>> + u8 h_src[ETH_ALEN];
>> +};
>> +/**
>> + * ath_dyn_txbuf - tx frame ring buffer
>> + */
>> +struct ath_dyn_txbuf {
>> + u16 h_rb, t_rb;
>> + struct haddr_pair addr[ATH_DYN_BUF];
>> + struct ts_info ts[ATH_DYN_BUF];
>> +};
>> +
>> +/**
>> + * ath_dynack - dyn ack processing info
>> + * @fix_to: use static ack timeout
>> + * @ackto: current ack timeout
>> + * @lto: last ack timeout computation
>> + * @nodes: ath_node linked list
>> + * @qlock: ts queue spinlock
>> + * @ack_rbf: ack ts ring buffer
>> + * @st_rbf: status ts ring buffer
>> + */
>> +struct ath_dynack {
>> + bool fix_to;
>> + int ackto;
>> + unsigned long lto;
>> +
>> + struct list_head nodes;
>> +
>> + /* protect timestamp queue access */
>> + spinlock_t qlock;
>> + struct ath_dyn_rxbuf ack_rbf;
>> + struct ath_dyn_txbuf st_rbf;
>> +};
>> +
>> +#if defined(CONFIG_ATH9K_DYNACK)
>> +void ath_dynack_reset(struct ath_hw *ah);
>> +void ath_dynack_init(struct ath_hw *ah);
>> +void ath_dynack_sample_ack_ts(struct ath_hw *ah, struct sk_buff *skb, u32 ts);
>> +void ath_dynack_sample_tx_ts(struct ath_hw *ah, struct sk_buff *skb,
>> + struct ath_tx_status *ts);
>> +#else
>> +static inline void ath_dynack_reset(struct ath_hw *ah) {}
>> +static inline void ath_dynack_init(struct ath_hw *ah) {}
>> +static inline void ath_dynack_sample_ack_ts(struct ath_hw *ah,
>> + struct sk_buff *skb, u32 ts) {}
>> +static inline void ath_dynack_sample_tx_ts(struct ath_hw *ah,
>> + struct sk_buff *skb,
>> + struct ath_tx_status *ts) {}
>> +#endif
>> +
>> +#endif /* DYNACK_H */
>> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
>> index e88896c..9a6b113 100644
>> --- a/drivers/net/wireless/ath/ath9k/hw.c
>> +++ b/drivers/net/wireless/ath/ath9k/hw.c
>> @@ -647,6 +647,8 @@ int ath9k_hw_init(struct ath_hw *ah)
>> return ret;
>> }
>>
>> + ath_dynack_init(ah);
>> +
>> return 0;
>> }
>> EXPORT_SYMBOL(ath9k_hw_init);
>> diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
>> index f36d971..b9eef33 100644
>> --- a/drivers/net/wireless/ath/ath9k/hw.h
>> +++ b/drivers/net/wireless/ath/ath9k/hw.h
>> @@ -29,6 +29,7 @@
>> #include "reg.h"
>> #include "phy.h"
>> #include "btcoex.h"
>> +#include "dynack.h"
>>
>> #include "../regd.h"
>>
>> @@ -924,6 +925,8 @@ struct ath_hw {
>> int (*external_reset)(void);
>>
>> const struct firmware *eeprom_blob;
>> +
>> + struct ath_dynack dynack;
>> };
>>
>> struct ath_bus_ops {
>> --
>> 1.9.1
>>
>> _______________________________________________
>> ath9k-devel mailing list
>> [email protected]
>> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>

Best regards,

Lorenzo

--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

2014-07-07 09:31:53

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 05/10] ath9k: do not overwrite ack timeout estimation

Do not overwrite ack timeout estimation in ath9k_hw_init_global_settings() if
dynack processing is enabled

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/hw.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 9a6b113..d9406ba 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1055,6 +1055,14 @@ void ath9k_hw_init_global_settings(struct ath_hw *ah)
ctstimeout += 48 - sifstime - ah->slottime;
}

+ if (!ah->dynack.fix_to) {
+ acktimeout = ah->dynack.ackto;
+ ctstimeout = acktimeout;
+ slottime = (acktimeout - 3) / 2;
+ } else {
+ ah->dynack.ackto = acktimeout;
+ }
+
ath9k_hw_set_sifs_time(ah, sifstime);
ath9k_hw_setslottime(ah, slottime);
ath9k_hw_set_ack_timeout(ah, acktimeout);
--
1.9.1


2014-07-07 12:02:55

by Felix Fietkau

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC 03/10] ath9k: add dynamic ack timeout estimation

On 2014-07-07 13:41, Thomas H?hn wrote:

>> +{
>> + struct ath_node *an;
>> + u32 to = 0;
>> + struct ath_dynack *da = &ah->dynack;
>> + struct ath_common *common = ath9k_hw_common(ah);
>> +
>
>> + list_for_each_entry(an, &da->nodes, list)
>> + if (an->ackto > to)
>> + to = an->ackto;
>> +
>
> This list parsing would probably need rcu protection like:
>
> rcu_read_lock();
> list_for_each_entry(an, &da->nodes, list)
> if (an->ackto > to)
> to = an->ackto;
> rcu_read_unlock();
Nope, that's already done in the calling code.

> I am not sure that you need to call the entire function with spin_lock as you do it now.
>
>> + if (to && da->ackto != to) {
>> + u32 slottime;
>> +
>> + slottime = (to - 3) / 2;
>
> Should the case to < 3 be covered or is it safe to have potentially slottime = 0 ?
slottime should never be 0. It is not user configurable.

>> +/**
>> + * ath_dynack_compute_to - compute ack timeout
>> + * @ah: ath hw
>> + *
>> + * should be called while holding qlock
>> + */
>> +static void ath_dynack_compute_to(struct ath_hw *ah)
>> +{
>> + u32 ackto, ack_ts;
>> + u8 *dst, *src;
>> + struct ieee80211_sta *sta;
>> + struct ath_node *an;
>> + struct ts_info *st_ts;
>> + struct ath_dynack *da = &ah->dynack;
>> +
>> + rcu_read_lock();
>> +
>> + while (da->st_rbf.h_rb != da->st_rbf.t_rb &&
>> + da->ack_rbf.h_rb != da->ack_rbf.t_rb) {
>> + ack_ts = da->ack_rbf.tstamp[da->ack_rbf.h_rb];
>> + st_ts = &da->st_rbf.ts[da->st_rbf.h_rb];
>> + dst = da->st_rbf.addr[da->st_rbf.h_rb].h_dest;
>> + src = da->st_rbf.addr[da->st_rbf.h_rb].h_src;
>> +
>> + ath_dbg(ath9k_hw_common(ah), DYNACK,
>> + "ack_ts %u st_ts %u st_dur %u [%u-%u]\n",
>> + ack_ts, st_ts->tstamp, st_ts->dur,
>> + da->ack_rbf.h_rb, da->st_rbf.h_rb);
>> +
>> + if (ack_ts > st_ts->tstamp + st_ts->dur) {
>> + ackto = ack_ts - st_ts->tstamp - st_ts->dur;
>> +
>> + if (ackto < MAX_DELAY) {
>> + sta = ieee80211_find_sta_by_ifaddr(ah->hw, dst,
>> + src);
>> + if (sta) {
>> + an = (struct ath_node *)sta->drv_priv;
>> + an->ackto = DYNACK_EWMA((u32)ackto,
>> + an->ackto);
>> + ath_dbg(ath9k_hw_common(ah), DYNACK,
>> + "%pM to %u\n", dst, an->ackto);
>> + if (time_is_before_jiffies(da->lto)) {
>> + ath_dynack_compute_ackto(ah);
>> + da->lto = jiffies + COMPUTE_TO;
>> + }
>> + }
>> + INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
>> + }
>> + INCR(da->st_rbf.h_rb, ATH_DYN_BUF);
>> + } else {
>> + INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
>> + }
>> + }
>> +
>> + rcu_read_unlock();
>
> I think it is sufficient to have the rcu_read_unlock just around
> ieee80211_find_sta_by_ifaddr(). So the lock does not need to
> include the whole while loop under lock.
That doesn't make things any better - rcu_read_lock is not like a normal
lock. Doing it once outside of the loop is not only simpler, but also
slightly more efficient.

- Felix

2014-07-07 09:31:51

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 01/10] ath9k: export methods related to ack timeout estimation

Remove static keyword and export ath9k_hw_setslottime(),
ath9k_hw_set_ack_timeout() and ath9k_hw_set_cts_timeout() in hw.h. These
methods will be used in dynamic ack timeout estimation

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/hw.c | 6 +++---
drivers/net/wireless/ath/ath9k/hw.h | 4 ++++
2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index fd0158f..e88896c 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -935,21 +935,21 @@ static void ath9k_hw_set_sifs_time(struct ath_hw *ah, u32 us)
REG_WRITE(ah, AR_D_GBL_IFS_SIFS, val);
}

-static void ath9k_hw_setslottime(struct ath_hw *ah, u32 us)
+void ath9k_hw_setslottime(struct ath_hw *ah, u32 us)
{
u32 val = ath9k_hw_mac_to_clks(ah, us);
val = min(val, (u32) 0xFFFF);
REG_WRITE(ah, AR_D_GBL_IFS_SLOT, val);
}

-static void ath9k_hw_set_ack_timeout(struct ath_hw *ah, u32 us)
+void ath9k_hw_set_ack_timeout(struct ath_hw *ah, u32 us)
{
u32 val = ath9k_hw_mac_to_clks(ah, us);
val = min(val, (u32) MS(0xFFFFFFFF, AR_TIME_OUT_ACK));
REG_RMW_FIELD(ah, AR_TIME_OUT, AR_TIME_OUT_ACK, val);
}

-static void ath9k_hw_set_cts_timeout(struct ath_hw *ah, u32 us)
+void ath9k_hw_set_cts_timeout(struct ath_hw *ah, u32 us)
{
u32 val = ath9k_hw_mac_to_clks(ah, us);
val = min(val, (u32) MS(0xFFFFFFFF, AR_TIME_OUT_CTS));
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 51b4ebe..f36d971 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -1080,6 +1080,10 @@ void ar9002_hw_load_ani_reg(struct ath_hw *ah, struct ath9k_channel *chan);
void ath9k_ani_reset(struct ath_hw *ah, bool is_scanning);
void ath9k_hw_ani_monitor(struct ath_hw *ah, struct ath9k_channel *chan);

+void ath9k_hw_set_ack_timeout(struct ath_hw *ah, u32 us);
+void ath9k_hw_set_cts_timeout(struct ath_hw *ah, u32 us);
+void ath9k_hw_setslottime(struct ath_hw *ah, u32 us);
+
#ifdef CONFIG_ATH9K_BTCOEX_SUPPORT
static inline bool ath9k_hw_btcoex_is_enabled(struct ath_hw *ah)
{
--
1.9.1


2014-07-07 09:31:53

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 04/10] ath9k: add config for (en|dis)abling ack timeout estimation

Add ack timeout estimation to ath9k Makefile

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/Kconfig | 7 +++++++
drivers/net/wireless/ath/ath9k/Makefile | 3 +++
2 files changed, 10 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
index 8fcc029..ee61a4c 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -151,3 +151,10 @@ config ATH9K_HTC_DEBUGFS
depends on ATH9K_HTC && DEBUG_FS
---help---
Say Y, if you need access to ath9k_htc's statistics.
+
+config ATH9K_DYNACK
+ bool "Atheros ath9k ack timeout estimation algorithm (EXPERIMENTAL)"
+ depends on ATH9K
+ default n
+ ---help---
+ This option enables ath9k dynamic ack timeout estimation algorithm
diff --git a/drivers/net/wireless/ath/ath9k/Makefile b/drivers/net/wireless/ath/ath9k/Makefile
index 6b4020a..73704c1 100644
--- a/drivers/net/wireless/ath/ath9k/Makefile
+++ b/drivers/net/wireless/ath/ath9k/Makefile
@@ -49,6 +49,9 @@ ath9k_hw-$(CONFIG_ATH9K_WOW) += ar9003_wow.o

ath9k_hw-$(CONFIG_ATH9K_BTCOEX_SUPPORT) += btcoex.o \
ar9003_mci.o
+
+ath9k_hw-$(CONFIG_ATH9K_DYNACK) += dynack.o
+
obj-$(CONFIG_ATH9K_HW) += ath9k_hw.o

obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
--
1.9.1


2014-07-07 09:31:55

by Lorenzo Bianconi

[permalink] [raw]
Subject: [RFC 06/10] ath9k: add sampling methods for (tx|rx) timestamp

Add sampling methods for ack RX timestamp in ath_rx_tasklet() and for TX frame
timestamp in ath_tx_complete_aggr() and in ath_tx_process_buffer()

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
drivers/net/wireless/ath/ath9k/recv.c | 5 +++++
drivers/net/wireless/ath/ath9k/xmit.c | 5 +++++
2 files changed, 10 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 74ab1d0..ad317a4 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -991,6 +991,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
unsigned long flags;
dma_addr_t new_buf_addr;
unsigned int budget = 512;
+ struct ieee80211_hdr *hdr;

if (edma)
dma_type = DMA_BIDIRECTIONAL;
@@ -1120,6 +1121,10 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
ath9k_apply_ampdu_details(sc, &rs, rxs);
ath_debug_rate_stats(sc, &rs, skb);

+ hdr = (struct ieee80211_hdr *)skb->data;
+ if (ieee80211_is_ack(hdr->frame_control))
+ ath_dynack_sample_ack_ts(sc->sc_ah, skb, rs.rs_tstamp);
+
ieee80211_rx(hw, skb);

requeue_drop_frag:
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index d4927c9..9497ed7 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -588,6 +588,10 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
memcpy(tx_info->control.rates, rates, sizeof(rates));
ath_tx_rc_status(sc, bf, ts, nframes, nbad, txok);
rc_update = false;
+ if (bf == bf->bf_lastbf)
+ ath_dynack_sample_tx_ts(sc->sc_ah,
+ bf->bf_mpdu,
+ ts);
}

ath_tx_complete_buf(sc, bf, txq, &bf_head, ts,
@@ -688,6 +692,7 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
memcpy(info->control.rates, bf->rates,
sizeof(info->control.rates));
ath_tx_rc_status(sc, bf, ts, 1, txok ? 0 : 1, txok);
+ ath_dynack_sample_tx_ts(sc->sc_ah, bf->bf_mpdu, ts);
}
ath_tx_complete_buf(sc, bf, txq, bf_head, ts, txok);
} else
--
1.9.1


2014-07-07 11:48:31

by Thomas Huehn

[permalink] [raw]
Subject: Re: [ath9k-devel] [RFC 03/10] ath9k: add dynamic ack timeout estimation

Hi Lorenzo,

My comments are inline.


> Add dynamic ack timeout estimation algorithm based on ack frame RX timestamp,
> TX frame timestamp and frame duration.
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
> drivers/net/wireless/ath/ath.h | 2 +
> drivers/net/wireless/ath/ath9k/ath9k.h | 3 +
> drivers/net/wireless/ath/ath9k/dynack.c | 293 ++++++++++++++++++++++++++++++++
> drivers/net/wireless/ath/ath9k/dynack.h | 81 +++++++++
> drivers/net/wireless/ath/ath9k/hw.c | 2 +
> drivers/net/wireless/ath/ath9k/hw.h | 3 +
> 6 files changed, 384 insertions(+)
> create mode 100644 drivers/net/wireless/ath/ath9k/dynack.c
> create mode 100644 drivers/net/wireless/ath/ath9k/dynack.h
>
> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> index fd9e530..4e51072 100644
> --- a/drivers/net/wireless/ath/ath.h
> +++ b/drivers/net/wireless/ath/ath.h
> @@ -234,6 +234,7 @@ void ath_printk(const char *level, const struct ath_common *common,
> * AR9462.
> * @ATH_DBG_DFS: radar datection
> * @ATH_DBG_WOW: Wake on Wireless
> + * @ATH_DBG_DYNACK: dynack handling
> * @ATH_DBG_ANY: enable all debugging
> *
> * The debug level is used to control the amount and type of debugging output
> @@ -261,6 +262,7 @@ enum ATH_DEBUG {
> ATH_DBG_MCI = 0x00008000,
> ATH_DBG_DFS = 0x00010000,
> ATH_DBG_WOW = 0x00020000,
> + ATH_DBG_DYNACK = 0x00040000,
> ATH_DBG_ANY = 0xffffffff
> };
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 11b5e4d..65a2587 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -272,6 +272,9 @@ struct ath_node {
> struct ath_rx_rate_stats rx_rate_stats;
> #endif
> u8 key_idx[4];
> +
> + u32 ackto;
> + struct list_head list;
> };
>
> struct ath_tx_control {
> diff --git a/drivers/net/wireless/ath/ath9k/dynack.c b/drivers/net/wireless/ath/ath9k/dynack.c
> new file mode 100644
> index 0000000..50297e7
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath9k/dynack.c
> @@ -0,0 +1,293 @@
> +/*
> + * Copyright 2014, Lorenzo Bianconi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include "ath9k.h"
> +#include "hw.h"
> +#include "dynack.h"
> +
> +#define COMPUTE_TO (5 * HZ)
> +#define LATEACK_DELAY (10 * HZ)
> +#define LATEACK_TO 256
> +#define MAX_DELAY 300
> +#define EWMA_LEVEL 75
> +#define DYNACK_EWMA(old, new) \
> + (((new) * (100 - EWMA_LEVEL) + (old) * EWMA_LEVEL) / 100)
> +

You could change this EWMA calculation as we did it in Minstrel to use powers of two as the calculation speed increased.
This would change :
- EWMA_DIV from 100 to 2^7
- EWMA_LEVEL from 75 (/EWMA_DIV=100) to 2^6 + 2^5 (/EWMA_DIV=128)

Note that this changes EWMA_DIV - EWMA_LEVEL from 25 to 2^5 and keeps
EWMA_LEVEL / EWMA_DIV == 0.75.

Something like this:

+#define EWMA_LEVEL 96 /* ewma weighting factor [/EWMA_DIV] */
+#define EWMA_DIV 128

static inline int
dynack_ewma(int old, int new, int weight)
{
+ return (new * (EWMA_DIV - weight) + old * weight) / EWMA_DIV;
}


> +/**
> + * ath_dynack_get_sifs - get sifs time based on phy used
> + * @ah: ath hw
> + * @phy: phy used
> + */
> +static inline u32 ath_dynack_get_sifs(struct ath_hw *ah, int phy)
> +{
> + u32 sifs = CCK_SIFS_TIME;
> +
> + if (phy == WLAN_RC_PHY_OFDM) {
> + if (IS_CHAN_QUARTER_RATE(ah->curchan))
> + sifs = OFDM_SIFS_TIME_QUARTER;
> + else if (IS_CHAN_HALF_RATE(ah->curchan))
> + sifs = OFDM_SIFS_TIME_HALF;
> + else
> + sifs = OFDM_SIFS_TIME;
> + }
> + return sifs;
> +}
> +
> +static inline bool ath_dynack_bssidmask(struct ath_hw *ah, const u8 *mac)
> +{
> + int i;
> + struct ath_common *common = ath9k_hw_common(ah);
> +
> + for (i = 0; i < ETH_ALEN; i++) {
> + if ((common->macaddr[i] & common->bssidmask[i]) !=
> + (mac[i] & common->bssidmask[i]))
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/**
> + * ath_dynack_set_ackto - compute ack timeout based on sta timeout
> + * @ah: ath hw
> + *
> + * should be called while holding qlock
> + */
> +static void ath_dynack_compute_ackto(struct ath_hw *ah)

The function name does not match the name in the comment above (ath_dynack_set_ackto != ath_dynack_compute_ackto).

> +{
> + struct ath_node *an;
> + u32 to = 0;
> + struct ath_dynack *da = &ah->dynack;
> + struct ath_common *common = ath9k_hw_common(ah);
> +

> + list_for_each_entry(an, &da->nodes, list)
> + if (an->ackto > to)
> + to = an->ackto;
> +

This list parsing would probably need rcu protection like:

rcu_read_lock();
list_for_each_entry(an, &da->nodes, list)
if (an->ackto > to)
to = an->ackto;
rcu_read_unlock();

I am not sure that you need to call the entire function with spin_lock as you do it now.

> + if (to && da->ackto != to) {
> + u32 slottime;
> +
> + slottime = (to - 3) / 2;

Should the case to < 3 be covered or is it safe to have potentially slottime = 0 ?

> + da->ackto = to;
> + ath_dbg(common, DYNACK, "ack timeout %u slottime %u\n",
> + da->ackto, slottime);
> + ath9k_hw_setslottime(ah, slottime);
> + ath9k_hw_set_ack_timeout(ah, da->ackto);
> + ath9k_hw_set_cts_timeout(ah, da->ackto);
> + }
> +}
> +
> +/**
> + * ath_dynack_compute_to - compute ack timeout
> + * @ah: ath hw
> + *
> + * should be called while holding qlock
> + */
> +static void ath_dynack_compute_to(struct ath_hw *ah)
> +{
> + u32 ackto, ack_ts;
> + u8 *dst, *src;
> + struct ieee80211_sta *sta;
> + struct ath_node *an;
> + struct ts_info *st_ts;
> + struct ath_dynack *da = &ah->dynack;
> +
> + rcu_read_lock();
> +
> + while (da->st_rbf.h_rb != da->st_rbf.t_rb &&
> + da->ack_rbf.h_rb != da->ack_rbf.t_rb) {
> + ack_ts = da->ack_rbf.tstamp[da->ack_rbf.h_rb];
> + st_ts = &da->st_rbf.ts[da->st_rbf.h_rb];
> + dst = da->st_rbf.addr[da->st_rbf.h_rb].h_dest;
> + src = da->st_rbf.addr[da->st_rbf.h_rb].h_src;
> +
> + ath_dbg(ath9k_hw_common(ah), DYNACK,
> + "ack_ts %u st_ts %u st_dur %u [%u-%u]\n",
> + ack_ts, st_ts->tstamp, st_ts->dur,
> + da->ack_rbf.h_rb, da->st_rbf.h_rb);
> +
> + if (ack_ts > st_ts->tstamp + st_ts->dur) {
> + ackto = ack_ts - st_ts->tstamp - st_ts->dur;
> +
> + if (ackto < MAX_DELAY) {
> + sta = ieee80211_find_sta_by_ifaddr(ah->hw, dst,
> + src);
> + if (sta) {
> + an = (struct ath_node *)sta->drv_priv;
> + an->ackto = DYNACK_EWMA((u32)ackto,
> + an->ackto);
> + ath_dbg(ath9k_hw_common(ah), DYNACK,
> + "%pM to %u\n", dst, an->ackto);
> + if (time_is_before_jiffies(da->lto)) {
> + ath_dynack_compute_ackto(ah);
> + da->lto = jiffies + COMPUTE_TO;
> + }
> + }
> + INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
> + }
> + INCR(da->st_rbf.h_rb, ATH_DYN_BUF);
> + } else {
> + INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
> + }
> + }
> +
> + rcu_read_unlock();

I think it is sufficient to have the rcu_read_unlock just around ieee80211_find_sta_by_ifaddr(). So the lock does not need to
include the whole while loop under lock.


Greetings Thomas

> +}
> +
> +/**
> + * ath_dynack_sample_tx_ts - status ts sampling method
> + * @ah: ath hw
> + * @skb: socket buffer
> + * @ts: tx status info
> + *
> + */
> +void ath_dynack_sample_tx_ts(struct ath_hw *ah, struct sk_buff *skb,
> + struct ath_tx_status *ts)
> +{
> + u8 ridx;
> + struct ieee80211_hdr *hdr;
> + struct ath_dynack *da = &ah->dynack;
> + struct ath_common *common = ath9k_hw_common(ah);
> + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> +
> + if ((info->flags & IEEE80211_TX_CTL_NO_ACK) || da->fix_to)
> + return;
> +
> + spin_lock_bh(&da->qlock);
> +
> + hdr = (struct ieee80211_hdr *)skb->data;
> +
> + /* late ack */
> + if (ts->ts_status & ATH9K_TXERR_XRETRY) {
> + if (ieee80211_is_assoc_req(hdr->frame_control) ||
> + ieee80211_is_assoc_resp(hdr->frame_control)) {
> + ath_dbg(common, DYNACK, "late ack\n");
> + ath9k_hw_setslottime(ah, (LATEACK_TO - 3) / 2);
> + ath9k_hw_set_ack_timeout(ah, LATEACK_TO);
> + ath9k_hw_set_cts_timeout(ah, LATEACK_TO);
> + da->lto = jiffies + LATEACK_DELAY;
> + }
> +
> + spin_unlock_bh(&da->qlock);
> + return;
> + }
> +
> + ridx = ts->ts_rateindex;
> +
> + da->st_rbf.ts[da->st_rbf.t_rb].tstamp = ts->ts_tstamp;
> + da->st_rbf.ts[da->st_rbf.t_rb].dur = ts->duration[ts->ts_rateindex];
> + ether_addr_copy(da->st_rbf.addr[da->st_rbf.t_rb].h_dest, hdr->addr1);
> + ether_addr_copy(da->st_rbf.addr[da->st_rbf.t_rb].h_src, hdr->addr2);
> +
> + if (!(info->status.rates[ridx].flags & IEEE80211_TX_RC_MCS)) {
> + u32 phy, sifs;
> + const struct ieee80211_rate *rate;
> + struct ieee80211_tx_rate *rates = info->status.rates;
> +
> + rate = &common->sbands[info->band].bitrates[rates[ridx].idx];
> + if (info->band == IEEE80211_BAND_2GHZ &&
> + !(rate->flags & IEEE80211_RATE_ERP_G))
> + phy = WLAN_RC_PHY_CCK;
> + else
> + phy = WLAN_RC_PHY_OFDM;
> +
> + sifs = ath_dynack_get_sifs(ah, phy);
> + da->st_rbf.ts[da->st_rbf.t_rb].dur -= sifs;
> + }
> +
> + ath_dbg(common, DYNACK, "{%pM} tx sample %u [dur %u][h %u-t %u]\n",
> + hdr->addr1, da->st_rbf.ts[da->st_rbf.t_rb].tstamp,
> + da->st_rbf.ts[da->st_rbf.t_rb].dur, da->st_rbf.h_rb,
> + (da->st_rbf.t_rb + 1) % ATH_DYN_BUF);
> +
> + INCR(da->st_rbf.t_rb, ATH_DYN_BUF);
> + if (da->st_rbf.t_rb == da->st_rbf.h_rb)
> + INCR(da->st_rbf.h_rb, ATH_DYN_BUF);
> +
> + ath_dynack_compute_to(ah);
> +
> + spin_unlock_bh(&da->qlock);
> +}
> +EXPORT_SYMBOL(ath_dynack_sample_tx_ts);
> +
> +/**
> + * ath_dynack_sample_ack_ts - ack ts sampling method
> + * @ah: ath hw
> + * @skb: socket buffer
> + * @ts: rx timestamp
> + *
> + */
> +void ath_dynack_sample_ack_ts(struct ath_hw *ah, struct sk_buff *skb,
> + u32 ts)
> +{
> + struct ath_dynack *da = &ah->dynack;
> + struct ath_common *common = ath9k_hw_common(ah);
> + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> +
> + if (!ath_dynack_bssidmask(ah, hdr->addr1) || da->fix_to)
> + return;
> +
> + spin_lock_bh(&da->qlock);
> + da->ack_rbf.tstamp[da->ack_rbf.t_rb] = ts;
> +
> + ath_dbg(common, DYNACK, "rx sample %u [h %u-t %u]\n",
> + da->ack_rbf.tstamp[da->ack_rbf.t_rb],
> + da->ack_rbf.h_rb, (da->ack_rbf.t_rb + 1) % ATH_DYN_BUF);
> +
> + INCR(da->ack_rbf.t_rb, ATH_DYN_BUF);
> + if (da->ack_rbf.t_rb == da->ack_rbf.h_rb)
> + INCR(da->ack_rbf.h_rb, ATH_DYN_BUF);
> +
> + ath_dynack_compute_to(ah);
> +
> + spin_unlock_bh(&da->qlock);
> +}
> +EXPORT_SYMBOL(ath_dynack_sample_ack_ts);
> +
> +/**
> + * ath_dynack_reset - reset dynack processing
> + * @ah: ath hw
> + *
> + */
> +void ath_dynack_reset(struct ath_hw *ah)
> +{
> + /* ackto = slottime + sifs + air delay */
> + u32 ackto = ATH9K_SLOT_TIME_9 + 16 + 64;
> + struct ath_dynack *da = &ah->dynack;
> +
> + da->lto = jiffies;
> + da->ackto = ackto;
> +
> + da->st_rbf.t_rb = 0;
> + da->st_rbf.h_rb = 0;
> + da->ack_rbf.t_rb = 0;
> + da->ack_rbf.h_rb = 0;
> +
> + /* init acktimeout */
> + ath9k_hw_setslottime(ah, (ackto - 3) / 2);
> + ath9k_hw_set_ack_timeout(ah, ackto);
> + ath9k_hw_set_cts_timeout(ah, ackto);
> +}
> +EXPORT_SYMBOL(ath_dynack_reset);
> +
> +/**
> + * ath_dynack_init - init dynack data structure
> + * @ah: ath hw
> + *
> + */
> +void ath_dynack_init(struct ath_hw *ah)
> +{
> + struct ath_dynack *da = &ah->dynack;
> +
> + memset(da, 0, sizeof(struct ath_dynack));
> +
> + spin_lock_init(&da->qlock);
> + INIT_LIST_HEAD(&da->nodes);
> +
> + ath_dynack_reset(ah);
> +}
> +
> diff --git a/drivers/net/wireless/ath/ath9k/dynack.h b/drivers/net/wireless/ath/ath9k/dynack.h
> new file mode 100644
> index 0000000..386f176
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath9k/dynack.h
> @@ -0,0 +1,81 @@
> +/*
> + * Copyright 2014, Lorenzo Bianconi <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef DYNACK_H
> +#define DYNACK_H
> +
> +#define ATH_DYN_BUF 64
> +
> +struct ath_hw;
> +
> +/**
> + * ath_dyn_rxbuf - ack frame ring buffer
> + */
> +struct ath_dyn_rxbuf {
> + u16 h_rb, t_rb;
> + u32 tstamp[ATH_DYN_BUF];
> +};
> +
> +struct ts_info {
> + u32 tstamp;
> + u32 dur;
> +};
> +
> +struct haddr_pair {
> + u8 h_dest[ETH_ALEN];
> + u8 h_src[ETH_ALEN];
> +};
> +/**
> + * ath_dyn_txbuf - tx frame ring buffer
> + */
> +struct ath_dyn_txbuf {
> + u16 h_rb, t_rb;
> + struct haddr_pair addr[ATH_DYN_BUF];
> + struct ts_info ts[ATH_DYN_BUF];
> +};
> +
> +/**
> + * ath_dynack - dyn ack processing info
> + * @fix_to: use static ack timeout
> + * @ackto: current ack timeout
> + * @lto: last ack timeout computation
> + * @nodes: ath_node linked list
> + * @qlock: ts queue spinlock
> + * @ack_rbf: ack ts ring buffer
> + * @st_rbf: status ts ring buffer
> + */
> +struct ath_dynack {
> + bool fix_to;
> + int ackto;
> + unsigned long lto;
> +
> + struct list_head nodes;
> +
> + /* protect timestamp queue access */
> + spinlock_t qlock;
> + struct ath_dyn_rxbuf ack_rbf;
> + struct ath_dyn_txbuf st_rbf;
> +};
> +
> +#if defined(CONFIG_ATH9K_DYNACK)
> +void ath_dynack_reset(struct ath_hw *ah);
> +void ath_dynack_init(struct ath_hw *ah);
> +void ath_dynack_sample_ack_ts(struct ath_hw *ah, struct sk_buff *skb, u32 ts);
> +void ath_dynack_sample_tx_ts(struct ath_hw *ah, struct sk_buff *skb,
> + struct ath_tx_status *ts);
> +#else
> +static inline void ath_dynack_reset(struct ath_hw *ah) {}
> +static inline void ath_dynack_init(struct ath_hw *ah) {}
> +static inline void ath_dynack_sample_ack_ts(struct ath_hw *ah,
> + struct sk_buff *skb, u32 ts) {}
> +static inline void ath_dynack_sample_tx_ts(struct ath_hw *ah,
> + struct sk_buff *skb,
> + struct ath_tx_status *ts) {}
> +#endif
> +
> +#endif /* DYNACK_H */
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index e88896c..9a6b113 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -647,6 +647,8 @@ int ath9k_hw_init(struct ath_hw *ah)
> return ret;
> }
>
> + ath_dynack_init(ah);
> +
> return 0;
> }
> EXPORT_SYMBOL(ath9k_hw_init);
> diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
> index f36d971..b9eef33 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.h
> +++ b/drivers/net/wireless/ath/ath9k/hw.h
> @@ -29,6 +29,7 @@
> #include "reg.h"
> #include "phy.h"
> #include "btcoex.h"
> +#include "dynack.h"
>
> #include "../regd.h"
>
> @@ -924,6 +925,8 @@ struct ath_hw {
> int (*external_reset)(void);
>
> const struct firmware *eeprom_blob;
> +
> + struct ath_dynack dynack;
> };
>
> struct ath_bus_ops {
> --
> 1.9.1
>
> _______________________________________________
> ath9k-devel mailing list
> [email protected]
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel


2014-07-09 17:47:14

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [RFC 03/10] ath9k: add dynamic ack timeout estimation

Hi Sujith,

> Lorenzo Bianconi wrote:
>> +/*
>> + * Copyright 2014, Lorenzo Bianconi <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>
> I have not reviewed the patches, but the ath9k driver is licensed
> under ISC. Introducing a file with GPLv2 license is bound to cause
> complications and I think might require legal clearance from a
> QCA employee maintaining ath9k.
>
> Right now, I am not sure if there is an official maintainer for
> ath9k, but is there any specific reason why you have chosen
> GPLv2 ?
>
> Sujith

Nope. I didn't know ath9k is under ISC license. I think I can replace
GPLv2 license with an ISC one.
Best regards,

Lorenzo


--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep