Hi Christian, Actually I debugged this issue quite some time back for a customer project. Now I don’t have the setup also as I have moved out of the project. So I am afraid I will not be able to test it. Regards Amit -----Original Message----- From: Christian Lamparter [mailto:chunkeey@xxxxxxxxxxxxxx] Sent: Wednesday, February 06, 2013 7:03 PM To: Amit SHAKYA Cc: Johannes Berg; John W. Linville; linux-wireless Subject: Re: [PATCH] mac80211: Fix PN corruption in case of multiple virtual interface On Wednesday, February 06, 2013 07:56:46 AM Amit SHAKYA wrote: > From: Johannes Berg [mailto:johannes@xxxxxxxxxxxxxxxx] > 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 ��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
