Re: ipw2100: race between isr_indicate_associated and rx path

Am Mittwoch, 28. Januar 2009 schrieb Zhu, Yi:
> Do you mean the EAP response frame is handed to ieee80211/ipw2100 by
> wpa_supplicant but you didn't see it in the air? 

Exactly.

> I assume at this point, the BSSID is already got and the state is
> ASSOCIATED.

Yes.

> Can you confirm wpa_supplicant has already received the wext ASSOCIATED
> event? 

Yes.

> Anyway, I couldn't tell without seeing your patch ;-)      

Uhh, yes. Here it is :)

The patch is far from being ready, it's just a prototype. So, just ignore
typos, coding style etc.

Helmut


---

diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
index 823c2bf..36b5d52 100644
--- a/drivers/net/wireless/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/ipw2x00/ipw2100.c
@@ -2393,6 +2393,8 @@ static void isr_rx(struct ipw2100_priv *priv, int i,
 {
 	struct ipw2100_status *status = &priv->status_queue.drv[i];
 	struct ipw2100_rx_packet *packet = &priv->rx_buffers[i];
+	struct ipw2100_skb_buffer *skb_buff;
+
 
 	IPW_DEBUG_RX("Handler...\n");
 
@@ -2413,7 +2415,7 @@ static void isr_rx(struct ipw2100_priv *priv, int i,
 	}
 
 	if (unlikely(priv->ieee->iw_mode != IW_MODE_MONITOR &&
-		     !(priv->status & STATUS_ASSOCIATED))) {
+		     !(priv->status & (STATUS_ASSOCIATED | STATUS_ASSOCIATING)))) {
 		IPW_DEBUG_DROP("Dropping packet while not associated.\n");
 		priv->wstats.discard.misc++;
 		return;
@@ -2433,7 +2435,24 @@ static void isr_rx(struct ipw2100_priv *priv, int i,
 					     IPW_RX_NIC_BUFFER_LENGTH));
 #endif
 
-	if (!ieee80211_rx(priv->ieee, packet->skb, stats)) {
+	if (priv->status & STATUS_ASSOCIATING) {
+		/* firmware is already associated and might accept frames
+		 * but we the user space does not know about that state
+		 * yet. Hence buffer all frames until the status moves to
+		 * associated.
+		 */
+		skb_buff = (struct ipw2100_skb_buffer*)kmalloc(sizeof(struct ipw2100_skb_buffer), GFP_ATOMIC);
+
+		atomic_inc(&packet->skb->users);
+		INIT_LIST_HEAD(&skb_buff->list);
+		skb_buff->skb = packet->skb;
+		memcpy(&skb_buff->stats, stats, sizeof(struct ieee80211_rx_stats));
+		list_add_tail(&skb_buff->list, &priv->skb_buffer_list);
+
+		IPW_DEBUG_INFO("%s: FW is already associated, buffering packet.\n",
+			       priv->net_dev->name);
+
+	} else if (!ieee80211_rx(priv->ieee, packet->skb, stats)) {
 #ifdef IPW2100_RX_DEBUG
 		IPW_DEBUG_DROP("%s: Non consumed packet:\n",
 			       priv->net_dev->name);
@@ -6118,6 +6137,8 @@ static struct net_device *ipw2100_alloc_device(struct pci_dev *pci_dev,
 	INIT_LIST_HEAD(&priv->fw_pend_list);
 	INIT_STAT(&priv->fw_pend_stat);
 
+	INIT_LIST_HEAD(&priv->skb_buffer_list);
+
 	priv->workqueue = create_workqueue(DRV_NAME);
 
 	INIT_DELAYED_WORK(&priv->reset_work, ipw2100_reset_adapter);
@@ -8298,6 +8319,8 @@ static void ipw2100_wx_event_work(struct work_struct *work)
 		container_of(work, struct ipw2100_priv, wx_event_work.work);
 	union iwreq_data wrqu;
 	int len = ETH_ALEN;
+	bool test = false;
+	struct ipw2100_skb_buffer* skb_buffer;
 
 	if (priv->status & STATUS_STOPPING)
 		return;
@@ -8323,8 +8346,10 @@ static void ipw2100_wx_event_work(struct work_struct *work)
 		memcpy(priv->ieee->bssid, priv->bssid, ETH_ALEN);
 		priv->status &= ~STATUS_ASSOCIATING;
 		priv->status |= STATUS_ASSOCIATED;
+
 		netif_carrier_on(priv->net_dev);
 		netif_wake_queue(priv->net_dev);
+		test = true;
 	}
 
 	if (!(priv->status & STATUS_ASSOCIATED)) {
@@ -8341,6 +8366,31 @@ static void ipw2100_wx_event_work(struct work_struct *work)
 	}
 
 	wireless_send_event(priv->net_dev, SIOCGIWAP, &wrqu, NULL);
+
+	if (test) {
+		/* now deliver all buffered frames */
+		while ( !list_empty(&priv->skb_buffer_list) ) {
+			IPW_DEBUG_INFO("%s: Delivering queued packet.\n",
+				       priv->net_dev->name);
+
+			skb_buffer = list_entry(priv->skb_buffer_list.next, struct ipw2100_skb_buffer, list);
+			list_del(&skb_buffer->list);
+
+			if (!ieee80211_rx(priv->ieee, skb_buffer->skb, &skb_buffer->stats)) {
+/*#ifdef IPW2100_RX_DEBUG
+				IPW_DEBUG_DROP("%s: Non consumed packet:\n",
+			       priv->net_dev->name);
+				printk_buf(IPW_DL_DROP, packet_data, status->frame_size);
+#endif*/
+				priv->ieee->stats.rx_errors++;
+
+				/* ieee80211_rx failed, so it didn't free the SKB */
+				dev_kfree_skb_any(skb_buffer->skb);
+				skb_buffer->skb = NULL;
+			}
+			kfree(skb_buffer);
+		}
+	}
 }
 
 #define IPW2100_FW_MAJOR_VERSION 1
diff --git a/drivers/net/wireless/ipw2x00/ipw2100.h b/drivers/net/wireless/ipw2x00/ipw2100.h
index bbf1ddc..7c11c6a 100644
--- a/drivers/net/wireless/ipw2x00/ipw2100.h
+++ b/drivers/net/wireless/ipw2x00/ipw2100.h
@@ -487,6 +487,12 @@ enum {
 #define CAP_SHARED_KEY          (1<<0)	/* Off = OPEN */
 #define CAP_PRIVACY_ON          (1<<1)	/* Off = No privacy */
 
+struct ipw2100_skb_buffer {
+	struct list_head list;
+	struct sk_buff* skb;
+	struct ieee80211_rx_stats stats;
+};
+
 struct ipw2100_priv {
 
 	int stop_hang_check;	/* Set 1 when shutting down to kill hang_check */
@@ -601,6 +607,8 @@ struct ipw2100_priv {
 	struct mutex adapter_mutex;
 
 	wait_queue_head_t wait_command_queue;
+
+	struct list_head skb_buffer_list;
 };
 
 /*********************************************************

------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword