Re: [ath9k-devel] Patching ath9k/mac80211 to change MAC layer features

From: Fred Matthews
Date: Mon Jun 27 2011 - 13:37:48 EST


Hi all,

Much appreciated Mohammed Shafi, disabling aggregation works when
replacing those lines with
sc->sc_flags &= ~SC_OP_TXAGGR;
sc->sc_flags &= ~SC_OP_RXAGGR;
only issue is I must always recompile compat-wireless to
re-enable/disable it again. I tried to port it using debugfs to be
able to enable and disable at runtime but ath9k kept rebooting.

Also I would be very grateful if someone can help regarding my
attached patch and previous query:

------------
need to patch ath9k so that I can be able to modify/fix the A-MPDU
aggregation frame length and other parameters such that they can be
configured at runtime via debugfs. I made a newbie attempt at trying
to change the aggregate length (attached), but I am unsure of the
correctness/efficiency - when applied throughput varies with
different frame lengths entered but I can't be sure if its actually
changing the aggregation length (passes the framelen/aggr_limit).
Hence I would be very grateful if someone can assist in the correct
way of doing this (possibly directly via mac80211).


Also if the ampdu aggregate length is changed I believe the
number/length of the subframes of the aggregate stays constant (at the
default which is 32). I try to change this in my patch as well as the
maximum software retries by just changing the default definition in
ath9k.h, thus is there a (more) correct/better way of changing these.
Finally, how can I patch the driver to be able to disable Block-Acks
via debugfs or otherwise.
---------------

Thank you very much



On Mon, Jun 20, 2011 at 9:15 AM, Mohammed Shafi
<shafi.wireless@xxxxxxxxx> wrote:
> On Fri, Jun 17, 2011 at 2:09 AM, Fred Matthews <fredmm@xxxxxxxxxxxxx> wrote:
>> Hello all,
>>
>>
>> I have a query regarding patching ath9k/mac80211 to change MAC layer
>> features and I would be most grateful if someone can assist.
>>
>>
>> I need to patch ath9k so that I can be able to modify/fix the A-MPDU
>> aggregation frame length and other parameters such that they can be
>> configured at runtime via debugfs. I made a newbie attempt at trying to
>> change the aggregate length (attached), but I am unsure of the
>> correctness/efficiency  - when applied throughput varies with different
>> frame lengths entered but I can't be sure if its actually changing the
>> aggregation length (passes the framelen/aggr_limit). Hence I would be very
>> grateful if someone can assist in the correct way of doing this (possibly
>> directly via mac80211).
>>
>>
>>
>> Also if the ampdu aggregate length is changed I believe the number/length of
>> the subframes of the aggregate stays constant (at the default which is
>> 32). I try to change this in my patch as well as the maximum software
>> retries by just changing the default definition in ath9k.h, thus is there a
>> (more) correct/better way of changing these. Finally, how can I patch the
>> driver to be able to disable Block-Acks/aggregation via debugfs or
>> otherwise.
>
>
> to disable aggregation in the driver, please try this
> diff --git a/drivers/net/wireless/ath/ath9k/init.c
> b/drivers/net/wireless/ath/ath9k/init.c
> index d4b166c..e0175d0 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -524,8 +524,6 @@ static void ath9k_init_misc(struct ath_softc *sc)
>        sc->config.txpowlimit = ATH_TXPOWER_MAX;
>
>        if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
> -               sc->sc_flags |= SC_OP_TXAGGR;
> -               sc->sc_flags |= SC_OP_RXAGGR;
>        }
>
>        common->tx_chainmask = sc->sc_ah->caps.tx_chainmask;
>
> if you want only tx-aggregation to be disabled , please comment out only that.
> also you can completely disable HT by
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c
> b/drivers/net/wireless/ath/ath9k/hw.c
> index 6de2655..696b4cc 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -1915,10 +1915,7 @@ int ath9k_hw_fill_cap_info(struct ath_hw *ah)
>
>        common->crypt_caps |= ATH_CRYPT_CAP_CIPHER_AESCCM;
>
> -       if (ah->hw_version.devid != AR2427_DEVID_PCIE)
> -               pCap->hw_caps |= ATH9K_HW_CAP_HT;
> -       else
> -               pCap->hw_caps &= ~ATH9K_HW_CAP_HT;
> +       pCap->hw_caps &= ~ATH9K_HW_CAP_HT;
>
>        if (AR_SREV_9271(ah))
>                pCap->num_gpio_pins = AR9271_NUM_GPIO;
>
>
>
>
>>
>>
>>
>> I highly appreciate, help in any of the above, as well as other advice,
>> Many thanks
>>
>> _______________________________________________
>> ath9k-devel mailing list
>> ath9k-devel@xxxxxxxxxxxxxxx
>> https://lists.ath9k.org/mailman/listinfo/ath9k-devel
>>
>>
>
>
>
> --
> shafi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
diff -Naur a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
--- a/drivers/net/wireless/ath/ath9k/ath9k.h 2011-05-27 20:04:49.000000000 +0100
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h 2011-06-02 19:59:28.043327000 +0100
@@ -137,7 +137,7 @@
#define ATH_AGGR_ENCRYPTDELIM 10
/* minimum h/w qdepth to be sustained to maximize aggregation */
#define ATH_AGGR_MIN_QDEPTH 2
-#define ATH_AMPDU_SUBFRAME_DEFAULT 32
+#define ATH_AMPDU_SUBFRAME_DEFAULT sc->debug.subframe_size

#define IEEE80211_SEQ_SEQ_SHIFT 4
#define IEEE80211_SEQ_MAX 4096
@@ -537,7 +537,7 @@
#define DEFAULT_CACHELINE 32
#define ATH_REGCLASSIDS_MAX 10
#define ATH_CABQ_READY_TIME 80 /* % of beacon interval */
-#define ATH_MAX_SW_RETRIES 10
+#define ATH_MAX_SW_RETRIES sc->debug.retry_limit
#define ATH_CHAN_MAX 255

#define ATH_TXPOWER_MAX 100 /* .5 dBm units */
diff -Naur a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
--- a/drivers/net/wireless/ath/ath9k/debug.c 2011-05-27 20:04:49.000000000 +0100
+++ b/drivers/net/wireless/ath/ath9k/debug.c 2011-06-02 19:54:09.260320000 +0100
@@ -1045,6 +1048,162 @@
.llseek = default_llseek,
};

+static ssize_t read_file_ampdu_len(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ char buf[32];
+ unsigned int len;
+
+ len = sprintf(buf, "%u\n", sc->debug.ampdu_len);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_file_ampdu_len(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ unsigned long ampdu_len;
+ char buf[32];
+ ssize_t len;
+
+ len = min(count, sizeof(buf) - 1);
+ if (copy_from_user(buf, user_buf, len))
+ return -EFAULT;
+
+ buf[len] = '\0';
+ if (strict_strtoul(buf, 0, &ampdu_len))
+ return -EINVAL;
+
+ sc->debug.ampdu_len = ampdu_len;
+ return count;
+}
+
+static const struct file_operations fops_ampdu_len = {
+ .read = read_file_ampdu_len,
+ .write = write_file_ampdu_len,
+ .open = ath9k_debugfs_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
+static ssize_t read_file_retry_limit(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ char buf[16];
+ unsigned int len;
+
+ len = sprintf(buf, "%u\n", sc->debug.retry_limit);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_file_retry_limit(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ unsigned long retry_limit;
+ char buf[16];
+ ssize_t len;
+
+ len = min(count, sizeof(buf) - 1);
+ if (copy_from_user(buf, user_buf, len))
+ return -EFAULT;
+
+ buf[len] = '\0';
+ if (strict_strtoul(buf, 0, &retry_limit))
+ return -EINVAL;
+
+ sc->debug.retry_limit = retry_limit;
+ return count;
+}
+
+static const struct file_operations fops_retry_limit = {
+ .read = read_file_retry_limit,
+ .write = write_file_retry_limit,
+ .open = ath9k_debugfs_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
+static ssize_t read_file_subframe_size(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ char buf[16];
+ unsigned int len;
+
+ len = sprintf(buf, "%u\n", sc->debug.subframe_size);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_file_subframe_size(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ unsigned long subframe_size;
+ char buf[16];
+ ssize_t len;
+
+ len = min(count, sizeof(buf) - 1);
+ if (copy_from_user(buf, user_buf, len))
+ return -EFAULT;
+
+ buf[len] = '\0';
+ if (strict_strtoul(buf, 0, &subframe_size))
+ return -EINVAL;
+
+ sc->debug.subframe_size = subframe_size;
+ return count;
+}
+
+static const struct file_operations fops_subframe_size = {
+ .read = read_file_subframe_size,
+ .write = write_file_subframe_size,
+ .open = ath9k_debugfs_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
+static ssize_t read_file_auto_ampdu(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ char buf[8];
+ unsigned int len;
+
+ len = sprintf(buf, "%u\n", sc->debug.auto_ampdu);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_file_auto_ampdu(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_softc *sc = file->private_data;
+ unsigned long auto_ampdu;
+ char buf[8];
+ ssize_t len;
+
+ len = min(count, sizeof(buf) - 1);
+ if (copy_from_user(buf, user_buf, len))
+ return -EFAULT;
+
+ buf[len] = '\0';
+ if (strict_strtoul(buf, 0, &auto_ampdu))
+ return -EINVAL;
+
+ sc->debug.auto_ampdu = auto_ampdu;
+ return count;
+}
+
+static const struct file_operations fops_auto_ampdu = {
+ .read = read_file_auto_ampdu,
+ .write = write_file_auto_ampdu,
+ .open = ath9k_debugfs_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
static ssize_t read_file_regval(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
@@ -1162,6 +1407,15 @@
sc->debug.debugfs_phy, sc, &fops_tx_chainmask);
debugfs_create_file("regidx", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
sc, &fops_regidx);
+
+ debugfs_create_file("ampdu_len", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
+ sc, &fops_ampdu_len);
+ debugfs_create_file("retry_limit", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
+ sc, &fops_retry_limit);
+ debugfs_create_file("subframe_size", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
+ sc, &fops_subframe_size);
+ debugfs_create_file("auto_ampdu[0:OFF,1:ON]", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
+ sc, &fops_auto_ampdu);
debugfs_create_file("regval", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
sc, &fops_regval);
debugfs_create_bool("ignore_extcca", S_IRUSR | S_IWUSR,
@@ -1177,5 +1302,8 @@
sc->debug.debugfs_phy, &sc->sc_ah->gpio_val);

sc->debug.regidx = 0;
+ sc->debug.auto_ampdu = 1;
+ sc->debug.ampdu_len = ATH_AMPDU_LIMIT_MAX;
+ sc->debug.retry_limit = 10;
return 0;
}

diff -Naur a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
--- a/drivers/net/wireless/ath/ath9k/debug.h 2011-05-27 20:04:49.000000000 +0100
+++ b/drivers/net/wireless/ath/ath9k/debug.h 2011-06-02 19:55:33.332337000 +0100
@@ -179,6 +180,10 @@
struct ath9k_debug {
struct dentry *debugfs_phy;
u32 regidx;
+ u32 ampdu_len;
+ u16 retry_limit;
+ u16 subframe_size;
+ u8 auto_ampdu;
struct ath_stats stats;
};

diff -Naur a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
--- a/drivers/net/wireless/ath/ath9k/xmit.c 2011-05-27 20:04:49.000000000 +0100
+++ b/drivers/net/wireless/ath/ath9k/xmit.c 2011-06-02 19:58:19.647347000 +0100
@@ -616,8 +616,12 @@
if (rates[i].flags & IEEE80211_TX_RC_SHORT_GI)
modeidx++;

- frmlen = ath_max_4ms_framelen[modeidx][rates[i].idx];
- max_4ms_framelen = min(max_4ms_framelen, frmlen);
+ frmlen = ath_max_4ms_framelen[modeidx][rates[i].idx];
+
+ if (sc->debug.auto_ampdu==1)
+ max_4ms_framelen = min(max_4ms_framelen, frmlen);
+ else
+ max_4ms_framelen = sc->debug.ampdu_len;
}
}

@@ -641,7 +645,7 @@
* The IE, however can hold up to 65536, which shows up here
* as zero. Ignore 65536 since we are constrained by hw.
*/
- if (tid->an->maxampdu)
+ if (sc->debug.auto_ampdu==1 && tid->an->maxampdu)
aggr_limit = min(aggr_limit, tid->an->maxampdu);

return aggr_limit;