| Message ID | 1390391564-18481-1-git-send-email-karl.beldan@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hmm. I guess you're right about the spec, but I vaguely remember races in this with the delBA going out too soon or so? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 22, 2014 at 01:34:39PM +0100, Johannes Berg wrote: > Hmm. I guess you're right about the spec, but I vaguely remember races > in this with the delBA going out too soon or so? > Indeed, this was intended by cf6bb79 ("Use appropriate TID for sending BAR, ADDBA and DELBA frames") and I overlooked it .. will look into it, thanks. I Cced Helmut who authored the said commit. Karl -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 22, 2014 at 2:09 PM, Karl Beldan <karl.beldan@gmail.com> wrote: > On Wed, Jan 22, 2014 at 01:34:39PM +0100, Johannes Berg wrote: >> Hmm. I guess you're right about the spec, but I vaguely remember races >> in this with the delBA going out too soon or so? >> > > Indeed, this was intended by cf6bb79 ("Use appropriate TID for sending > BAR, ADDBA and DELBA frames") and I overlooked it .. will look into it, > thanks. > I Cced Helmut who authored the said commit. You're right. There were some issues with that, but that was 2 years ago :) Sending ADDBA over AV_VO should be safe. If a DELBA is sent as AC_VO it might get received before the last AMPDU of the BlockAck session. So, the pending AMPDUs will get dropped at the receiver. In theory this could also be avoided by properly flushing all pending AMPDUs of the TID in question from the hw queues or by waiting for the tx status of all pending AMPDUs. Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 22, 2014 at 02:33:14PM +0100, Helmut Schaa wrote: > On Wed, Jan 22, 2014 at 2:09 PM, Karl Beldan <karl.beldan@gmail.com> wrote: > > On Wed, Jan 22, 2014 at 01:34:39PM +0100, Johannes Berg wrote: > >> Hmm. I guess you're right about the spec, but I vaguely remember races > >> in this with the delBA going out too soon or so? > >> > > > > Indeed, this was intended by cf6bb79 ("Use appropriate TID for sending > > BAR, ADDBA and DELBA frames") and I overlooked it .. will look into it, > > thanks. > > I Cced Helmut who authored the said commit. > > You're right. There were some issues with that, but that was 2 years ago :) > > Sending ADDBA over AV_VO should be safe. > If a DELBA is sent as AC_VO it might get received before the last AMPDU > of the BlockAck session. So, the pending AMPDUs will get dropped at the > receiver. > > In theory this could also be avoided by properly flushing all pending AMPDUs > of the TID in question from the hw queues or by waiting for the tx status > of all pending AMPDUs. > I just looked at the code with your change in mind and couldn't find any issue caused by sending {add,del}ba on AC_VO. As of today, a delba is sent only after a driver has called 'purposely' ieee80211_stop_tx_ba_cb_irqsafe so I see no issue with the delba either, do you see one today ? Johannes, does it look good to you ? maybe add some comments in the changelog relating to cf6bb79 ? Karl -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 22, 2014 at 4:28 PM, Karl Beldan <karl.beldan@gmail.com> wrote: > On Wed, Jan 22, 2014 at 02:33:14PM +0100, Helmut Schaa wrote: >> On Wed, Jan 22, 2014 at 2:09 PM, Karl Beldan <karl.beldan@gmail.com> wrote: >> > On Wed, Jan 22, 2014 at 01:34:39PM +0100, Johannes Berg wrote: >> >> Hmm. I guess you're right about the spec, but I vaguely remember races >> >> in this with the delBA going out too soon or so? >> >> >> > >> > Indeed, this was intended by cf6bb79 ("Use appropriate TID for sending >> > BAR, ADDBA and DELBA frames") and I overlooked it .. will look into it, >> > thanks. >> > I Cced Helmut who authored the said commit. >> >> You're right. There were some issues with that, but that was 2 years ago :) >> >> Sending ADDBA over AV_VO should be safe. >> If a DELBA is sent as AC_VO it might get received before the last AMPDU >> of the BlockAck session. So, the pending AMPDUs will get dropped at the >> receiver. >> >> In theory this could also be avoided by properly flushing all pending AMPDUs >> of the TID in question from the hw queues or by waiting for the tx status >> of all pending AMPDUs. >> > > I just looked at the code with your change in mind and couldn't find any > issue caused by sending {add,del}ba on AC_VO. > As of today, a delba is sent only after a driver has called 'purposely' > ieee80211_stop_tx_ba_cb_irqsafe so I see no issue with the delba either, > do you see one today ? If the driver does the right thing by flushing the right frames in the hw tx queues I think this should be ok. At least rt2x00 is not doing that, but maybe that's just an issue in rt2x00 :( Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 22, 2014 at 04:36:14PM +0100, Helmut Schaa wrote: > On Wed, Jan 22, 2014 at 4:28 PM, Karl Beldan <karl.beldan@gmail.com> wrote: > > On Wed, Jan 22, 2014 at 02:33:14PM +0100, Helmut Schaa wrote: > >> On Wed, Jan 22, 2014 at 2:09 PM, Karl Beldan <karl.beldan@gmail.com> wrote: > >> > On Wed, Jan 22, 2014 at 01:34:39PM +0100, Johannes Berg wrote: > >> >> Hmm. I guess you're right about the spec, but I vaguely remember races > >> >> in this with the delBA going out too soon or so? > >> >> > >> > > >> > Indeed, this was intended by cf6bb79 ("Use appropriate TID for sending > >> > BAR, ADDBA and DELBA frames") and I overlooked it .. will look into it, > >> > thanks. > >> > I Cced Helmut who authored the said commit. > >> > >> You're right. There were some issues with that, but that was 2 years ago :) > >> > >> Sending ADDBA over AV_VO should be safe. > >> If a DELBA is sent as AC_VO it might get received before the last AMPDU > >> of the BlockAck session. So, the pending AMPDUs will get dropped at the > >> receiver. > >> > >> In theory this could also be avoided by properly flushing all pending AMPDUs > >> of the TID in question from the hw queues or by waiting for the tx status > >> of all pending AMPDUs. > >> > > > > I just looked at the code with your change in mind and couldn't find any > > issue caused by sending {add,del}ba on AC_VO. > > As of today, a delba is sent only after a driver has called 'purposely' > > ieee80211_stop_tx_ba_cb_irqsafe so I see no issue with the delba either, > > do you see one today ? > > If the driver does the right thing by flushing the right frames in the > hw tx queues > I think this should be ok. At least rt2x00 is not doing that, but > maybe that's just > an issue in rt2x00 :( > I see you are one of the maintainers of the ralink drivers, do you intend to fix it in those ? Callers of ieee80211_stop_tx_ba_cb_irqsafe are: ath9k ath9k_htc carl9170 wcn36xx brcm80211 iwlegacy iwlwifi mwl8k rt2x00 rtlwifi Johannes, what is your impression ? Karl -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2014-01-22 at 17:41 +0100, Karl Beldan wrote: > > >> If a DELBA is sent as AC_VO it might get received before the last AMPDU > > >> of the BlockAck session. So, the pending AMPDUs will get dropped at the > > >> receiver. > > >> > > >> In theory this could also be avoided by properly flushing all pending AMPDUs > > >> of the TID in question from the hw queues or by waiting for the tx status > > >> of all pending AMPDUs. Let's get the issue straight first. The (current) documentation for the TX_STOP ampdu actions says: * @IEEE80211_AMPDU_TX_STOP_CONT: stop TX aggregation but continue transmitting * queued packets, now unaggregated. After all packets are transmitted the * driver has to call ieee80211_stop_tx_ba_cb_irqsafe(). * @IEEE80211_AMPDU_TX_STOP_FLUSH: stop TX aggregation and flush all packets, * called when the station is removed. There's no need or reason to call * ieee80211_stop_tx_ba_cb_irqsafe() in this case as mac80211 assumes the * session is gone and removes the station. * @IEEE80211_AMPDU_TX_STOP_FLUSH_CONT: called when TX aggregation is stopped * but the driver hasn't called ieee80211_stop_tx_ba_cb_irqsafe() yet and * now the connection is dropped and the station will be removed. Drivers * should clean up and drop remaining packets when this is called. I say "current" because we only completed this fairly recently (couple months ago?) to make it complete with the three different possibilities. Therefore, I don't think there's a need to ever *flush* the queues, although the term "flush" is getting confusing and we should probably call this IEEE80211_AMPDU_TX_STOP_DROP instead of _FLUSH, as that's what it really means, drop all the remaining frames (if any.) Given the fact that we only send the frame from ieee80211_stop_tx_ba_cb() I don't see any problem. Even if we were to send the frame directly after calling the ampdu_action, it seems it would be fine, since the callback (now) requires sending the remaining frames unaggregated. (Given that, I'm not even sure why we required the packets to be sent unaggregated, Emmanuel, do you remember?) > Callers of ieee80211_stop_tx_ba_cb_irqsafe are: > ath9k > ath9k_htc > carl9170 > wcn36xx > brcm80211 > iwlegacy > iwlwifi > mwl8k > rt2x00 > rtlwifi > > Johannes, what is your impression ? I'm pretty sure iwlwifi should be safe. ath9k/htc/carl9170/wcn36xx are probably fine, I can't really say anything about the other ones. I'd guess brcm80211 should be OK since it builds aggregates in software and should be able to easily stop transmitting aggregates. But see above - I don't see how even if the driver didn't stop sending aggregates it could be wrong. Unless the driver called ieee80211_stop_tx_ba_cb() immediately but didn't actually stop sending. That's pretty much a bug anyway though. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 22, 2014 at 07:51:53PM +0100, Johannes Berg wrote: > On Wed, 2014-01-22 at 17:41 +0100, Karl Beldan wrote: > > > > >> If a DELBA is sent as AC_VO it might get received before the last AMPDU > > > >> of the BlockAck session. So, the pending AMPDUs will get dropped at the > > > >> receiver. > > > >> > > > >> In theory this could also be avoided by properly flushing all pending AMPDUs > > > >> of the TID in question from the hw queues or by waiting for the tx status > > > >> of all pending AMPDUs. > > Let's get the issue straight first. > > The (current) documentation for the TX_STOP ampdu actions says: > > * @IEEE80211_AMPDU_TX_STOP_CONT: stop TX aggregation but continue > transmitting > * queued packets, now unaggregated. After all packets are > transmitted the > * driver has to call ieee80211_stop_tx_ba_cb_irqsafe(). > * @IEEE80211_AMPDU_TX_STOP_FLUSH: stop TX aggregation and flush all > packets, > * called when the station is removed. There's no need or reason to > call > * ieee80211_stop_tx_ba_cb_irqsafe() in this case as mac80211 > assumes the > * session is gone and removes the station. > * @IEEE80211_AMPDU_TX_STOP_FLUSH_CONT: called when TX aggregation is > stopped > * but the driver hasn't called ieee80211_stop_tx_ba_cb_irqsafe() > yet and > * now the connection is dropped and the station will be removed. > Drivers > * should clean up and drop remaining packets when this is called. > > I say "current" because we only completed this fairly recently (couple > months ago?) to make it complete with the three different possibilities. > > Therefore, I don't think there's a need to ever *flush* the queues, > although the term "flush" is getting confusing and we should probably > call this IEEE80211_AMPDU_TX_STOP_DROP instead of _FLUSH, as that's what > it really means, drop all the remaining frames (if any.) > > Given the fact that we only send the frame from > ieee80211_stop_tx_ba_cb() I don't see any problem. Even if we were to > send the frame directly after calling the ampdu_action, it seems it > would be fine, since the callback (now) requires sending the remaining > frames unaggregated. (Given that, I'm not even sure why we required the > packets to be sent unaggregated, Emmanuel, do you remember?) > I'd expect most device to not block ack such frames, and they'd be right to do so, sending them unaggregated seems the right thing to do. > > Callers of ieee80211_stop_tx_ba_cb_irqsafe are: > > ath9k > > ath9k_htc > > carl9170 > > wcn36xx > > brcm80211 > > iwlegacy > > iwlwifi > > mwl8k > > rt2x00 > > rtlwifi > > > > Johannes, what is your impression ? > > I'm pretty sure iwlwifi should be safe. ath9k/htc/carl9170/wcn36xx are > probably fine, I can't really say anything about the other ones. I'd > guess brcm80211 should be OK since it builds aggregates in software and > should be able to easily stop transmitting aggregates. But see above - I > don't see how even if the driver didn't stop sending aggregates it could > be wrong. Unless the driver called ieee80211_stop_tx_ba_cb() immediately > but didn't actually stop sending. That's pretty much a bug anyway > though. > So, I guess you are taking what I sent ? Karl -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2014-01-22 at 20:16 +0100, Karl Beldan wrote: > > Given the fact that we only send the frame from > > ieee80211_stop_tx_ba_cb() I don't see any problem. Even if we were to > > send the frame directly after calling the ampdu_action, it seems it > > would be fine, since the callback (now) requires sending the remaining > > frames unaggregated. (Given that, I'm not even sure why we required the > > packets to be sent unaggregated, Emmanuel, do you remember?) > > > I'd expect most device to not block ack such frames, and they'd be > right to do so, sending them unaggregated seems the right thing to do. Oh, I roughly remember now - we didn't want to separate the cases of us sending a delBA and us receiving a delBA. If we receive a delBA, we should stop sending aggregated frames immediately (actually for iwlwifi the firmware will do that) or as quickly as possible, hence the requirement If we decide to tear down the session ourselves then we could continue sending until later, but it's not worth it. > So, I guess you are taking what I sent ? Haven't really made up my mind yet ... I think it's more correct, so I should, but I also don't really want to break the ralink drivers over what seems to me to be a fairly small issue. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg <johannes@sipsolutions.net> schrieb: >On Wed, 2014-01-22 at 20:16 +0100, Karl Beldan wrote: > >> > Given the fact that we only send the frame from >> > ieee80211_stop_tx_ba_cb() I don't see any problem. Even if we were >to >> > send the frame directly after calling the ampdu_action, it seems it >> > would be fine, since the callback (now) requires sending the >remaining >> > frames unaggregated. (Given that, I'm not even sure why we required >the >> > packets to be sent unaggregated, Emmanuel, do you remember?) >> > >> I'd expect most device to not block ack such frames, and they'd be >> right to do so, sending them unaggregated seems the right thing to >do. > >Oh, I roughly remember now - we didn't want to separate the cases of us >sending a delBA and us receiving a delBA. If we receive a delBA, we >should stop sending aggregated frames immediately (actually for iwlwifi >the firmware will do that) or as quickly as possible, hence the >requirement > >If we decide to tear down the session ourselves then we could continue >sending until later, but it's not worth it. > >> So, I guess you are taking what I sent ? > >Haven't really made up my mind yet ... I think it's more correct, so I >should, but I also don't really want to break the ralink drivers over >what seems to me to be a fairly small issue. I think I'm fine with this now. Let's just see if someone experiences any issues ... Furthermore I think i even remember that you can force ralink HW to stop a BA session. But all in all lets better comply with the spec ... Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PiBKb2hhbm5lcyBCZXJnIDxqb2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0PiBzY2hyaWViOg0KPiA+ T24gV2VkLCAyMDE0LTAxLTIyIGF0IDIwOjE2ICswMTAwLCBLYXJsIEJlbGRhbiB3cm90ZToNCj4g Pg0KPiA+PiA+IEdpdmVuIHRoZSBmYWN0IHRoYXQgd2Ugb25seSBzZW5kIHRoZSBmcmFtZSBmcm9t DQo+ID4+ID4gaWVlZTgwMjExX3N0b3BfdHhfYmFfY2IoKSBJIGRvbid0IHNlZSBhbnkgcHJvYmxl bS4gRXZlbiBpZiB3ZSB3ZXJlDQo+ID50bw0KPiA+PiA+IHNlbmQgdGhlIGZyYW1lIGRpcmVjdGx5 IGFmdGVyIGNhbGxpbmcgdGhlIGFtcGR1X2FjdGlvbiwgaXQgc2VlbXMgaXQNCj4gPj4gPiB3b3Vs ZCBiZSBmaW5lLCBzaW5jZSB0aGUgY2FsbGJhY2sgKG5vdykgcmVxdWlyZXMgc2VuZGluZyB0aGUN Cj4gPnJlbWFpbmluZw0KPiA+PiA+IGZyYW1lcyB1bmFnZ3JlZ2F0ZWQuIChHaXZlbiB0aGF0LCBJ J20gbm90IGV2ZW4gc3VyZSB3aHkgd2UgcmVxdWlyZWQNCj4gPnRoZQ0KPiA+PiA+IHBhY2tldHMg dG8gYmUgc2VudCB1bmFnZ3JlZ2F0ZWQsIEVtbWFudWVsLCBkbyB5b3UgcmVtZW1iZXI/KQ0KPiA+ PiA+DQo+ID4+IEknZCBleHBlY3QgbW9zdCBkZXZpY2UgdG8gbm90IGJsb2NrIGFjayBzdWNoIGZy YW1lcywgYW5kIHRoZXknZCBiZQ0KPiA+PiByaWdodCB0byBkbyBzbywgc2VuZGluZyB0aGVtIHVu YWdncmVnYXRlZCBzZWVtcyB0aGUgcmlnaHQgdGhpbmcgdG8NCj4gPmRvLg0KPiA+DQo+ID5PaCwg SSByb3VnaGx5IHJlbWVtYmVyIG5vdyAtIHdlIGRpZG4ndCB3YW50IHRvIHNlcGFyYXRlIHRoZSBj YXNlcyBvZiB1cw0KPiA+c2VuZGluZyBhIGRlbEJBIGFuZCB1cyByZWNlaXZpbmcgYSBkZWxCQS4g SWYgd2UgcmVjZWl2ZSBhIGRlbEJBLCB3ZQ0KPiA+c2hvdWxkIHN0b3Agc2VuZGluZyBhZ2dyZWdh dGVkIGZyYW1lcyBpbW1lZGlhdGVseSAoYWN0dWFsbHkgZm9yIGl3bHdpZmkNCj4gPnRoZSBmaXJt d2FyZSB3aWxsIGRvIHRoYXQpIG9yIGFzIHF1aWNrbHkgYXMgcG9zc2libGUsIGhlbmNlIHRoZQ0K PiA+cmVxdWlyZW1lbnQNCj4gPg0KPiA+SWYgd2UgZGVjaWRlIHRvIHRlYXIgZG93biB0aGUgc2Vz c2lvbiBvdXJzZWx2ZXMgdGhlbiB3ZSBjb3VsZCBjb250aW51ZQ0KPiA+c2VuZGluZyB1bnRpbCBs YXRlciwgYnV0IGl0J3Mgbm90IHdvcnRoIGl0Lg0KPiA+DQoNCkV4YWN0bHkgLSB3ZSB3YW50ZWQg dG8gdW5pZnkgdGhlIGluaXRpYXRvciAodGhlIFRYaW5nIHNpZGUgZGVjaWRlcyB0byBzdG9wIHRo ZSBCQSBhZ3JlZW1lbnQpIGFuZCB0aGUgcmVjaXBpZW50ICh0aGUgUlhpbmcgc2lkZSBkZWNpZGVz IHRvIHN0b3AgdGhlIEJBIGFncmVlbWVudCkgY2FzZXMuIFNvIGl0IGdvZXMgbGlrZSB0aGlzOg0K SWYgeW91IGdldCBhcmUgVFhpbmcgYW5kIHlvdSBnZXQgYSAocmVjaXBpZW50KSBERUxCQSwgdGhl biB5b3UgbmVlZCB0byBzdG9wIHNlbmRpbmcgQU1QRFVzIHN0cmFpZ2h0IGF3YXkuIFRoaXMgaXMg d2h5IEludGVsIGNhcmRzIGhhdmUgdGhlIGZpcm13YXJlIGRvIHRoaXMuIEJ1dCBzaW5jZSBpd2x3 aWZpIHVzZXMgZGlmZmVyZW50IHF1ZXVlcyBmb3Igbm9uLWFnZ3JlZ2F0ZWQgdHJhZmZpYyBhbmQg QU1QRFVzLCB3ZSBtdXN0IHdhaXQgdW50aWwgYWxsIHRoZSBwYWNrZXRzIGluIHRoZSBBTVBEVSBx dWV1ZSB3aWxsIGRyYWluLCBhbmQgb25seSB0aGVuLCBhbGxvdyBtYWM4MDIxMSB0byBzZW5kIHBh Y2tldHMgdG8gdGhlIG5vbi1hZ2dyZWdhdGVkIHRyYWZmaWMgcXVldWUuIE5vdyAtIGV2ZW4gdmVu ZG9ycyB0aGF0IGRvbid0IHByZXZlbnQgdGhlIEFNUERVcyBpbiBmaXJtd2FyZSBzaG91bGQgc3Rp bGwgZG8gdGhlIHNhbWUgKHByb3ZpZGVkIHRoYXQgdGhleSB1c2UgZGlmZmVyZW50IHF1ZXVlcyBm b3IgQU1QRFUgYW5kIG5vbi1BTVBEVSB0cmFmZmljKS4gVGhpcyBpcyBub3QgdG8gcHJldmVudCBz ZW5kaW5nIEFNUERVcyBhZnRlciBERUxCQSwgYnV0IG1vcmUgdG8gcHJldmVudCBvdXQtb2Ytb3Jk ZXIgVFguDQpJbiB0aGUgY2FzZSBvZiBvcmlnaW5hdG9yIERFTEJBLCBpdCBpcyBzbGlnaHRseSBk aWZmZXJlbnQuIFdlIGNhbiBjaG9vc2Ugd2hlbiB0byBzZW5kIHRoZSBERUxCQSBhZnRlciBhbGwu IE9mIGNvdXJzZSB3ZSBzdGlsbCBoYXZlIHRoZSBvdXQtb2Ytb3JkZXIgdGhpbmcgdG8gdGFrZSBj YXJlIGFib3V0LCBidXQgYXQgbGVhc3QsIHdlIGNhbiBhdm9pZCB0aGUgIkFNUERVcyBhZnRlciBE RUxCQSIgZXZlbiB3aXRob3V0IGZpcm13YXJlIHN1cHBvcnQuDQpUaGUgb25seSB0aGluZyB5b3Ug aGF2ZSB0byBkbyBpcyB0byBtYWtlIHN1cmUgdGhhdCBpZWVlODAyMTFfc3RvcF90eF9iYV9jYiBp cyBjYWxsZWQgb25seSAqYWZ0ZXIqIHRoZSBkcml2ZXIgaXMgc3VyZSB0aGF0IGFsbCB0aGUgcGFj a2V0cyBpbiB0aGUgQU1QRFUgcXVldWUgYXJlIHNlbnQuIE1hYzgwMjExIHdvbid0IHNlbmQgdGhl IERFTEJBIGJlZm9yZSB0aGUgbG93IGxldmVsIGRyaXZlciBjYWxscyBpZWVlODAyMTFfc3RvcF90 eF9iYV9jYi4gU28sIGlmIHRoZSBkcml2ZXIgaXMgYWJsZSB0byB0ZWxsIG1hYzgwMjExIHdoZW4g aXQgaXMgZmluaXNoZWQgd2l0aCB0aGUgQU1QRFUgcGFja2V0cywgeW91IGNhbiBzYWZlbHkgc2Vu ZCBpdCBpbiBWTyBhbmQgbm90IGZlYXIgYW55ICJBTVBEVSBhZnRlciBERUxCQSIgcHJvYmxlbS4N CkFub3RoZXIgKHNpZGUpIHBvaW50LiBFdmVuIGlmIHlvdSBjYW4gc3RvcCBzZW5kaW5nIEFNUERV cyBzdHJhaWdodCBhd2F5IChpd2x3aWZpIGNhbiBkbyB0aGF0IHdpdGggYSBjb21tYW5kIHRvIHRo ZSBmaXJtd2FyZSB0aGF0IHdpbGwgYmUgbXVjaCBmYXN0ZXIgdGhhbiB3YWl0aW5nIGZvciBxdWV1 ZXMgdG8gZHJhaW4pLCB5b3Ugc3RpbGwgaGF2ZSB0aGUgVHggb3V0LW9mLW9yZGVyIHByb2JsZW0u DQpTbyB0aGF0IHRlY2huaWNhbGx5LCB3ZSBjb3VsZCBoYXZlIGEgbW9yZSBmaW5lIGdyYWluIHJl c29sdXRpb24gaW4gbWFjODAyMTEgdGhhdCBhbGxvd3MgdGhlIGxvdyBsZXZlbCBkcml2ZXIgdG8g aGF2ZSBtYWM4MDIxMSBzZW5kIHRoZSBERUxCQSBlYXJsaWVyLCBidXQgc3RpbGwgcHJldmVudHMg bWFjODAyMTEgZnJvbSBzZW5kaW5nIHBhY2tldHMgdG8gdGhlIGxvdyBsZXZlbCBkcml2ZXIgdW50 aWwgdGhlIGFsbCB0aGUgcXVldWVzIGFyZSBkcmFpbmVkLg0KQnV0IHRoaXMgaXMgZGVmaW5pdGVs eSBub3Qgd29ydGggdGhlIGNvbXBsZXhpdHkgc2luY2UgZGVsYXlpbmcgdGhlIERFTEJBIGJ5IGEg Yml0IGRvZXNuJ3QgY29zdCBhbnl0aGluZy4NCg0KSG9wZSB0aGF0IGhlbHBzIDopDQoNCj4gPj4g U28sIEkgZ3Vlc3MgeW91IGFyZSB0YWtpbmcgd2hhdCBJIHNlbnQgPw0KPiA+DQo+ID5IYXZlbid0 IHJlYWxseSBtYWRlIHVwIG15IG1pbmQgeWV0IC4uLiBJIHRoaW5rIGl0J3MgbW9yZSBjb3JyZWN0 LCBzbyBJDQo+ID5zaG91bGQsIGJ1dCBJIGFsc28gZG9uJ3QgcmVhbGx5IHdhbnQgdG8gYnJlYWsg dGhlIHJhbGluayBkcml2ZXJzIG92ZXINCj4gPndoYXQgc2VlbXMgdG8gbWUgdG8gYmUgYSBmYWly bHkgc21hbGwgaXNzdWUuDQo+IA0KPiBJIHRoaW5rIEknbSBmaW5lIHdpdGggdGhpcyBub3cuIExl dCdzIGp1c3Qgc2VlIGlmIHNvbWVvbmUgZXhwZXJpZW5jZXMgYW55DQo+IGlzc3VlcyAuLi4NCj4g DQo+ICBGdXJ0aGVybW9yZSBJIHRoaW5rIGkgZXZlbiByZW1lbWJlciB0aGF0IHlvdSBjYW4gZm9y Y2UgcmFsaW5rIEhXIHRvIHN0b3AgYQ0KPiBCQSBzZXNzaW9uLiBCdXQgYWxsIGluIGFsbCBsZXRz IGJldHRlciBjb21wbHkgd2l0aCB0aGUgc3BlYyAuLi4NCg0KV2VsbCAtIHlvdSBuZWVkIHRvIHNl ZSB3aGF0ICJzdG9wIGEgQkEgc2Vzc2lvbiIgbWVhbi4gSWYgeW91IGFyZSBhYmxlIHRvIHRlbGwg dGhlIEhXIG5vIEFNUERVcyBhbnltb3JlICpub3cqLCB0aGVuIG9rIC0gYnV0IHlvdSBzdGlsbCBo YXZlIHRoZSBvdXQtb2Ytb3JkZXIgVFggcHJvYmxlbS4gVW5sZXNzIHlvdSB1c2UgdGhlIHNhbWUg VFggcXVldWUgZm9yIEFNUFVzIGFuZCBub24tQU1QRFVzIGFuZCBkb24ndCBmZWFyIHJlb3JkZXJp bmcgaXNzdWVzDQoNCj4gDQo+IEhlbG11dA0K -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 23, 2014 at 7:08 AM, Grumbach, Emmanuel <emmanuel.grumbach@intel.com> wrote: >> >> So, I guess you are taking what I sent ? >> > >> >Haven't really made up my mind yet ... I think it's more correct, so I >> >should, but I also don't really want to break the ralink drivers over >> >what seems to me to be a fairly small issue. >> >> I think I'm fine with this now. Let's just see if someone experiences any >> issues ... >> >> Furthermore I think i even remember that you can force ralink HW to stop a >> BA session. But all in all lets better comply with the spec ... > > Well - you need to see what "stop a BA session" mean. If you are able to tell the HW no AMPDUs anymore *now*, then ok - but you still have the out-of-order TX problem. Unless you use the same TX queue for AMPUs and non-AMPDUs and don't fear reordering issues We shouldn't have reordering issues in rt2x00 since AMPDUs and MPDUs go through the same hw queue. However, at the moment the hw will keep on sending AMPDUs that are in the TX queue even after we received a DELBA. But this is an issue with the ralink drivers ... Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2014-01-23 at 14:42 +0100, Helmut Schaa wrote: > On Thu, Jan 23, 2014 at 7:08 AM, Grumbach, Emmanuel > <emmanuel.grumbach@intel.com> wrote: > >> >> So, I guess you are taking what I sent ? > >> > > >> >Haven't really made up my mind yet ... I think it's more correct, so I > >> >should, but I also don't really want to break the ralink drivers over > >> >what seems to me to be a fairly small issue. > >> > >> I think I'm fine with this now. Let's just see if someone experiences any > >> issues ... > >> > >> Furthermore I think i even remember that you can force ralink HW to stop a > >> BA session. But all in all lets better comply with the spec ... > > > > Well - you need to see what "stop a BA session" mean. If you are able to tell the HW no AMPDUs anymore *now*, then ok - but you still have the out-of-order TX problem. Unless you use the same TX queue for AMPUs and non-AMPDUs and don't fear reordering issues > > We shouldn't have reordering issues in rt2x00 since AMPDUs and MPDUs > go through the same hw queue. > However, at the moment the hw will keep on sending AMPDUs that are in > the TX queue even after we received a DELBA. > But this is an issue with the ralink drivers ... Ok. Karl, can you rewrite the commit log, maybe referencing the original commit and this discussion (I'd suggest using http://mid.gmane.org/1390391564-18481-1-git-send-email-karl.beldan@gmail.com) please? Then I'll apply that. Thanks, johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 13b7683..ce9633a 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -107,7 +107,7 @@ static void ieee80211_send_addba_request(struct ieee80211_sub_if_data *sdata, mgmt->u.action.u.addba_req.start_seq_num = cpu_to_le16(start_seq_num << 4); - ieee80211_tx_skb_tid(sdata, skb, tid); + ieee80211_tx_skb(sdata, skb); } void ieee80211_send_bar(struct ieee80211_vif *vif, u8 *ra, u16 tid, u16 ssn) diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c index fab7b91..dc3c280 100644 --- a/net/mac80211/ht.c +++ b/net/mac80211/ht.c @@ -375,7 +375,7 @@ void ieee80211_send_delba(struct ieee80211_sub_if_data *sdata, mgmt->u.action.u.delba.params = cpu_to_le16(params); mgmt->u.action.u.delba.reason_code = cpu_to_le16(reason_code); - ieee80211_tx_skb_tid(sdata, skb, tid); + ieee80211_tx_skb(sdata, skb); } void ieee80211_process_delba(struct ieee80211_sub_if_data *sdata,