Setting tx retry count in ath10k

Setting tx retry count in ath10k

[Ben Greear]

I spent a bit of time looking into setting the tx retry count in
ath10k (wave-1 firmware).  The firmware has support for setting this as
a vdev parameter, and it defaults to '2', at least in my wave-1 firmware.

I enabled propagating the setting from mac80211, ie:
iw phy wiphy0 set retry short 2 long 2

And while debugging this, I noticed that mac80211 has a default of
4, but the ath10k firmware has a default of 2.  Now, I am not sure
if I should enable setting the retry count since it will change
the behaviour even if users don't set anything.

Any opinions on this?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: Setting tx retry count in ath10k

[Benjamin Beichler]

Hi,

Am 17.07.2018 um 02:37 schrieb Ben Greear:
> I spent a bit of time looking into setting the tx retry count in
> ath10k (wave-1 firmware).  The firmware has support for setting this as
> a vdev parameter, and it defaults to '2', at least in my wave-1 firmware.
>
> I enabled propagating the setting from mac80211, ie:
> iw phy wiphy0 set retry short 2 long 2
>
> And while debugging this, I noticed that mac80211 has a default of
> 4, but the ath10k firmware has a default of 2.  Now, I am not sure
> if I should enable setting the retry count since it will change
> the behaviour even if users don't set anything.
>
Maybe I'm wrong, but I have in mind, that 7 retries is the default
setting of mac80211. Although 2 or even 4 seems to be pretty low for the
overall retry count, so maybe the values are somehow changed in the
firmware? From our experiments we know (at least for 802.11n) you need
for normal operation a retry count of something between 5 - 9, but
sometimes also 12 or 15 is beneficial.

We use for our experimental setup mainly ath9k cards and rt28xx nics,
and with them you need definitely more retries.

Nonetheless, I don't think the change from 2 to 4 does really affect the
behavior in a negative way (if it works as expected).

> Any opinions on this?
>
> Thanks,
> Ben
>

-- 
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: Benjamin.Beichler@uni-rostock.de
www: http://www.imd.uni-rostock.de/




_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: Setting tx retry count in ath10k

[SEDE]

Hi,

In the standard, 7 is the default for the short retry count, 4 is well 
the default for long retry count.

In ath10k, there is also non_agg_sw_retry_th to control this, will this 
still be used?
Or what is the difference with rc_num_retries?

Kind regards,
Sébastien.


On 2018-07-17 09:39, Benjamin Beichler wrote:
> Hi,
>
> Am 17.07.2018 um 02:37 schrieb Ben Greear:
>> I spent a bit of time looking into setting the tx retry count in
>> ath10k (wave-1 firmware).  The firmware has support for setting this as
>> a vdev parameter, and it defaults to '2', at least in my wave-1 firmware.
>>
>> I enabled propagating the setting from mac80211, ie:
>> iw phy wiphy0 set retry short 2 long 2
>>
>> And while debugging this, I noticed that mac80211 has a default of
>> 4, but the ath10k firmware has a default of 2.  Now, I am not sure
>> if I should enable setting the retry count since it will change
>> the behaviour even if users don't set anything.
>>
> Maybe I'm wrong, but I have in mind, that 7 retries is the default
> setting of mac80211. Although 2 or even 4 seems to be pretty low for the
> overall retry count, so maybe the values are somehow changed in the
> firmware? From our experiments we know (at least for 802.11n) you need
> for normal operation a retry count of something between 5 - 9, but
> sometimes also 12 or 15 is beneficial.
>
> We use for our experimental setup mainly ath9k cards and rt28xx nics,
> and with them you need definitely more retries.
>
> Nonetheless, I don't think the change from 2 to 4 does really affect the
> behavior in a negative way (if it works as expected).
>
>> Any opinions on this?
>>
>> Thanks,
>> Ben
>>


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: Setting tx retry count in ath10k

[Ben Greear]

On 07/17/2018 12:56 AM, SEDE wrote:
> Hi,
>
> In the standard, 7 is the default for the short retry count, 4 is well the default for long retry count.
>
> In ath10k, there is also non_agg_sw_retry_th to control this, will this still be used?
> Or what is the difference with rc_num_retries?
>
> Kind regards,
> Sébastien.

The ath10k firmware has no idea of long vs short retries, so I just used the
long setting.

I will investigate that non_agg_sw_retry_th as well, and I did notice my wave-1
firmware (at least) uses 15 retries for self-generated frames.  But, in my case,
those self-gen frames are not much used anyway since I disable firmware keep-alive.

And, I need to see how the mac80211 stack handles its own retries when working
with ath10k.

Thanks,
Ben

>
>
> On 2018-07-17 09:39, Benjamin Beichler wrote:
>> Hi,
>>
>> Am 17.07.2018 um 02:37 schrieb Ben Greear:
>>> I spent a bit of time looking into setting the tx retry count in
>>> ath10k (wave-1 firmware).  The firmware has support for setting this as
>>> a vdev parameter, and it defaults to '2', at least in my wave-1 firmware.
>>>
>>> I enabled propagating the setting from mac80211, ie:
>>> iw phy wiphy0 set retry short 2 long 2
>>>
>>> And while debugging this, I noticed that mac80211 has a default of
>>> 4, but the ath10k firmware has a default of 2.  Now, I am not sure
>>> if I should enable setting the retry count since it will change
>>> the behaviour even if users don't set anything.
>>>
>> Maybe I'm wrong, but I have in mind, that 7 retries is the default
>> setting of mac80211. Although 2 or even 4 seems to be pretty low for the
>> overall retry count, so maybe the values are somehow changed in the
>> firmware? From our experiments we know (at least for 802.11n) you need
>> for normal operation a retry count of something between 5 - 9, but
>> sometimes also 12 or 15 is beneficial.
>>
>> We use for our experimental setup mainly ath9k cards and rt28xx nics,
>> and with them you need definitely more retries.
>>
>> Nonetheless, I don't think the change from 2 to 4 does really affect the
>> behavior in a negative way (if it works as expected).
>>
>>> Any opinions on this?
>>>
>>> Thanks,
>>> Ben
>>>
>

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

RE: Setting tx retry count in ath10k

[Jean-Pierre TOSONI]

Hi,

We made retries configurable in our mac80211+ath9k system, and we ended with 3 counts:
1) short retry count, defaults to 4
2) long retrys count, defauts to 7
3) software retry count, defaults to 30
This last one is used separately for each frame in an aggregated frame, since they can be separately acknowledged.

Regards,
Jean-Pierre

> -----Message d'origine-----
> De : linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-owner@vger.kernel.org] De la part de
> Ben Greear
> Envoyé : mardi 17 juillet 2018 17:08
> À : SEDE; Benjamin Beichler; ath10k; linux-wireless@vger.kernel.org
> Objet : Re: Setting tx retry count in ath10k
> 
> 
> 
> On 07/17/2018 12:56 AM, SEDE wrote:
> > Hi,
> >
> > In the standard, 7 is the default for the short retry count, 4 is well the default for long retry
> count.
> >
> > In ath10k, there is also non_agg_sw_retry_th to control this, will this still be used?
> > Or what is the difference with rc_num_retries?
> >
> > Kind regards,
> > Sébastien.
> 
> The ath10k firmware has no idea of long vs short retries, so I just used the
> long setting.
> 
> I will investigate that non_agg_sw_retry_th as well, and I did notice my wave-1
> firmware (at least) uses 15 retries for self-generated frames.  But, in my case,
> those self-gen frames are not much used anyway since I disable firmware keep-alive.
> 
> And, I need to see how the mac80211 stack handles its own retries when working
> with ath10k.
> 
> Thanks,
> Ben
> 
> >
> >
> > On 2018-07-17 09:39, Benjamin Beichler wrote:
> >> Hi,
> >>
> >> Am 17.07.2018 um 02:37 schrieb Ben Greear:
> >>> I spent a bit of time looking into setting the tx retry count in
> >>> ath10k (wave-1 firmware).  The firmware has support for setting this as
> >>> a vdev parameter, and it defaults to '2', at least in my wave-1 firmware.
> >>>
> >>> I enabled propagating the setting from mac80211, ie:
> >>> iw phy wiphy0 set retry short 2 long 2
> >>>
> >>> And while debugging this, I noticed that mac80211 has a default of
> >>> 4, but the ath10k firmware has a default of 2.  Now, I am not sure
> >>> if I should enable setting the retry count since it will change
> >>> the behaviour even if users don't set anything.
> >>>
> >> Maybe I'm wrong, but I have in mind, that 7 retries is the default
> >> setting of mac80211. Although 2 or even 4 seems to be pretty low for the
> >> overall retry count, so maybe the values are somehow changed in the
> >> firmware? From our experiments we know (at least for 802.11n) you need
> >> for normal operation a retry count of something between 5 - 9, but
> >> sometimes also 12 or 15 is beneficial.
> >>
> >> We use for our experimental setup mainly ath9k cards and rt28xx nics,
> >> and with them you need definitely more retries.
> >>
> >> Nonetheless, I don't think the change from 2 to 4 does really affect the
> >> behavior in a negative way (if it works as expected).
> >>
> >>> Any opinions on this?
> >>>
> >>> Thanks,
> >>> Ben
> >>>
> >
> 
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: Setting tx retry count in ath10k

[Ben Greear]

On 07/18/2018 08:50 AM, Jean-Pierre TOSONI wrote:
> Hi,
>
> We made retries configurable in our mac80211+ath9k system, and we ended with 3 counts:
> 1) short retry count, defaults to 4
> 2) long retrys count, defauts to 7
> 3) software retry count, defaults to 30
> This last one is used separately for each frame in an aggregated frame, since they can be separately acknowledged.

Did you have to change code for #3, and if so, can you share the patch?

I wonder also if retries should be different for different types of data.  For instance,
if someone is using UDP, maybe they don't care so much about lost packets and would
prefer a lower retry count.  Or, maybe IP type-of-service could be taken into account
and retry frames different amounts based on ToS?

Thanks,
Ben

>
> Regards,
> Jean-Pierre
>
>> -----Message d'origine-----
>> De : linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-owner@vger.kernel.org] De la part de
>> Ben Greear
>> Envoyé : mardi 17 juillet 2018 17:08
>> À : SEDE; Benjamin Beichler; ath10k; linux-wireless@vger.kernel.org
>> Objet : Re: Setting tx retry count in ath10k
>>
>>
>>
>> On 07/17/2018 12:56 AM, SEDE wrote:
>>> Hi,
>>>
>>> In the standard, 7 is the default for the short retry count, 4 is well the default for long retry
>> count.
>>>
>>> In ath10k, there is also non_agg_sw_retry_th to control this, will this still be used?
>>> Or what is the difference with rc_num_retries?
>>>
>>> Kind regards,
>>> Sébastien.
>>
>> The ath10k firmware has no idea of long vs short retries, so I just used the
>> long setting.
>>
>> I will investigate that non_agg_sw_retry_th as well, and I did notice my wave-1
>> firmware (at least) uses 15 retries for self-generated frames.  But, in my case,
>> those self-gen frames are not much used anyway since I disable firmware keep-alive.
>>
>> And, I need to see how the mac80211 stack handles its own retries when working
>> with ath10k.
>>
>> Thanks,
>> Ben
>>
>>>
>>>
>>> On 2018-07-17 09:39, Benjamin Beichler wrote:
>>>> Hi,
>>>>
>>>> Am 17.07.2018 um 02:37 schrieb Ben Greear:
>>>>> I spent a bit of time looking into setting the tx retry count in
>>>>> ath10k (wave-1 firmware).  The firmware has support for setting this as
>>>>> a vdev parameter, and it defaults to '2', at least in my wave-1 firmware.
>>>>>
>>>>> I enabled propagating the setting from mac80211, ie:
>>>>> iw phy wiphy0 set retry short 2 long 2
>>>>>
>>>>> And while debugging this, I noticed that mac80211 has a default of
>>>>> 4, but the ath10k firmware has a default of 2.  Now, I am not sure
>>>>> if I should enable setting the retry count since it will change
>>>>> the behaviour even if users don't set anything.
>>>>>
>>>> Maybe I'm wrong, but I have in mind, that 7 retries is the default
>>>> setting of mac80211. Although 2 or even 4 seems to be pretty low for the
>>>> overall retry count, so maybe the values are somehow changed in the
>>>> firmware? From our experiments we know (at least for 802.11n) you need
>>>> for normal operation a retry count of something between 5 - 9, but
>>>> sometimes also 12 or 15 is beneficial.
>>>>
>>>> We use for our experimental setup mainly ath9k cards and rt28xx nics,
>>>> and with them you need definitely more retries.
>>>>
>>>> Nonetheless, I don't think the change from 2 to 4 does really affect the
>>>> behavior in a negative way (if it works as expected).
>>>>
>>>>> Any opinions on this?
>>>>>
>>>>> Thanks,
>>>>> Ben
>>>>>
>>>
>>
>> --
>> Ben Greear <greearb@candelatech.com>
>> Candela Technologies Inc  http://www.candelatech.com


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: Setting tx retry count in ath10k

[Toke Høiland-Jørgensen]

Ben Greear <greearb@candelatech.com> writes:

> On 07/18/2018 08:50 AM, Jean-Pierre TOSONI wrote:
>> Hi,
>>
>> We made retries configurable in our mac80211+ath9k system, and we ended with 3 counts:
>> 1) short retry count, defaults to 4
>> 2) long retrys count, defauts to 7
>> 3) software retry count, defaults to 30
>> This last one is used separately for each frame in an aggregated frame, since they can be separately acknowledged.
>
> Did you have to change code for #3, and if so, can you share the patch?
>
> I wonder also if retries should be different for different types of
> data. For instance, if someone is using UDP, maybe they don't care so
> much about lost packets and would prefer a lower retry count. Or,
> maybe IP type-of-service could be taken into account and retry frames
> different amounts based on ToS?

For general internet traffic, a retry count of 30 is way too high; that
is up to 120 ms of HOL blocking latency. Better to just drop the packet
at that point.

Ideally, the retry count should be dynamically calculated in units of
time (which would depend on the rate and aggregate size), and also take
queueing time into account. I've been meaning to experiment with this
for minstrel and ath9k, but haven't gotten around to it...

-Toke

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

RE: Setting tx retry count in ath10k

[Jean-Pierre TOSONI]

[-- Attachment #1: Type: text/plain, Size: 2677 bytes --]

Hi Ben,

I attached the patch. Please remind that it applies to ath9k.

At the end there are 3 comments in French, translation follows:
1) " longretry gives directly the transmit count, the +1 is useless.
     Should rather have -1 to account for the first try"
2) " The retries just made for this frame must be added in to know
     If the max was overstepped"
3) " Add the 1st try (probably useless).
     Upstream version adds 1 to A-MPDU retries but I don't understand why.
     Since above I removed the +1 to fix the count, I add +1 here once per
     frame in case the "+1" is actually hiding a bug in the upstream version"

@Toke: As you can see in the patch, the value 30 was the fixed value defined
 in ath9k, I kept it for compatibility only (and that's why I wanted to make
 it configurable :-)
On another hand, Minstrel in mac80211 seems to vary retries according to what
you say, i.e. Minstrel tries to stay below a certain amount of time, but this
only applies to short/long retries.

Jean-Pierre

> -----Message d'origine-----
> De : Toke Høiland-Jørgensen [mailto:toke@toke.dk]
> Envoyé : mercredi 18 juillet 2018 18:22
> À : Ben Greear; Jean-Pierre TOSONI; SEDE; Benjamin Beichler; ath10k; linux-wireless@vger.kernel.org
> Objet : Re: Setting tx retry count in ath10k
> 
> Ben Greear <greearb@candelatech.com> writes:
> 
> > On 07/18/2018 08:50 AM, Jean-Pierre TOSONI wrote:
> >> Hi,
> >>
> >> We made retries configurable in our mac80211+ath9k system, and we ended with 3 counts:
> >> 1) short retry count, defaults to 4
> >> 2) long retrys count, defauts to 7
> >> 3) software retry count, defaults to 30
> >> This last one is used separately for each frame in an aggregated frame, since they can be
> separately acknowledged.
> >
> > Did you have to change code for #3, and if so, can you share the patch?
> >
> > I wonder also if retries should be different for different types of
> > data. For instance, if someone is using UDP, maybe they don't care so
> > much about lost packets and would prefer a lower retry count. Or,
> > maybe IP type-of-service could be taken into account and retry frames
> > different amounts based on ToS?
> 
> For general internet traffic, a retry count of 30 is way too high; that
> is up to 120 ms of HOL blocking latency. Better to just drop the packet
> at that point.
> 
> Ideally, the retry count should be dynamically calculated in units of
> time (which would depend on the rate and aggregate size), and also take
> queueing time into account. I've been meaning to experiment with this
> for minstrel and ath9k, but haven't gotten around to it...
> 
> -Toke

[-- Attachment #2: a919-acksys-Add-parameter-for-software-retry.patch --]
[-- Type: application/octet-stream, Size: 4644 bytes --]

From 3b614d719a6fb806fef7fc560841e6c93e812753 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20DEBARGE?= <cedric.debarge@acksys.fr>
Date: Thu, 5 Jun 2014 12:13:05 +0200
Subject: [PATCH 2/2] Add parameter for software retry

When the aggregate frame are sent, the ATH9K manage a sofware retries.
The maximum sofware retries is set to ATH9K_SW_MAX_RETRIES (30)
constant.

Add max_sw_retries field in ath_softc structure.
Set the default value to ATH9K_SW_MAX_RETRIES (to keep backward
compatibility).
Add entry in debugfs to configure this new value.
---
 drivers/net/wireless/ath/ath9k/ath9k.h |    1 +
 drivers/net/wireless/ath/ath9k/debug.c |   41 ++++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/init.c  |    1 +
 drivers/net/wireless/ath/ath9k/xmit.c  |    2 +-
 4 files changed, 44 insertions(+), 1 deletion(-)

--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -990,6 +990,7 @@ struct ath_softc {
 
 	u8 gtt_cnt;
 	u32 intrstatus;
+	u8 max_sw_retries;
 	u16 ps_flags; /* PS_* */
 	bool ps_enabled;
 	bool ps_idle;
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1296,6 +1296,45 @@ void ath9k_get_et_stats(struct ieee80211
 	WARN_ON(i != ATH9K_SSTATS_LEN);
 }
 
+static ssize_t read_file_max_sw_retries(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, "%d\n", sc->max_sw_retries);
+	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_max_sw_retries(struct file *file, const char __user *user_buf,
+			     size_t count, loff_t *ppos)
+{
+	struct ath_softc *sc = file->private_data;
+	unsigned long max_sw_retries;
+	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 (kstrtoul(buf, 0, &max_sw_retries))
+		return -EINVAL;
+
+	sc->max_sw_retries = max_sw_retries;
+	return count;
+}
+
+static const struct file_operations fops_max_sw_retries = {
+	.read = read_file_max_sw_retries,
+	.write = write_max_sw_retries,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 void ath9k_deinit_debug(struct ath_softc *sc)
 {
 	ath9k_cmn_spectral_deinit_debug(&sc->spec_priv);
@@ -1550,6 +1589,8 @@ int ath9k_init_debug(struct ath_hw *ah)
 			  &ah->rxchainmask);
 	debugfs_create_u8("tx_chainmask", S_IRUSR, sc->debug.debugfs_phy,
 			  &ah->txchainmask);
+	debugfs_create_file("max_sw_retries", S_IRUSR | S_IWUSR,
+			    sc->debug.debugfs_phy, sc, &fops_max_sw_retries);
 	debugfs_create_file("ani", S_IRUSR | S_IWUSR,
 			    sc->debug.debugfs_phy, sc, &fops_ani);
 	debugfs_create_bool("paprd", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy,
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -383,6 +383,7 @@ static void ath9k_init_misc(struct ath_s
 	sc->spec_priv.spec_config.endless = false;
 	sc->spec_priv.spec_config.period = 0xFF;
 	sc->spec_priv.spec_config.fft_period = 0xF;
+	sc->max_sw_retries = ATH_MAX_SW_RETRIES;
 }
 
 static void ath9k_init_pcoem_platform(struct ath_softc *sc)
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -471,7 +471,9 @@ static void ath_tx_complete_aggr(struct
 
 	memcpy(rates, bf->rates, sizeof(rates));
 
-	retries = ts->ts_longretry + 1;
+	/* longretry donne directement le nombre de transmissions, le +1 est
+	 * inutile. On devrait plutot avoir -1 pour enlever le 1er try. */
+	retries = ts->ts_longretry;
 	for (i = 0; i < ts->ts_rateindex; i++)
 		retries += rates[i].count;
 
@@ -560,10 +562,22 @@ static void ath_tx_complete_aggr(struct
 			acked_cnt++;
 		} else if (flush) {
 			txpending = 1;
-		} else if (fi->retries < ATH_MAX_SW_RETRIES) {
+		} else if (fi->retries+retries < sc->max_sw_retries) {
+			/* Les retries qui viennent d'être faits sur cette
+			 * trame doivent être ajoutés pour savoir si on a
+			 * dépassé le max */
+			/*
+			 * Ajouter le 1er essai (probablement inutile).
+			 * La version upstream ajoute 1 aux retries du A-MPDU
+			 * mais je ne comprends pas pourquoi.
+			 * Comme ci-dessus j'ai retiré le +1 pour corriger le
+			 * comptage, j'ajoute +1 une seule fois par trame au cas
+			 * ou le +1 cacherait un bug dans la version upstream.
+			 */
+			int try = !fi->retries; /* 1 si 1ere fois, 0 sinon */
 			if (txok || !an->sleeping)
 				ath_tx_set_retry(sc, txq, bf->bf_mpdu,
-						 retries);
+						 try + retries);
 
 			txpending = 1;
 		} else {

[-- Attachment #3: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

RE: Setting tx retry count in ath10k

[Toke Høiland-Jørgensen]

Jean-Pierre TOSONI <jp.tosoni@acksys.fr> writes:

> @Toke: As you can see in the patch, the value 30 was the fixed value
> defined in ath9k, I kept it for compatibility only (and that's why I
> wanted to make it configurable :-)

Yup, I'm aware that this is the default from ath9k; doesn't make it any
less wrong ;)

> On another hand, Minstrel in mac80211 seems to vary retries according
> to what you say, i.e. Minstrel tries to stay below a certain amount of
> time, but this only applies to short/long retries.

Right, haven't had time to poke into what exactly Minstrel does; have
just observed that it sometimes runs really long. But for most cases
it's not too bad...

-Toke

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: Setting tx retry count in ath10k

[Benjamin Beichler]

Am 18.07.2018 um 19:01 schrieb Jean-Pierre TOSONI:
> Hi Ben,
>
> I attached the patch. Please remind that it applies to ath9k.
>
> At the end there are 3 comments in French, translation follows:
> 1) " longretry gives directly the transmit count, the +1 is useless.
>      Should rather have -1 to account for the first try"
> 2) " The retries just made for this frame must be added in to know
>      If the max was overstepped"
> 3) " Add the 1st try (probably useless).
>      Upstream version adds 1 to A-MPDU retries but I don't understand why.
>      Since above I removed the +1 to fix the count, I add +1 here once per
>      frame in case the "+1" is actually hiding a bug in the upstream version"
>
> @Toke: As you can see in the patch, the value 30 was the fixed value defined
>  in ath9k, I kept it for compatibility only (and that's why I wanted to make
>  it configurable :-)
> On another hand, Minstrel in mac80211 seems to vary retries according to what
> you say, i.e. Minstrel tries to stay below a certain amount of time, but this
> only applies to short/long retries.
>
> Jean-Pierre
We also experienced the problems regarding the inconsistencies between
documentation and implementation. I will further throw in another set of
patches:
https://github.com/steinwurf/openwrt-patches/tree/master/openwrt-r41097/mac80211

I think they have have similarities but also other aspects to really
restrict ath9k to the specified retry limits. This problem is as Toke
already stated additionally depended on minstrel. It is really sad, that
all this stuff is offtree and but there is the illusion of an setting in
the mainline kernel, which firstly will not set the value as documented
and different drivers will apply them (without warnings) in different ways.

>> For general internet traffic, a retry count of 30 is way too high; that
>> is up to 120 ms of HOL blocking latency. Better to just drop the packet
>> at that point.
>>
>> Ideally, the retry count should be dynamically calculated in units of
>> time (which would depend on the rate and aggregate size), and also take
>> queueing time into account. I've been meaning to experiment with this
>> for minstrel and ath9k, but haven't gotten around to it...
We have running current research on this topic but focused on the
effects in 802.11s mesh networks. With multiple(forwarding) wireless
links, the retry limit have a bigger impact as only in managed mode
setups, since every forwarding step is doing repeated transmissions. But
I also totally agree, that the retry count needs to be considered in the
bufferbloat/airtime queuing stuff, which Toke proposed.

Nonetheless, since this ambiguities are known, wouldn't it be nice to
clean up all this patches to enable at least ath9k and ath10k to do the
right thing, or do anybody can tell why they weren't included the first
time ?


-- 
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: Benjamin.Beichler@uni-rostock.de
www: http://www.imd.uni-rostock.de/




_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: Setting tx retry count in ath10k

[Ben Greear]

On 07/19/2018 05:39 AM, Benjamin Beichler wrote:
> Am 18.07.2018 um 19:01 schrieb Jean-Pierre TOSONI:
>> Hi Ben,
>>
>> I attached the patch. Please remind that it applies to ath9k.
>>
>> At the end there are 3 comments in French, translation follows:
>> 1) " longretry gives directly the transmit count, the +1 is useless.
>>      Should rather have -1 to account for the first try"
>> 2) " The retries just made for this frame must be added in to know
>>      If the max was overstepped"
>> 3) " Add the 1st try (probably useless).
>>      Upstream version adds 1 to A-MPDU retries but I don't understand why.
>>      Since above I removed the +1 to fix the count, I add +1 here once per
>>      frame in case the "+1" is actually hiding a bug in the upstream version"
>>
>> @Toke: As you can see in the patch, the value 30 was the fixed value defined
>>  in ath9k, I kept it for compatibility only (and that's why I wanted to make
>>  it configurable :-)
>> On another hand, Minstrel in mac80211 seems to vary retries according to what
>> you say, i.e. Minstrel tries to stay below a certain amount of time, but this
>> only applies to short/long retries.
>>
>> Jean-Pierre
> We also experienced the problems regarding the inconsistencies between
> documentation and implementation. I will further throw in another set of
> patches:
> https://github.com/steinwurf/openwrt-patches/tree/master/openwrt-r41097/mac80211
>
> I think they have have similarities but also other aspects to really
> restrict ath9k to the specified retry limits. This problem is as Toke
> already stated additionally depended on minstrel. It is really sad, that
> all this stuff is offtree and but there is the illusion of an setting in
> the mainline kernel, which firstly will not set the value as documented
> and different drivers will apply them (without warnings) in different ways.
>
>>> For general internet traffic, a retry count of 30 is way too high; that
>>> is up to 120 ms of HOL blocking latency. Better to just drop the packet
>>> at that point.
>>>
>>> Ideally, the retry count should be dynamically calculated in units of
>>> time (which would depend on the rate and aggregate size), and also take
>>> queueing time into account. I've been meaning to experiment with this
>>> for minstrel and ath9k, but haven't gotten around to it...
> We have running current research on this topic but focused on the
> effects in 802.11s mesh networks. With multiple(forwarding) wireless
> links, the retry limit have a bigger impact as only in managed mode
> setups, since every forwarding step is doing repeated transmissions. But
> I also totally agree, that the retry count needs to be considered in the
> bufferbloat/airtime queuing stuff, which Toke proposed.
>
> Nonetheless, since this ambiguities are known, wouldn't it be nice to
> clean up all this patches to enable at least ath9k and ath10k to do the
> right thing, or do anybody can tell why they weren't included the first
> time ?

To change the topic slightly, has anyone even verified that ath10k
will do hardware (or firmware) retransmits for aggregated frames when doing block-ack?

I was expecting to find re-queue logic in the firmware when I looked the
other day, but I only saw it do re-queues for locally generated frames,
not stuff sent from the driver.  I may have just misread the code, however,
or maybe the hardware itself somehow magically retransmits dropped
aggregated frames based on the block-ack packets?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: Setting tx retry count in ath10k

[Toke Høiland-Jørgensen]

Benjamin Beichler <Benjamin.Beichler@uni-rostock.de> writes:

>>> For general internet traffic, a retry count of 30 is way too high; that
>>> is up to 120 ms of HOL blocking latency. Better to just drop the packet
>>> at that point.
>>>
>>> Ideally, the retry count should be dynamically calculated in units of
>>> time (which would depend on the rate and aggregate size), and also take
>>> queueing time into account. I've been meaning to experiment with this
>>> for minstrel and ath9k, but haven't gotten around to it...
> We have running current research on this topic but focused on the
> effects in 802.11s mesh networks. With multiple(forwarding) wireless
> links, the retry limit have a bigger impact as only in managed mode
> setups, since every forwarding step is doing repeated transmissions.
> But I also totally agree, that the retry count needs to be considered
> in the bufferbloat/airtime queuing stuff, which Toke proposed.

Ah, cool. Looking forward to seeing your results. And yeah, it's
probably even worse in meshes...

> Nonetheless, since this ambiguities are known, wouldn't it be nice to
> clean up all this patches to enable at least ath9k and ath10k to do
> the right thing, or do anybody can tell why they weren't included the
> first time ?

No objection from me, certainly! :)

-Toke

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k