| Message ID | 1540230918-27712-1-git-send-email-tamizhr@codeaurora.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | cfg80211/mac80211: Add support for TID specific configuration | expand |
On Mon, 2018-10-22 at 23:25 +0530, 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. Not sure I understand this, how can you expect to control something on a per-packet basis using this? Sergey, looks like your A-MPDU control is already in here per RA/TID, and A-MSDU could be added easily? johannes
> On Mon, 2018-10-22 at 23:25 +0530, 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. > > Not sure I understand this, how can you expect to control something on a > per-packet basis using this? > > Sergey, looks like your A-MPDU control is already in here per RA/TID, > and A-MSDU could be added easily? Hello Johannes, Thanks for pointing me at this patch series. Indeed, it looks like an exact match for proper RA/TID aware implementation of AMPDU control. AMSDU can be added following the same approach. Regards, Sergey
Hi, On Tue, 2018-11-06 at 10:45 +0000, Sergey Matyukevich wrote: > > On Mon, 2018-10-22 at 23:25 +0530, 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. > > > > Not sure I understand this, how can you expect to control something on a > > per-packet basis using this? > > > > Sergey, looks like your A-MPDU control is already in here per RA/TID, > > and A-MSDU could be added easily? > Thanks for pointing me at this patch series. Indeed, it looks like an > exact match for proper RA/TID aware implementation of AMPDU control. > AMSDU can be added following the same approach. > Great. I guess if you could take a look that'd be nice, and perhaps also see if you could actually implement it? Your driver patch seemed to imply the firmware only has global control, rather than per RA/TID. Also, do you think A-MPDU length control would be something useful? Perhaps that should be instead of enable/disable (since setting length to 0 or 1 could easily mean "no A-MPDU") johannes
On Tue, Nov 06, 2018 at 12:28:11PM +0100, Johannes Berg wrote: > > WARNING: External email to Quantenna Communications. Please exercise caution! > > > Hi, > > On Tue, 2018-11-06 at 10:45 +0000, Sergey Matyukevich wrote: > > > On Mon, 2018-10-22 at 23:25 +0530, 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. > > > > > > Not sure I understand this, how can you expect to control something on a > > > per-packet basis using this? > > > > > > Sergey, looks like your A-MPDU control is already in here per RA/TID, > > > and A-MSDU could be added easily? > > > Thanks for pointing me at this patch series. Indeed, it looks like an > > exact match for proper RA/TID aware implementation of AMPDU control. > > AMSDU can be added following the same approach. > > > > Great. I guess if you could take a look that'd be nice, and perhaps also > see if you could actually implement it? Your driver patch seemed to > imply the firmware only has global control, rather than per RA/TID. > > Also, do you think A-MPDU length control would be something useful? > Perhaps that should be instead of enable/disable (since setting length > to 0 or 1 could easily mean "no A-MPDU") Hello Johannes, I will send a follow-up A-MSDU patch after this patch set lands in your tree. And then qtnfmac driver patches for both A-MPDU/A-MSDU changes. As for A-MPDU chain length control, I don't think we have any practical use-case for this feature at the moment. However Ben Greear suggested one possible use-case: to decrease A-MPDU chain length for voice TID in order to decrease latency. Anyways, current A-MPDU patches can be easily adapted to support it if needed. Regards, Sergey
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? 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 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.
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. >
Hello Tamizh, > 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. > > Tamizh chelvam (3): > nl80211: Add netlink attribute for AMPDU aggregation enable/disable > tid conf 3 > ath10k: Add support to configure TID specific configuration > > Vasanthakumar Thiagarajan (1): > New netlink command for TID specific configuration Could you please share your further plans regarding this patch series ? Do you plan to send next revision in the forseable future ? Or have you decided to abandon this approach ? Regards, Sergey
Hi Sergey, > Hello Tamizh, > >> 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. >> >> Tamizh chelvam (3): >> nl80211: Add netlink attribute for AMPDU aggregation enable/disable >> tid conf 3 >> ath10k: Add support to configure TID specific configuration >> >> Vasanthakumar Thiagarajan (1): >> New netlink command for TID specific configuration > > Could you please share your further plans regarding this patch series ? > Do you plan to send next revision in the forseable future ? > Or have you decided to abandon this approach ? > I'm working on new patchsets. I will send it in a week. Thanks, Tamizh.