From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]) by merlin.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fwlxR-0007B9-PV for ath10k@lists.infradead.org; Mon, 03 Sep 2018 10:19:46 +0000 MIME-Version: 1.0 Date: Mon, 03 Sep 2018 15:49:32 +0530 From: Anilkumar Kolli Subject: Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status() In-Reply-To: <87r2ia28ii.fsf@toke.dk> References: <1524291786-30850-1-git-send-email-akolli@codeaurora.org> <87k1o6d8hw.fsf@kamboji.qca.qualcomm.com> <87zhx21uqc.fsf@toke.dk> <87r2ia28ii.fsf@toke.dk> Message-ID: <01897c3dab4aa7c2371ef6791487413b@codeaurora.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: base64 Content-Type: text/plain; charset="utf-8"; Format="flowed" Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: =?UTF-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= Cc: linux-wireless-owner@vger.kernel.org, Johannes Berg , linux-wireless@vger.kernel.org, ath10k@lists.infradead.org, Kalle Valo T24gMjAxOC0wOS0wMyAxNTo0MywgVG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2VuIHdyb3RlOgo+IEFu aWxrdW1hciBLb2xsaSA8YWtvbGxpQGNvZGVhdXJvcmEub3JnPiB3cml0ZXM6Cj4gCj4+IE9uIDIw MTgtMDgtMzEgMTk6NTIsIFRva2UgSMO4aWxhbmQtSsO4cmdlbnNlbiB3cm90ZToKPj4+IEthbGxl IFZhbG8gPGt2YWxvQGNvZGVhdXJvcmEub3JnPiB3cml0ZXM6Cj4+PiAKPj4+PiBBbmlsa3VtYXIg S29sbGkgPGFrb2xsaUBjb2RlYXVyb3JhLm9yZz4gd3JpdGVzOgo+Pj4+IAo+Pj4+PiBNZXNoIHBh dGggbWV0cmljIG5lZWRzIHR4cmF0ZSBpbmZvcm1hdGlvbiBmcm9tIAo+Pj4+PiBpZWVlODAyMTFf dHhfc3RhdHVzKCkKPj4+Pj4gY2FsbCBidXQgaW4gYXRoMTBrIHRoZXJlIGlzIG5vIG1lY2hhbmlz bSB0byByZXBvcnQgdHggcmF0ZQo+Pj4+PiBpbmZvcm1hdGlvbgo+Pj4+PiB2aWEgaWVlZTgwMjEx X3R4X3N0YXR1cygpLCB0aGUgcmF0ZSBpcyBvbmx5IGFjY2Vzc2libGUgdmlhCj4+Pj4+IHN0YV9z dGF0aXNjcygpIG9wLgo+Pj4+PiAKPj4+Pj4gUGVyIHBlZXIgc3RhdHMgaGFzIHR4IHJhdGUgaW5m byBhdmFpbGFibGUsIHRoaXMgcGF0Y2ggc3RvcmVzIHBlciAKPj4+Pj4gcGVlcgo+Pj4+PiBsYXN0 IHR4IHJhdGUgYW5kIHVwZGF0ZXMgdGhlIHR4IHJhdGUgaW5mbyBzdHJ1Y3R1cmVzIGluIHR4Cj4+ Pj4+IGNvbXBsZXRpdGlvbi4KPj4+Pj4gVGhlIHJhdGUgdXBkYXRlZCBpbiBpZWVlODAyMTFfdHhf c3RhdHVzKCkgaXMgbm90IGV4YWN0bHkgZm9yIHRoZSAKPj4+Pj4gbGFzdAo+Pj4+PiB0cmFuc21p dHRlZCBmcmFtZSBpbnN0ZWFkIHRoZSByYXRlIGlzIGZyb20gb25lIG9mIHRoZSBwcmV2aW91cwo+ Pj4+PiBmcmFtZXMuCj4+Pj4+IAo+Pj4+PiBQZXIgcGVlciB0eHJhdGUgaW5mb3JtYXRpb24gaXMg dXBkYXRlZCB0aHJvdWdoIHBlciBwZWVyIHN0YXRpc3RpY3MKPj4+Pj4gYW5kIGlzIGF2YWlsYWJs ZSBmb3IgUUNBOTg4OC9RQ0E5OTg0L1FDQTQwMTkvUUNBOTk4WCBvbmx5Cj4+Pj4+IAo+Pj4+PiBU ZXN0ZWQgb24gUUNBOTk4NCB3aXRoIGZpcm13YXJlLTUuYmluXzEwLjQtMy41LjMtMDAwNTMKPj4+ Pj4gVGVzdGVkIG9uIFFDQTk5OFggd2l0aCBmaXJtd2FyZS01LmJpbl8xMC4yLjQtMS4wLTAwMDM2 Cj4+Pj4+IAo+Pj4+PiBTaWduZWQtb2ZmLWJ5OiBBbmlsa3VtYXIgS29sbGkgPGFrb2xsaUBjb2Rl YXVyb3JhLm9yZz4KPj4+PiAKPj4+PiBUaGlzIGlzIGEgcGF0Y2ggZnJvbSBsYXN0IE1hcmNoLCBm dWxsIHBhdGNoIGhlcmU6Cj4+Pj4gCj4+Pj4gaHR0cHM6Ly9wYXRjaHdvcmsua2VybmVsLm9yZy9w YXRjaC8xMDI2NzY5My8KPj4+PiAKPj4+Pj4gQEAgLTExOSw2ICsxMjIsMTggQEAgaW50IGF0aDEw a190eHJ4X3R4X3VucmVmKHN0cnVjdCBhdGgxMGtfaHR0IAo+Pj4+PiAqaHR0LAo+Pj4+PiAgCQkJ aW5mby0+ZmxhZ3MgJj0gfklFRUU4MDIxMV9UWF9TVEFUX0FDSzsKPj4+Pj4gIAl9Cj4+Pj4+IAo+ Pj4+PiArCWlmIChzdGEpIHsKPj4+Pj4gKwkJc3Bpbl9sb2NrX2JoKCZhci0+ZGF0YV9sb2NrKTsK Pj4+Pj4gKwkJYXJzdGEgPSAoc3RydWN0IGF0aDEwa19zdGEgKilzdGEtPmRydl9wcml2Owo+Pj4+ PiArCQlpbmZvLT5zdGF0dXMucmF0ZXNbMF0uaWR4ID0KPj4+Pj4gKwkJCWFyc3RhLT50eF9pbmZv LnN0YXR1cy5yYXRlc1swXS5pZHg7Cj4+Pj4+ICsJCWluZm8tPnN0YXR1cy5yYXRlc1swXS5jb3Vu dCA9Cj4+Pj4+ICsJCQlhcnN0YS0+dHhfaW5mby5zdGF0dXMucmF0ZXNbMF0uY291bnQ7Cj4+Pj4+ ICsJCWluZm8tPnN0YXR1cy5yYXRlc1swXS5mbGFncyA9Cj4+Pj4gPD4gKwkJCWFyc3RhLT50eF9p bmZvLnN0YXR1cy5yYXRlc1swXS5mbGFnczsKPj4+Pj4gKwkJc3Bpbl91bmxvY2tfYmgoJmFyLT5k YXRhX2xvY2spOwo+Pj4+PiArCX0KPj4+Pj4gKwo+Pj4+PiAgCWllZWU4MDIxMV90eF9zdGF0dXMo aHR0LT5hci0+aHcsIG1zZHUpOwo+Pj4+PiAgCS8qIHdlIGRvIG5vdCBvd24gdGhlIG1zZHUgYW55 bW9yZSAqLwo+Pj4+IAo+Pj4+ICJpcyBub3QgZXhhY3RseSIgaXMgSU1ITyBhbiB1bmRlcnN0YXRl bWVudC4gV2hhdCBpdCBtZWFucyB0aGF0IHdpdGgKPj4+PiB0aGlzCj4+Pj4gcGF0Y2ggYXRoMTBr IHJlcG9ydHMgdGhlIHJhdGUgaW5mb3JtYXRpb24gZnJvbSBhbm90aGVyIGZyYW1lIGluc3RlYWQK Pj4+PiBvZgo+Pj4+IHRoZSBjdXJyZW50IHNrYiwgYmVjYXVzZSB0aGUgZmlybXdhcmUgcHJvdmlk ZXMgdGhlIHJhdGUgaW5mb3JtYXRpb24KPj4+PiAib3V0Cj4+Pj4gb2YgYmFuZCIuIEEgc2ltcGxl IGV4YW1wbGUgdG8gY2xhcmlmeToKPj4+PiAKPj4+PiAgICBMZXQncyBzYXkgYXRoMTBrIHRyYW5z bWl0cyBmcmFtZXMgQSwgQiBhbmQgQy4gVGhlbiBhdGgxMGsgY2FsbHMKPj4+PiAgICBpZWVlODAy MTFfdHhfc3RhdHVzKCkgZm9yIGZyYW1lIEMgdGhlIHJhdGUgaW5mb3JtYXRpb24gY291bGQgYmUg Cj4+Pj4gZm9yCj4+Pj4gICAgZnJhbWUgQSwgb3IgaXQgY291bGQgYmUgdGhlIG90aGVyIGFyb3Vu ZCBmb3IgZnJhbWUgQSBzdGF0dXMgdGhlCj4+Pj4gcmF0ZQo+Pj4+ICAgIGluZm9ybWF0aW9uIGlz IGZyb20gZnJhbWUgQy4KPj4+PiAKPj4+PiBJbiBvdGhlciB3b3JkcywgdGhlcmUncyBubyBndWFy YW50ZWUgZnJvbSB3aGF0IGZyYW1lIHRoZSByYXRlCj4+Pj4gaW5mb3JtYXRpb24gaXMgZnJvbS4K Pj4+PiAKPj4+PiBUb28gbWUgdGhpcyBmZWVscyBsaWtlIGEgYmFkIGlkZWEgYnV0IEknbSBub3Qg ZmFtaWxpYXIgZW5vdWdoIHdpdGgKPj4+PiBtYWM4MDIxMSB0byByZWFsbHkgY29tbWVudCBvbiB0 aGlzLiBXaGF0IGtpbmQgb2YgaW1wbGljYXRpb25zIGRvZXMKPj4+PiB0aGlzCj4+Pj4gaGF2ZSBm b3IgTWVzaCBvciBBVEYsIGZvciBleGFtcGxlPyBBZGRpbmcgSm9oYW5uZXMgYW5kIFRva2UgdG8g aGVhcgo+Pj4+IGFib3V0IHRoZWlyIG9waW5pb24gYWJvdXQgdGhpcy4KPj4+IAo+Pj4gSSB3YXMg dW5kZXIgdGhlIGltcHJlc3Npb24gdGhhdCB0aGUgdmFsdWVzIGdhdGhlcmVkIChhdCBsZWFzdCBm b3IKPj4+IGFpcnRpbWUpIHdvdWxkIGJlIGN1bXVsYXRpdmUgdmFsdWVzPyBJZiBpdCdzIGp1c3Qg dGhlIGFpcnRpbWUgb2YgYQo+Pj4gc2luZ2xlIHJhbmRvbSBmcmFtZSwgd2hpY2ggaXMgd2hhdCBJ IHVuZGVyc3RhbmQgZnJvbSB5b3VyIGV4YW1wbGUsIAo+Pj4gaXQncwo+Pj4gbm90IGdvaW5nIHRv IGJlIHRlcnJpYmx5IHVzZWZ1bCB0byBwcm92aWRlIEFURiBhdCBsZWFzdC4uLgo+Pj4gCj4+PiAt VG9rZQo+PiAKPj4gVGhlIGRlc2lnbjoKPj4gV2hlbmV2ZXIgcmFkaW8gdHJhbnNtaXRzIHBhY2tl dCwgZmlybXdhcmUgd2lsbCByZWNvcmQgbnVtYmVycyBvZiBieXRlcwo+PiBzZW50LCBNU0RV4oCZ cyBzZW50LCBUWCBkdXJhdGlvbiwgQU1QRFUgaW5mb3JtYXRpb24sIEFDSyBmYWlsLCBCQSBmYWls LAo+PiBSYXRlIGF0IHdoaWNoIHBhY2tldCBpcyBzZW50LiBUaGlzIGlzIHJlY29yZGVkIGZvciA0 IGZyYW1lcyBzZW50IG9uIAo+PiB0aGF0Cj4+IHBlZXIuIE9uY2UgNCBmcmFtZXMgYXJlIHNlbnQg Zm9yIHRoYXQgcGVlciwgVFggcGFja2V0IGV2ZW50IGlzIHNlbnQgdG8KPj4gYXRoMTBrIGRyaXZl ciB3aXRoIHRoZSByZWNvcmRlZCB2YWx1ZXMuIFRoZXNlIHJhdGUgdmFsdWVzIGFyZSB1cGRhdGVk IAo+PiB0bwo+PiBtYWM4MDIxMSB1c2luZyBpZWVlODAyMTFfdHhfc3RhdHVzKCkuCj4gCj4gU28g dGhlIHZhbHVlcyByZXBvcnRlZCBhcmUgdGhlIHN1bXMgZm9yIGFsbCBmb3VyIHBhY2tldHM/IEJ1 dCB0aGUgCj4gbGF0ZXN0Cj4gdmFsdWUgZm9yIHJhdGUgaW5mb3JtYXRpb24/Cj4gCj4gLVRva2UK ClR4IHJhdGUgaXMgdXBkYXRlZCBmb3IgdGhlIDR0aCBwYWNrZXQuCgpUaGFua3MKQW5pbC4KCl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmF0aDEwayBtYWls aW5nIGxpc3QKYXRoMTBrQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVh ZC5vcmcvbWFpbG1hbi9saXN0aW5mby9hdGgxMGsK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:53430 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727333AbeICOjE (ORCPT ); Mon, 3 Sep 2018 10:39:04 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Date: Mon, 03 Sep 2018 15:49:32 +0530 From: Anilkumar Kolli To: =?UTF-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= Cc: Kalle Valo , ath10k@lists.infradead.org, linux-wireless@vger.kernel.org, Johannes Berg , linux-wireless-owner@vger.kernel.org Subject: Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status() In-Reply-To: <87r2ia28ii.fsf@toke.dk> References: <1524291786-30850-1-git-send-email-akolli@codeaurora.org> <87k1o6d8hw.fsf@kamboji.qca.qualcomm.com> <87zhx21uqc.fsf@toke.dk> <87r2ia28ii.fsf@toke.dk> Message-ID: <01897c3dab4aa7c2371ef6791487413b@codeaurora.org> (sfid-20180903_121937_581840_CFB19D79) Sender: linux-wireless-owner@vger.kernel.org List-ID: On 2018-09-03 15:43, Toke Høiland-Jørgensen wrote: > Anilkumar Kolli writes: > >> On 2018-08-31 19:52, Toke Høiland-Jørgensen wrote: >>> Kalle Valo writes: >>> >>>> Anilkumar Kolli writes: >>>> >>>>> Mesh path metric needs txrate information from >>>>> ieee80211_tx_status() >>>>> call but in ath10k there is no mechanism to report tx rate >>>>> information >>>>> via ieee80211_tx_status(), the rate is only accessible via >>>>> sta_statiscs() op. >>>>> >>>>> Per peer stats has tx rate info available, this patch stores per >>>>> peer >>>>> last tx rate and updates the tx rate info structures in tx >>>>> completition. >>>>> The rate updated in ieee80211_tx_status() is not exactly for the >>>>> last >>>>> transmitted frame instead the rate is from one of the previous >>>>> frames. >>>>> >>>>> Per peer txrate information is updated through per peer statistics >>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only >>>>> >>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053 >>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036 >>>>> >>>>> Signed-off-by: Anilkumar Kolli >>>> >>>> This is a patch from last March, full patch here: >>>> >>>> https://patchwork.kernel.org/patch/10267693/ >>>> >>>>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt >>>>> *htt, >>>>> info->flags &= ~IEEE80211_TX_STAT_ACK; >>>>> } >>>>> >>>>> + if (sta) { >>>>> + spin_lock_bh(&ar->data_lock); >>>>> + arsta = (struct ath10k_sta *)sta->drv_priv; >>>>> + info->status.rates[0].idx = >>>>> + arsta->tx_info.status.rates[0].idx; >>>>> + info->status.rates[0].count = >>>>> + arsta->tx_info.status.rates[0].count; >>>>> + info->status.rates[0].flags = >>>> <> + arsta->tx_info.status.rates[0].flags; >>>>> + spin_unlock_bh(&ar->data_lock); >>>>> + } >>>>> + >>>>> ieee80211_tx_status(htt->ar->hw, msdu); >>>>> /* we do not own the msdu anymore */ >>>> >>>> "is not exactly" is IMHO an understatement. What it means that with >>>> this >>>> patch ath10k reports the rate information from another frame instead >>>> of >>>> the current skb, because the firmware provides the rate information >>>> "out >>>> of band". A simple example to clarify: >>>> >>>> Let's say ath10k transmits frames A, B and C. Then ath10k calls >>>> ieee80211_tx_status() for frame C the rate information could be >>>> for >>>> frame A, or it could be the other around for frame A status the >>>> rate >>>> information is from frame C. >>>> >>>> In other words, there's no guarantee from what frame the rate >>>> information is from. >>>> >>>> Too me this feels like a bad idea but I'm not familiar enough with >>>> mac80211 to really comment on this. What kind of implications does >>>> this >>>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear >>>> about their opinion about this. >>> >>> I was under the impression that the values gathered (at least for >>> airtime) would be cumulative values? If it's just the airtime of a >>> single random frame, which is what I understand from your example, >>> it's >>> not going to be terribly useful to provide ATF at least... >>> >>> -Toke >> >> The design: >> Whenever radio transmits packet, firmware will record numbers of bytes >> sent, MSDU’s sent, TX duration, AMPDU information, ACK fail, BA fail, >> Rate at which packet is sent. This is recorded for 4 frames sent on >> that >> peer. Once 4 frames are sent for that peer, TX packet event is sent to >> ath10k driver with the recorded values. These rate values are updated >> to >> mac80211 using ieee80211_tx_status(). > > So the values reported are the sums for all four packets? But the > latest > value for rate information? > > -Toke Tx rate is updated for the 4th packet. Thanks Anil.