All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Greear <greearb@candelatech.com>
To: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>,
	Tamizh chelvam <tamizhr@codeaurora.org>
Cc: "ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
	"johannes@sipsolutions.net" <johannes@sipsolutions.net>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
Subject: Re: [PATCH 0/4] cfg80211/mac80211: Add support for TID specific configuration
Date: Thu, 8 Nov 2018 08:31:42 -0800	[thread overview]
Message-ID: <fec37af9-5a34-ea82-e81e-54e983278b4f@candelatech.com> (raw)
In-Reply-To: <2008ad85-3f13-cc6b-3fc2-d614f8422d68@quantenna.com>



On 11/07/2018 04:55 PM, Igor Mitsyanko wrote:
> On 10/22/2018 10:55 AM, Tamizh chelvam wrote:
>>
>> Add infrastructure for per TID aggregation/retry count configurations
>> such as retry count and AMPDU aggregation control(disable/enable).
>> In some scenario reducing the number of retry count for a specific data
>> traffic can reduce the latency by proceeding with the next packet
>> instead of retrying the same packet more time. This will be useful
>> where the next packet can resume the operation without an issue.
>> Here added NL80211_CMD_SET_TID_CONFIG to support this operation by
>> accepting retry count and AMPDU aggregation control.
>> This command can accept STA mac addreess to make the configuration
>> station specific rather than applying to all the connected stations
>> to the netdev.
>>
>
> It's not immediately clear how to make use of these settings, here are
> several comments:
>
> 1. What max retry count limit should actually be applied to? Retries
> decisions are in a rate adaptation domain, it should know how many
> retries should be done on each rate, single "max retry" value will not
> suffice. For example, it can retry twice on MCS9, twice on MCS7, three
> times on MCS5 or something like that.
>
> I'm not familiar with what ATH10k is doing, 4th patch defines
> ATH10K_MAX_RETRY_COUNT=30, what does it actually mean? It's unlikely "do
> 30 retries on the same rate". Does retry limit setting interacts with
> rate adaptation somehow in ath10k?
>
> Maybe it makes sense to extend max retry value specification to make it
> possible to define per-rate? I'm not sure how much flexibility we want
> here, something like retry value per MCS:BW:SGI?

For ath10k, my understanding is that each time it (re)sends a packet, it will
query FW rate-ctrl and choose the optimal rate.  It doesn't pay much attention to
whether a specific frame is retried or not, other than to maybe enable RTS/CTS,
but lots of retries will bump the rate-ctrl down to a lower rate.

There are no per-rate retry counter logic, but I think there is per-tid
control, though currently it might not be wired up to the driver.

>
> 2. AMPDU/AMSDU - the way it is, it is also relevant to rate in Tx
> direction only, correct? We keep advertised capabilities intact and peer
> has all rights to send AMPDUs/AMSDUs of whatever size that was advertised.
> Additionally, posted patches do not do anything with established
> blockack agreement.
>
> 3 With above being said, perhaps it would make sense for this new
> interface to indicate explicitly that it's related to Tx rate? That can
> be controlled per-TID, per-node or globally, depending on device
> capabilities.
> Some other settings that may be useful are fixed MCS, MCS limit, SGI
> on/off, bandwidth, maybe even provide rate retry rules.

I think there should be a way to configure the advertised capabilities, and also a way to
configure the settings actually used for transmit.  This is what we use
for test-related use cases, but maybe there is not a great deal of general
use for this type of thing.  For general use, the 'transmit' settings are probably
more useful.  I do know that several ath10k users are forcing it back to /n
mode which works around some bugs in their mesh setup.

You can already set a fixed transmit rate or set the MCS rates allowed to be used
(my supplicant, ath10k-ct driver/firmware is needed to take full advantage of this
  for ath10k).  In upstream kernels, this will not much affect the advertised capabilities.

I also have patches that allow setting the advertised rates and capabilities, so you can force
a station to advertise only a/n rates even though it and peer have /AC capability.
Those patches are not upstream, though if opinions are changed, I'd be happy
to repost and try to get them upstream.

Thanks,
Ben

> I don't see how it can be used in real product, unless there is an
> external rate adaptation logic of some kind. But definitely it will be
> useful for testing, and can be used for WFA certification.
>

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

WARNING: multiple messages have this Message-ID (diff)
From: Ben Greear <greearb@candelatech.com>
To: Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>,
	Tamizh chelvam <tamizhr@codeaurora.org>
Cc: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>,
	"johannes@sipsolutions.net" <johannes@sipsolutions.net>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>
Subject: Re: [PATCH 0/4] cfg80211/mac80211: Add support for TID specific configuration
Date: Thu, 8 Nov 2018 08:31:42 -0800	[thread overview]
Message-ID: <fec37af9-5a34-ea82-e81e-54e983278b4f@candelatech.com> (raw)
In-Reply-To: <2008ad85-3f13-cc6b-3fc2-d614f8422d68@quantenna.com>



On 11/07/2018 04:55 PM, Igor Mitsyanko wrote:
> On 10/22/2018 10:55 AM, Tamizh chelvam wrote:
>>
>> Add infrastructure for per TID aggregation/retry count configurations
>> such as retry count and AMPDU aggregation control(disable/enable).
>> In some scenario reducing the number of retry count for a specific data
>> traffic can reduce the latency by proceeding with the next packet
>> instead of retrying the same packet more time. This will be useful
>> where the next packet can resume the operation without an issue.
>> Here added NL80211_CMD_SET_TID_CONFIG to support this operation by
>> accepting retry count and AMPDU aggregation control.
>> This command can accept STA mac addreess to make the configuration
>> station specific rather than applying to all the connected stations
>> to the netdev.
>>
>
> It's not immediately clear how to make use of these settings, here are
> several comments:
>
> 1. What max retry count limit should actually be applied to? Retries
> decisions are in a rate adaptation domain, it should know how many
> retries should be done on each rate, single "max retry" value will not
> suffice. For example, it can retry twice on MCS9, twice on MCS7, three
> times on MCS5 or something like that.
>
> I'm not familiar with what ATH10k is doing, 4th patch defines
> ATH10K_MAX_RETRY_COUNT=30, what does it actually mean? It's unlikely "do
> 30 retries on the same rate". Does retry limit setting interacts with
> rate adaptation somehow in ath10k?
>
> Maybe it makes sense to extend max retry value specification to make it
> possible to define per-rate? I'm not sure how much flexibility we want
> here, something like retry value per MCS:BW:SGI?

For ath10k, my understanding is that each time it (re)sends a packet, it will
query FW rate-ctrl and choose the optimal rate.  It doesn't pay much attention to
whether a specific frame is retried or not, other than to maybe enable RTS/CTS,
but lots of retries will bump the rate-ctrl down to a lower rate.

There are no per-rate retry counter logic, but I think there is per-tid
control, though currently it might not be wired up to the driver.

>
> 2. AMPDU/AMSDU - the way it is, it is also relevant to rate in Tx
> direction only, correct? We keep advertised capabilities intact and peer
> has all rights to send AMPDUs/AMSDUs of whatever size that was advertised.
> Additionally, posted patches do not do anything with established
> blockack agreement.
>
> 3 With above being said, perhaps it would make sense for this new
> interface to indicate explicitly that it's related to Tx rate? That can
> be controlled per-TID, per-node or globally, depending on device
> capabilities.
> Some other settings that may be useful are fixed MCS, MCS limit, SGI
> on/off, bandwidth, maybe even provide rate retry rules.

I think there should be a way to configure the advertised capabilities, and also a way to
configure the settings actually used for transmit.  This is what we use
for test-related use cases, but maybe there is not a great deal of general
use for this type of thing.  For general use, the 'transmit' settings are probably
more useful.  I do know that several ath10k users are forcing it back to /n
mode which works around some bugs in their mesh setup.

You can already set a fixed transmit rate or set the MCS rates allowed to be used
(my supplicant, ath10k-ct driver/firmware is needed to take full advantage of this
  for ath10k).  In upstream kernels, this will not much affect the advertised capabilities.

I also have patches that allow setting the advertised rates and capabilities, so you can force
a station to advertise only a/n rates even though it and peer have /AC capability.
Those patches are not upstream, though if opinions are changed, I'd be happy
to repost and try to get them upstream.

Thanks,
Ben

> I don't see how it can be used in real product, unless there is an
> external rate adaptation logic of some kind. But definitely it will be
> useful for testing, and can be used for WFA certification.
>

-- 
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

  reply	other threads:[~2018-11-08 16:32 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-22 17:55 [PATCH 0/4] cfg80211/mac80211: Add support for TID specific configuration Tamizh chelvam
2018-10-22 17:55 ` Tamizh chelvam
2018-10-22 17:55 ` [PATCH 1/4] New netlink command " Tamizh chelvam
2018-10-22 17:55   ` Tamizh chelvam
2018-11-06 10:16   ` Sergey Matyukevich
2018-11-06 10:16     ` Sergey Matyukevich
2018-11-08 17:29     ` Tamizh chelvam
2018-11-08 17:29       ` Tamizh chelvam
2018-11-09  9:47       ` Sergey Matyukevich
2018-11-09  9:47         ` Sergey Matyukevich
2018-11-06 11:31   ` Johannes Berg
2018-11-06 11:31     ` Johannes Berg
2018-11-07 14:05   ` Sergey Matyukevich
2018-11-07 14:05     ` Sergey Matyukevich
2018-11-08 17:47     ` Tamizh chelvam
2018-11-08 17:47       ` Tamizh chelvam
2018-11-09  9:40       ` Sergey Matyukevich
2018-11-09  9:40         ` Sergey Matyukevich
2018-11-09 12:24         ` Johannes Berg
2018-11-09 12:24           ` Johannes Berg
2018-11-13  6:46           ` Tamizh chelvam
2018-11-13  6:46             ` Tamizh chelvam
2018-11-09 11:37   ` Johannes Berg
2018-11-09 11:37     ` Johannes Berg
2018-10-22 17:55 ` [PATCH 2/4] nl80211: Add netlink attribute for AMPDU aggregation enable/disable Tamizh chelvam
2018-10-22 17:55   ` Tamizh chelvam
2018-11-06 10:21   ` Sergey Matyukevich
2018-11-06 10:21     ` Sergey Matyukevich
2018-11-08 12:28     ` Tamizh chelvam
2018-11-08 12:28       ` Tamizh chelvam
2018-10-22 17:55 ` [PATCH 3/4] mac80211: Add api to support configuring TID specific configuration Tamizh chelvam
2018-10-22 17:55   ` Tamizh chelvam
2018-11-06 10:33   ` Sergey Matyukevich
2018-11-06 10:33     ` Sergey Matyukevich
2018-11-08 12:42     ` Tamizh chelvam
2018-11-08 12:42       ` Tamizh chelvam
2018-11-07 23:55   ` Igor Mitsyanko
2018-11-07 23:55     ` Igor Mitsyanko
2018-10-22 17:55 ` [PATCH 4/4] ath10k: Add support to configure " Tamizh chelvam
2018-10-22 17:55   ` Tamizh chelvam
2018-11-09  9:26   ` Sergey Matyukevich
2018-11-09  9:26     ` Sergey Matyukevich
2018-11-05 20:43 ` [PATCH 0/4] cfg80211/mac80211: Add support for " Johannes Berg
2018-11-05 20:43   ` Johannes Berg
2018-11-06 10:45   ` Sergey Matyukevich
2018-11-06 10:45     ` Sergey Matyukevich
2018-11-06 11:28     ` Johannes Berg
2018-11-06 11:28       ` Johannes Berg
2018-11-06 12:17       ` Sergey Matyukevich
2018-11-06 12:17         ` Sergey Matyukevich
2018-11-08  0:55 ` Igor Mitsyanko
2018-11-08  0:55   ` Igor Mitsyanko
2018-11-08 16:31   ` Ben Greear [this message]
2018-11-08 16:31     ` Ben Greear
2019-02-13 19:01 ` Sergey Matyukevich
2019-02-13 19:01   ` Sergey Matyukevich
     [not found]   ` <a7d97b692da245a8b85769d7766ceba7@aphydexm01f.ap.qualcomm.com>
2019-02-14  7:14     ` Tamizh chelvam
2019-02-14  7:14       ` Tamizh chelvam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fec37af9-5a34-ea82-e81e-54e983278b4f@candelatech.com \
    --to=greearb@candelatech.com \
    --cc=ath10k@lists.infradead.org \
    --cc=igor.mitsyanko.os@quantenna.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=sergey.matyukevich.os@quantenna.com \
    --cc=tamizhr@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.