TXkgY29tbWVudHMgaW5saW5lW0FTXToNCg0KLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZy
b206IEpvaGFubmVzIEJlcmcgW21haWx0bzpqb2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0XQ0KU2Vu
dDogTW9uZGF5LCBGZWJydWFyeSAwNCwgMjAxMyA4OjU4IFBNDQpUbzogQW1pdCBTSEFLWUE7IENo
cmlzdGlhbiBMYW1wYXJ0ZXINCkNjOiBKb2huIFcuIExpbnZpbGxlOyBsaW51eC13aXJlbGVzcw0K
U3ViamVjdDogUmU6IFtQQVRDSF0gbWFjODAyMTE6IEZpeCBQTiBjb3JydXB0aW9uIGluIGNhc2Ug
b2YgbXVsdGlwbGUgdmlydHVhbCBpbnRlcmZhY2UNCg0KT24gTW9uLCAyMDEzLTAyLTA0IGF0IDE2
OjQ4ICswNTMwLCBBbWl0IFNoYWt5YSB3cm90ZToNCj4gSW4gY2FzZSBvZiBtdWx0aXBsZSB2aXJ0
dWFsIGludGVyZmFjZSwgaWYgQUVTIGlzIGNvbmZpZ3VyZWQgb24gDQo+IG11bHRpcGxlIGludGVy
ZmFjZSB0aGVuIHRoZXJlIGFyZSBjaGFuY2VzIG9mIHN0b3JlZCBQTiBjb3JydXB0aW9uLCANCj4g
Y2F1c2luZyB0cmFmZmljIHRvIHN0b3AuDQoNCkludGVyZXN0aW5nLCBuaWNlIGZpbmQuDQoNCj4g
SW4gY2FzZSwgaWVlZTgwMjExX3J4X2hhbmRsZXJzIHByb2Nlc3NpbmcgaXMgZ29pbmcgb24gZm9y
IHNrYnMgDQo+IHJlY2VpdmVkIG9uIG9uZSB2aWYgYW5kIGF0IHRoZSBzYW1lIHRpbWUsIHJ4IGFn
Z3JlZ2F0aW9uIHJlb3JkZXIgdGltZXIgDQo+IGV4cGlyZXMgb24gYW5vdGhlciB2aWYgdGhlbiBz
dGFfcnhfYWdnX3Jlb3JkZXJfdGltZXJfZXhwaXJlZCBpcyANCj4gaW52b2tlZCBhbmQgaXQgd2ls
bCBwdXNoIHNrYnMgaW50byB0aGUgc2luZ2xlIHF1ZXVlIChsb2NhbC0+cnhfc2tiX3F1ZXVlKS4N
Cj4gaWVlZTgwMjExX3J4X2hhbmRsZXJzIGluIHRoZSB3aGlsZSBsb29wIGFzc3VtZXMgdGhhdCB0
aGUgc2ticyBhcmUgZm9yIA0KPiB0aGUgc2FtZSBUSUQgYW5kIHNhbWUgc3RhLg0KDQpUaGlzIGlz
IGJlY2F1c2Ugb2Ygc3RydWN0IGllZWU4MDIxMV9yeF9kYXRhLCBwcmVzdW1hYmx5PyBJT1csIHRo
ZSBsb29wIGRvZXNuJ3QgcmVhbGx5IGFzc3VtZSBpdCwgaXQncyB0aGUgZmFjdCB0aGF0IHRoZSBs
b29wIGlzIGNhbGxlZCB3aXRoIGEgZ2l2ZW4gc3RydWN0IGllZWU4MDIxMV9yeF9kYXRhLCByaWdo
dD8NCltBU10gWWVzDQoNCj4gVGhpcyBhc3N1bXB0aW9uIGRvZXNuJ3QgaG9sZCBnb29kIGluIHRo
aXMgc2NlbmFyaW8gYW5kIHRoZSBQTiBnZXRzIA0KPiBjb3JydXB0ZWQgYnkgUE4gcmVjZWl2ZWQg
aW4gb3RoZXIgdmlmJ3Mgc2tiLCBjYXVzaW5nIHRyYWZmaWMgdG8gc3RvcCANCj4gZHVlIHRvIFBO
IG1pc21hdGNoLg0KPiBUaGlzIGNhbiBiZSBhdm9pZGVkIGJ5IGNvbXBhcmluZyBzb3VyY2UgbWFj
IGFkZHJlcyBpbiByZWNlaXZlZCBza2IncyANCj4gd2l0aCB0aGUgc3RhJ3MgbWFjIGFkZHJlc3Mg
Zm9yIHdoaWNoIHByb2Nlc3NpbmcgaXMgZ29pbmcgb24sIHdoZW4gZGUtcXVldWVpbmcuDQoNCg0K
DQo+IEBAIC0yNzkwLDcgKzI3OTEsMjAgQEAgc3RhdGljIHZvaWQgaWVlZTgwMjExX3J4X2hhbmRs
ZXJzKHN0cnVjdCANCj4gaWVlZTgwMjExX3J4X2RhdGEgKnJ4KQ0KPiAgDQo+ICAJcngtPmxvY2Fs
LT5ydW5uaW5nX3J4X2hhbmRsZXIgPSB0cnVlOw0KPiAgDQo+IC0Jd2hpbGUgKChza2IgPSBfX3Nr
Yl9kZXF1ZXVlKCZyeC0+bG9jYWwtPnJ4X3NrYl9xdWV1ZSkpKSB7DQo+ICsJc2tiX3F1ZXVlX3dh
bGtfc2FmZSgmcngtPmxvY2FsLT5yeF9za2JfcXVldWUsIHNrYiwgdG1wKSB7DQo+ICsJCWlmICgh
c2tiKQ0KPiArCQkJYnJlYWs7DQo+ICsJCWhkciA9IChzdHJ1Y3QgaWVlZTgwMjExX2hkciAqKSBz
a2ItPmRhdGE7DQo+ICsJCS8qDQo+ICsJCSogQWRkaXRpb25hbCBjaGVjayB0byBlbnN1cmUgdGhh
dCB0aGUgcGFja2V0cyBjb3JyZXNwb25kaW5nDQo+ICsJCSogdG8gc2FtZSBzdGEgZW50cnkgYXMg
aW4gcngtPnN0YSBhcmUgZGUtcXVldWVkLiBUaGUgcXVldWUNCj4gKwkJKiBjYW4gaGF2ZSBkaWZm
ZXJlbnQgaW50ZXJmYWNlIHBhY2tldHMgaW4gY2FzZSBvZiBtdWx0aXBsZSB2aWZzDQo+ICsJCSov
DQo+ICsJCWlmICgocngtPnN0YSAmJiBoZHIpICYmIChpZWVlODAyMTFfaXNfZGF0YShoZHItPmZy
YW1lX2NvbnRyb2wpKQ0KPiArCQkJJiYgKG1lbWNtcChyeC0+c3RhLT5zdGEuYWRkciwgaGRyLT5h
ZGRyMiwgRVRIX0FMRU4pKSkNCj4gKwkJCQkJY29udGludWU7DQo+ICsJCV9fc2tiX3VubGluayhz
a2IsICZyeC0+bG9jYWwtPnJ4X3NrYl9xdWV1ZSk7DQoNCkFwYXJ0IGZyb20gdGhlIGN1cmlvdXMg
Y29kaW5nIHN0eWxlICh5b3UgZG9uJ3QgbmVlZCBhbnkgb2YgdGhvc2UgZXh0cmEgcGFyZW50aGVz
ZXMsICYmIHNob3VsZCBiZSBvbiB0aGUgcHJldmlvdXMgbGluZSwgdGhlIGlmIGNvbnRpbnVhdGlv
biBpbmRlbnRlZCB0byB0aGUgb3BlbmluZyBwYXJlbnRoZXNpcywgY29udGludWUgb25seSBpbmRl
bnRlZCBvbmUgdGFiKSwgSSB3b25kZXIgaWYgdGhpcyBjb3VsZCBsZWFkIHRvIGxlYWtpbmcgZnJh
bWVzIGhlcmUsIGlmIHRoZSBzdGF0aW9uIGRpc2Nvbm5lY3RzIG9yIHNvbWV0aGluZyB3aGlsZSB0
aGVyZSBhcmUgZnJhbWVzIGZvciBpdCBvbiB0aGUgcXVldWU/DQpJT1csIHRoZSAianVzdCBza2lw
IHRoYXQgZnJhbWUiIHBpZWNlIHNlZW1zIGEgYml0IHF1ZXN0aW9uYWJsZS4NCg0KW0FTXSBZZWFo
LCBJIHJlYWxpemVkIHRoaXMgcGFyZW50aGVzaXMvaW5kZW50YXRpb24gaXNzdWUgKHdvdWxkIGZp
eCBpdCkuQlRXIHdlIGRpZCB0ZXN0IHRoaXMgb3V0IGFuZCBkaWRu4oCZdCBvYnNlcnZlIGFueSBz
dWNoIGlzc3VlLiBDYW4geW91IHBsZWFzZSBoZWxwIG1lIHVuZGVyc3RhbmQgdGhlIGZsb3cgd2hp
Y2ggY291bGQgbGVhZCB0byB0aGUgc2FtZT8gQWxzbyBpbiBjYXNlIHRoaXMgaXMgYW4gaXNzdWUs
IGNhbiB3ZSB0YWtlIGNhcmUgb2YgdGhpcyBpbiB0aGUgY2xlYW51cCByZWxhdGVkIHRvIGRpc2Nv
bm5lY3Q/IEhlcmUgaXQgc2VlbXMgYSBjb25zY2lvdXMgZWZmb3J0IGhhcyBiZWVuIG1hZGUgdG8g
YXZvaWQgc3BpbmxvY2sgKHJ4LT5sb2NhbC0+cnhfc2tiX3F1ZXVlLmxvY2spLCBhcyB0aGlzIGxv
Y2sgaXMgdGFrZW4gb25seSBmb3IgdGhlIGR1cmF0aW9uIG9mIGRlcXVldWUuIFRoZSBzdWdnZXN0
ZWQgc29sdXRpb24gYXZvaWRzIHVzaW5nIHNwaW5sb2NrLg0KDQpDaHJpc3RpYW4sIGlzIHRoZXJl
IGFueSByZWFzb24gdG8gbm90IGp1c3QgaGF2ZSB0aGUgcXVldWUgYmUgb24gdGhlIHN0YWNrLCBh
bmQgdXNlIGEgc2VwYXJhdGUgc3BpbmxvY2sgaW4gdGhlIGxvY2FsIHN0cnVjdCB0byBsb2NrIG91
dCB0aGUgdW53YW50ZWQgY29uY3VycmVuY3k/IEl0IHNlZW1zIHRvIG1lIHRoYXQgc2hvdWxkIHdv
cmsganVzdCBhcyB3ZWxsLCBzaW5jZSB0aGVyZSBhcmUgbmV2ZXIgZnJhbWVzIG9uIHRoZSByeF9z
a2JfcXVldWUgZm9yIHZlcnkgbG9uZywgcmlnaHQ/DQoNCmpvaGFubmVzDQoNCg==
On Wednesday, February 06, 2013 07:56:46 AM Amit SHAKYA wrote:
> From: Johannes Berg [mailto:[email protected]]
> On Mon, 2013-02-04 at 16:48 +0530, Amit Shakya wrote:
> > @@ -2790,7 +2791,20 @@ static void ieee80211_rx_handlers(struct
> > ieee80211_rx_data *rx)
> >
> > rx->local->running_rx_handler = true;
> >
> > - while ((skb = __skb_dequeue(&rx->local->rx_skb_queue))) {
> > + skb_queue_walk_safe(&rx->local->rx_skb_queue, skb, tmp) {
> > + if (!skb)
> > + break;
> > + hdr = (struct ieee80211_hdr *) skb->data;
> > + /*
> > + * Additional check to ensure that the packets corresponding
> > + * to same sta entry as in rx->sta are de-queued. The queue
> > + * can have different interface packets in case of multiple vifs
> > + */
> > + if ((rx->sta && hdr) && (ieee80211_is_data(hdr->frame_control))
> > + && (memcmp(rx->sta->sta.addr, hdr->addr2, ETH_ALEN)))
> > + continue;
> > + __skb_unlink(skb, &rx->local->rx_skb_queue);
> I wonder if this could lead to leaking frames here, if the station
> disconnects or something while there are frames for it on the queue?
> IOW, the "just skip that frame" piece seems a bit questionable.
>
>[AS] BTW we did test this out and didn’t observe any such issue. Can you
> please help me understand the flow which could lead to the same?
I read it like this: If a station suddenly disappears (for good) while
it still has some data in the reorder buffer, the reorder release timer
will put these orphaned frames into rx_skb_queue.
With this patch, they will never be cleared from the queue, until
ieee80211_unregister_hw is called [when the device is unregistered].
So, you would need to go through the rx_skb_queue everytime a HT
station is torn down and remove the affected frames from there.
> Also in case this is an issue, can we take care of this in the cleanup
> related to disconnect?
Sure, you could do that in ieee80211_sta_tear_down_BA_sessions. But you
don't need to. On Monday, I posted a patch:
<http://www.spinics.net/lists/linux-wireless/msg102725.html>
it should take care of the issue. So, can you test it please?
> Here it seems a conscious effort has been made to avoid spinlock
> (rx->local->rx_skb_queue.lock), as this lock is taken only for the
> duration of dequeue. The suggested solution avoids using spinlock.
Oh no, the locking is there. skb_unlink is defined in net/core/skbuff.c
as a spin_lock wrapped __skb_unlink. The same is true for skb_queue_tail
and __skb_queue_tail. (Or are you talking about something else?)
Regards
Christian
SGkgQ2hyaXN0aWFuLA0KDQpBY3R1YWxseSBJIGRlYnVnZ2VkIHRoaXMgaXNzdWUgcXVpdGUgc29t
ZSB0aW1lIGJhY2sgZm9yIGEgY3VzdG9tZXIgcHJvamVjdC4gTm93IEkgZG9u4oCZdCBoYXZlIHRo
ZSBzZXR1cCBhbHNvIGFzIEkgaGF2ZSBtb3ZlZCBvdXQgb2YgdGhlIHByb2plY3QuDQoNClNvIEkg
YW0gYWZyYWlkIEkgd2lsbCBub3QgYmUgYWJsZSB0byB0ZXN0IGl0Lg0KDQpSZWdhcmRzDQpBbWl0
DQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBDaHJpc3RpYW4gTGFtcGFydGVy
IFttYWlsdG86Y2h1bmtlZXlAZ29vZ2xlbWFpbC5jb21dIA0KU2VudDogV2VkbmVzZGF5LCBGZWJy
dWFyeSAwNiwgMjAxMyA3OjAzIFBNDQpUbzogQW1pdCBTSEFLWUENCkNjOiBKb2hhbm5lcyBCZXJn
OyBKb2huIFcuIExpbnZpbGxlOyBsaW51eC13aXJlbGVzcw0KU3ViamVjdDogUmU6IFtQQVRDSF0g
bWFjODAyMTE6IEZpeCBQTiBjb3JydXB0aW9uIGluIGNhc2Ugb2YgbXVsdGlwbGUgdmlydHVhbCBp
bnRlcmZhY2UNCg0KT24gV2VkbmVzZGF5LCBGZWJydWFyeSAwNiwgMjAxMyAwNzo1Njo0NiBBTSBB
bWl0IFNIQUtZQSB3cm90ZToNCj4gRnJvbTogSm9oYW5uZXMgQmVyZyBbbWFpbHRvOmpvaGFubmVz
QHNpcHNvbHV0aW9ucy5uZXRdDQo+IE9uIE1vbiwgMjAxMy0wMi0wNCBhdCAxNjo0OCArMDUzMCwg
QW1pdCBTaGFreWEgd3JvdGU6DQo+ID4gQEAgLTI3OTAsNyArMjc5MSwyMCBAQCBzdGF0aWMgdm9p
ZCBpZWVlODAyMTFfcnhfaGFuZGxlcnMoc3RydWN0IA0KPiA+IGllZWU4MDIxMV9yeF9kYXRhICpy
eCkNCj4gPiAgDQo+ID4gIAlyeC0+bG9jYWwtPnJ1bm5pbmdfcnhfaGFuZGxlciA9IHRydWU7DQo+
ID4gIA0KPiA+IC0Jd2hpbGUgKChza2IgPSBfX3NrYl9kZXF1ZXVlKCZyeC0+bG9jYWwtPnJ4X3Nr
Yl9xdWV1ZSkpKSB7DQo+ID4gKwlza2JfcXVldWVfd2Fsa19zYWZlKCZyeC0+bG9jYWwtPnJ4X3Nr
Yl9xdWV1ZSwgc2tiLCB0bXApIHsNCj4gPiArCQlpZiAoIXNrYikNCj4gPiArCQkJYnJlYWs7DQo+
ID4gKwkJaGRyID0gKHN0cnVjdCBpZWVlODAyMTFfaGRyICopIHNrYi0+ZGF0YTsNCj4gPiArCQkv
Kg0KPiA+ICsJCSogQWRkaXRpb25hbCBjaGVjayB0byBlbnN1cmUgdGhhdCB0aGUgcGFja2V0cyBj
b3JyZXNwb25kaW5nDQo+ID4gKwkJKiB0byBzYW1lIHN0YSBlbnRyeSBhcyBpbiByeC0+c3RhIGFy
ZSBkZS1xdWV1ZWQuIFRoZSBxdWV1ZQ0KPiA+ICsJCSogY2FuIGhhdmUgZGlmZmVyZW50IGludGVy
ZmFjZSBwYWNrZXRzIGluIGNhc2Ugb2YgbXVsdGlwbGUgdmlmcw0KPiA+ICsJCSovDQo+ID4gKwkJ
aWYgKChyeC0+c3RhICYmIGhkcikgJiYgKGllZWU4MDIxMV9pc19kYXRhKGhkci0+ZnJhbWVfY29u
dHJvbCkpDQo+ID4gKwkJCSYmIChtZW1jbXAocngtPnN0YS0+c3RhLmFkZHIsIGhkci0+YWRkcjIs
IEVUSF9BTEVOKSkpDQo+ID4gKwkJCQkJY29udGludWU7DQo+ID4gKwkJX19za2JfdW5saW5rKHNr
YiwgJnJ4LT5sb2NhbC0+cnhfc2tiX3F1ZXVlKTsNCg0KPiBJIHdvbmRlciBpZiB0aGlzIGNvdWxk
IGxlYWQgdG8gbGVha2luZyBmcmFtZXMgaGVyZSwgaWYgdGhlIHN0YXRpb24gDQo+IGRpc2Nvbm5l
Y3RzIG9yIHNvbWV0aGluZyB3aGlsZSB0aGVyZSBhcmUgZnJhbWVzIGZvciBpdCBvbiB0aGUgcXVl
dWU/DQo+IElPVywgdGhlICJqdXN0IHNraXAgdGhhdCBmcmFtZSIgcGllY2Ugc2VlbXMgYSBiaXQg
cXVlc3Rpb25hYmxlLg0KPiANCj5bQVNdIEJUVyB3ZSBkaWQgdGVzdCB0aGlzIG91dCBhbmQgZGlk
buKAmXQgb2JzZXJ2ZSBhbnkgc3VjaCBpc3N1ZS4gQ2FuIHlvdQ0KPiAgICAgcGxlYXNlIGhlbHAg
bWUgdW5kZXJzdGFuZCB0aGUgZmxvdyB3aGljaCBjb3VsZCBsZWFkIHRvIHRoZSBzYW1lPyANCkkg
cmVhZCBpdCBsaWtlIHRoaXM6IElmIGEgc3RhdGlvbiBzdWRkZW5seSBkaXNhcHBlYXJzIChmb3Ig
Z29vZCkgd2hpbGUgaXQgc3RpbGwgaGFzIHNvbWUgZGF0YSBpbiB0aGUgcmVvcmRlciBidWZmZXIs
IHRoZSByZW9yZGVyIHJlbGVhc2UgdGltZXIgd2lsbCBwdXQgdGhlc2Ugb3JwaGFuZWQgZnJhbWVz
IGludG8gcnhfc2tiX3F1ZXVlLiANCldpdGggdGhpcyBwYXRjaCwgdGhleSB3aWxsIG5ldmVyIGJl
IGNsZWFyZWQgZnJvbSB0aGUgcXVldWUsIHVudGlsIGllZWU4MDIxMV91bnJlZ2lzdGVyX2h3IGlz
IGNhbGxlZCBbd2hlbiB0aGUgZGV2aWNlIGlzIHVucmVnaXN0ZXJlZF0uDQoNClNvLCB5b3Ugd291
bGQgbmVlZCB0byBnbyB0aHJvdWdoIHRoZSByeF9za2JfcXVldWUgZXZlcnl0aW1lIGEgSFQgc3Rh
dGlvbiBpcyB0b3JuIGRvd24gYW5kIHJlbW92ZSB0aGUgYWZmZWN0ZWQgZnJhbWVzIGZyb20gdGhl
cmUuDQoNCj4gICAgIEFsc28gaW4gY2FzZSB0aGlzIGlzIGFuIGlzc3VlLCBjYW4gd2UgdGFrZSBj
YXJlIG9mIHRoaXMgaW4gdGhlIGNsZWFudXANCj4gICAgIHJlbGF0ZWQgdG8gZGlzY29ubmVjdD8N
ClN1cmUsIHlvdSBjb3VsZCBkbyB0aGF0IGluIGllZWU4MDIxMV9zdGFfdGVhcl9kb3duX0JBX3Nl
c3Npb25zLiBCdXQgeW91IGRvbid0IG5lZWQgdG8uIE9uIE1vbmRheSwgSSBwb3N0ZWQgYSBwYXRj
aDoNCjxodHRwOi8vd3d3LnNwaW5pY3MubmV0L2xpc3RzL2xpbnV4LXdpcmVsZXNzL21zZzEwMjcy
NS5odG1sPg0KaXQgc2hvdWxkIHRha2UgY2FyZSBvZiB0aGUgaXNzdWUuIFNvLCBjYW4geW91IHRl
c3QgaXQgcGxlYXNlPyANCg0KPiAgICAgSGVyZSBpdCBzZWVtcyBhIGNvbnNjaW91cyBlZmZvcnQg
aGFzIGJlZW4gbWFkZSB0byBhdm9pZCBzcGlubG9jaw0KPiAgICAgKHJ4LT5sb2NhbC0+cnhfc2ti
X3F1ZXVlLmxvY2spLCBhcyB0aGlzIGxvY2sgaXMgdGFrZW4gb25seSBmb3IgdGhlDQo+ICAgICBk
dXJhdGlvbiBvZiBkZXF1ZXVlLiBUaGUgc3VnZ2VzdGVkIHNvbHV0aW9uIGF2b2lkcyB1c2luZyBz
cGlubG9jay4NCk9oIG5vLCB0aGUgbG9ja2luZyBpcyB0aGVyZS4gc2tiX3VubGluayBpcyBkZWZp
bmVkIGluIG5ldC9jb3JlL3NrYnVmZi5jIGFzIGEgc3Bpbl9sb2NrIHdyYXBwZWQgX19za2JfdW5s
aW5rLiBUaGUgc2FtZSBpcyB0cnVlIGZvciBza2JfcXVldWVfdGFpbCBhbmQgX19za2JfcXVldWVf
dGFpbC4gKE9yIGFyZSB5b3UgdGFsa2luZyBhYm91dCBzb21ldGhpbmcgZWxzZT8pDQoNClJlZ2Fy
ZHMNCg0KQ2hyaXN0aWFuDQo=