Hi,
This patch fixes a some race conditions in IrLMP, and various
problems in IrLAP. The main IrLAP change is the addition of a new
events to make sure the stack doesn't deadlock if you connect while
the media is busy (this simple fix took me two days of work).
I didn't had too much time to test, but it seems solid. Please
send me feedback so that I get that in the kernel ASAP...
Have fun...
Jean
diff -u -p linux/include/net/irda/irlmp.d2.h linux/include/net/irda/irlmp.h
--- linux/include/net/irda/irlmp.d2.h Fri Nov 2 14:11:19 2001
+++ linux/include/net/irda/irlmp.h Fri Nov 2 18:37:29 2001
@@ -131,7 +131,6 @@ struct lap_cb {
struct irlap_cb *irlap; /* Instance of IrLAP layer */
hashbin_t *lsaps; /* LSAP associated with this link */
- int refcount;
__u8 caddr; /* Connection address */
__u32 saddr; /* Source device address */
diff -u -p linux/include/net/irda/irlmp_event.d2.h linux/include/net/irda/irlmp_event.h
--- linux/include/net/irda/irlmp_event.d2.h Fri Nov 2 18:47:47 2001
+++ linux/include/net/irda/irlmp_event.h Fri Nov 2 15:46:29 2001
@@ -103,10 +103,6 @@ void irlmp_watchdog_timer_expired(void *
void irlmp_discovery_timer_expired(void *data);
void irlmp_idle_timer_expired(void *data);
-void irlmp_next_station_state(IRLMP_STATE state);
-void irlmp_next_lsap_state(struct lsap_cb *self, LSAP_STATE state);
-void irlmp_next_lap_state(struct lap_cb *self, IRLMP_STATE state);
-
void irlmp_do_lap_event(struct lap_cb *self, IRLMP_EVENT event,
struct sk_buff *skb);
int irlmp_do_lsap_event(struct lsap_cb *self, IRLMP_EVENT event,
diff -u -p linux/include/net/irda/irlap_event.d2.h linux/include/net/irda/irlap_event.h
--- linux/include/net/irda/irlap_event.d2.h Fri Nov 2 18:47:12 2001
+++ linux/include/net/irda/irlap_event.h Fri Nov 2 18:11:20 2001
@@ -103,6 +103,7 @@ typedef enum {
DISCOVERY_TIMER_EXPIRED,
WD_TIMER_EXPIRED,
BACKOFF_TIMER_EXPIRED,
+ MEDIA_BUSY_TIMER_EXPIRED,
} IRLAP_EVENT;
/*
diff -u -p linux/net/irda/irlmp.d2.c linux/net/irda/irlmp.c
--- linux/net/irda/irlmp.d2.c Wed Oct 31 15:44:59 2001
+++ linux/net/irda/irlmp.c Fri Nov 2 16:53:17 2001
@@ -75,7 +75,7 @@ int irlmp_proc_read(char *buf, char **st
*/
int __init irlmp_init(void)
{
- IRDA_DEBUG(0, __FUNCTION__ "()\n");
+ IRDA_DEBUG(1, __FUNCTION__ "()\n");
/* Initialize the irlmp structure. */
irlmp = kmalloc( sizeof(struct irlmp_cb), GFP_KERNEL);
if (irlmp == NULL)
@@ -170,14 +170,14 @@ struct lsap_cb *irlmp_open_lsap(__u8 sls
#endif /* CONFIG_IRDA_ULTRA */
} else
self->dlsap_sel = LSAP_ANY;
- self->connected = FALSE;
+ /* self->connected = FALSE; -> already NULL via memset() */
init_timer(&self->watchdog_timer);
ASSERT(notify->instance != NULL, return NULL;);
self->notify = *notify;
- irlmp_next_lsap_state(self, LSAP_DISCONNECTED);
+ self->lsap_state = LSAP_DISCONNECTED;
/* Insert into queue of unconnected LSAPs */
hashbin_insert(irlmp->unconnected_lsaps, (irda_queue_t *) self, (int) self,
@@ -237,6 +237,7 @@ void irlmp_close_lsap(struct lsap_cb *se
ASSERT(lap->magic == LMP_LAP_MAGIC, return;);
lsap = hashbin_remove(lap->lsaps, (int) self, NULL);
}
+ self->lap = NULL;
/* Check if we found the LSAP! If not then try the unconnected lsaps */
if (!lsap) {
lsap = hashbin_remove(irlmp->unconnected_lsaps, (int) self,
@@ -281,7 +282,7 @@ void irlmp_register_link(struct irlap_cb
lap->daddr = DEV_ADDR_ANY;
lap->lsaps = hashbin_new(HB_GLOBAL);
- irlmp_next_lap_state(lap, LAP_STANDBY);
+ lap->lap_state = LAP_STANDBY;
init_timer(&lap->idle_timer);
@@ -346,7 +347,7 @@ int irlmp_connect_request(struct lsap_cb
"(), slsap_sel=%02x, dlsap_sel=%02x, saddr=%08x, daddr=%08x\n",
self->slsap_sel, dlsap_sel, saddr, daddr);
- if (self->connected)
+ if (test_bit(0, &self->connected))
return -EISCONN;
/* Client must supply destination device address */
@@ -435,7 +436,7 @@ int irlmp_connect_request(struct lsap_cb
hashbin_insert(self->lap->lsaps, (irda_queue_t *) self, (int) self, NULL);
- self->connected = TRUE;
+ set_bit(0, &self->connected); /* TRUE */
/*
* User supplied qos specifications?
@@ -481,6 +482,8 @@ void irlmp_connect_indication(struct lsa
self->notify.connect_indication(self->notify.instance, self,
&self->qos, max_seg_size,
max_header_size, skb);
+ else
+ dev_kfree_skb(skb);
}
/*
@@ -495,7 +498,7 @@ int irlmp_connect_response(struct lsap_c
ASSERT(self->magic == LMP_LSAP_MAGIC, return -1;);
ASSERT(userdata != NULL, return -1;);
- self->connected = TRUE;
+ set_bit(0, &self->connected); /* TRUE */
IRDA_DEBUG(2, __FUNCTION__ "(), slsap_sel=%02x, dlsap_sel=%02x\n",
self->slsap_sel, self->dlsap_sel);
@@ -543,7 +546,8 @@ void irlmp_connect_confirm(struct lsap_c
self->notify.connect_confirm(self->notify.instance, self,
&self->qos, max_seg_size,
max_header_size, skb);
- }
+ } else
+ dev_kfree_skb(skb);
}
/*
@@ -598,16 +602,18 @@ int irlmp_disconnect_request(struct lsap
ASSERT(self != NULL, return -1;);
ASSERT(self->magic == LMP_LSAP_MAGIC, return -1;);
+ ASSERT(userdata != NULL, return -1;);
- /* Already disconnected? */
- if (!self->connected) {
- WARNING(__FUNCTION__ "(), already disconnected!\n");
+ /* Already disconnected ?
+ * There is a race condition between irlmp_disconnect_indication()
+ * and us that might mess up the hashbins below. This fixes it.
+ * Jean II */
+ if (! test_and_clear_bit(0, &self->connected)) {
+ IRDA_DEBUG(0, __FUNCTION__ "(), already disconnected!\n");
+ dev_kfree_skb(userdata);
return -1;
}
- ASSERT(userdata != NULL, return -1;);
- ASSERT(self->connected == TRUE, return -1;);
-
skb_push(userdata, LMP_CONTROL_HEADER);
/*
@@ -634,7 +640,6 @@ int irlmp_disconnect_request(struct lsap
NULL);
/* Reset some values */
- self->connected = FALSE;
self->dlsap_sel = LSAP_ANY;
self->lap = NULL;
@@ -651,16 +656,23 @@ void irlmp_disconnect_indication(struct
{
struct lsap_cb *lsap;
- IRDA_DEBUG(1, __FUNCTION__ "(), reason=%s\n", lmp_reasons[reason]);
+ IRDA_DEBUG(1, __FUNCTION__ "(), reason=%s\n", lmp_reasons[reason]);
ASSERT(self != NULL, return;);
ASSERT(self->magic == LMP_LSAP_MAGIC, return;);
- ASSERT(self->connected == TRUE, return;);
IRDA_DEBUG(3, __FUNCTION__ "(), slsap_sel=%02x, dlsap_sel=%02x\n",
self->slsap_sel, self->dlsap_sel);
- self->connected = FALSE;
- self->dlsap_sel = LSAP_ANY;
+ /* Already disconnected ?
+ * There is a race condition between irlmp_disconnect_request()
+ * and us that might mess up the hashbins below. This fixes it.
+ * Jean II */
+ if (! test_and_clear_bit(0, &self->connected)) {
+ IRDA_DEBUG(0, __FUNCTION__ "(), already disconnected!\n");
+ if (userdata)
+ dev_kfree_skb(userdata);
+ return;
+ }
#ifdef CONFIG_IRDA_CACHE_LAST_LSAP
irlmp->cache.valid = FALSE;
@@ -679,6 +691,7 @@ void irlmp_disconnect_indication(struct
hashbin_insert(irlmp->unconnected_lsaps, (irda_queue_t *) lsap, (int) lsap,
NULL);
+ self->dlsap_sel = LSAP_ANY;
self->lap = NULL;
/*
@@ -689,7 +702,8 @@ void irlmp_disconnect_indication(struct
self, reason, userdata);
else {
IRDA_DEBUG(0, __FUNCTION__ "(), no handler\n");
- dev_kfree_skb(userdata);
+ if (userdata)
+ dev_kfree_skb(userdata);
}
}
@@ -1401,7 +1415,7 @@ __u32 irlmp_register_client(__u16 hint_m
irlmp_client_t *client;
__u32 handle;
- IRDA_DEBUG(0, __FUNCTION__ "()\n");
+ IRDA_DEBUG(1, __FUNCTION__ "()\n");
ASSERT(irlmp != NULL, return 0;);
/* Get a unique handle for this client */
@@ -1673,14 +1687,15 @@ int irlmp_proc_read(char *buf, char **st
len += sprintf(buf+len, "saddr: %#08x, daddr: %#08x, ",
lap->saddr, lap->daddr);
- len += sprintf(buf+len, "refcount: %d", lap->refcount);
+ len += sprintf(buf+len, "num lsaps: %d",
+ HASHBIN_GET_SIZE(lap->lsaps));
len += sprintf(buf+len, "\n");
- len += sprintf(buf+len, "\nConnected LSAPs:\n");
+ len += sprintf(buf+len, "\n Connected LSAPs:\n");
self = (struct lsap_cb *) hashbin_get_first(lap->lsaps);
while (self != NULL) {
ASSERT(self->magic == LMP_LSAP_MAGIC, return 0;);
- len += sprintf(buf+len, "lsap state: %s, ",
+ len += sprintf(buf+len, " lsap state: %s, ",
irlsap_state[ self->lsap_state]);
len += sprintf(buf+len,
"slsap_sel: %#02x, dlsap_sel: %#02x, ",
diff -u -p linux/net/irda/irlmp_event.d2.c linux/net/irda/irlmp_event.c
--- linux/net/irda/irlmp_event.d2.c Fri Nov 2 13:55:19 2001
+++ linux/net/irda/irlmp_event.c Fri Nov 2 19:06:59 2001
@@ -114,6 +114,25 @@ static int (*lsap_state[])( struct lsap_
irlmp_state_setup_pend
};
+static inline void irlmp_next_lap_state(struct lap_cb *self,
+ IRLMP_STATE state)
+{
+ /*
+ IRDA_DEBUG(4, __FUNCTION__ "(), LMP LAP = %s\n", irlmp_state[state]);
+ */
+ self->lap_state = state;
+}
+
+static inline void irlmp_next_lsap_state(struct lsap_cb *self,
+ LSAP_STATE state)
+{
+ /*
+ ASSERT(self != NULL, return;);
+ IRDA_DEBUG(4, __FUNCTION__ "(), LMP LSAP = %s\n", irlsap_state[state]);
+ */
+ self->lsap_state = state;
+}
+
/* Do connection control events */
int irlmp_do_lsap_event(struct lsap_cb *self, IRLMP_EVENT event,
struct sk_buff *skb)
@@ -223,7 +242,6 @@ static void irlmp_state_standby(struct l
IRDA_DEBUG(4, __FUNCTION__ "() LS_CONNECT_REQUEST\n");
irlmp_next_lap_state(self, LAP_U_CONNECT);
- self->refcount++;
/* FIXME: need to set users requested QoS */
irlap_connect_request(self->irlap, self->daddr, NULL, 0);
@@ -278,12 +296,12 @@ static void irlmp_state_u_connect(struct
* the lsaps may already have gone. This avoid getting stuck
* forever in LAP_ACTIVE state - Jean II */
if (HASHBIN_GET_SIZE(self->lsaps) == 0) {
+ IRDA_DEBUG(0, __FUNCTION__ "() NO LSAPs !\n");
irlmp_start_idle_timer(self, LM_IDLE_TIMEOUT);
}
break;
case LM_LAP_CONNECT_REQUEST:
/* Already trying to connect */
- self->refcount++;
break;
case LM_LAP_CONNECT_CONFIRM:
/* For all lsap_ce E Associated do LS_Connect_confirm */
@@ -298,12 +316,13 @@ static void irlmp_state_u_connect(struct
* the lsaps may already have gone. This avoid getting stuck
* forever in LAP_ACTIVE state - Jean II */
if (HASHBIN_GET_SIZE(self->lsaps) == 0) {
+ IRDA_DEBUG(0, __FUNCTION__ "() NO LSAPs !\n");
irlmp_start_idle_timer(self, LM_IDLE_TIMEOUT);
}
break;
case LM_LAP_DISCONNECT_INDICATION:
+ IRDA_DEBUG(1, __FUNCTION__ "(), LM_LAP_DISCONNECT_REQUEST\n");
irlmp_next_lap_state(self, LAP_STANDBY);
- self->refcount = 0;
/* Send disconnect event to all LSAPs using this link */
lsap = (struct lsap_cb *) hashbin_get_first( self->lsaps);
@@ -320,11 +339,13 @@ static void irlmp_state_u_connect(struct
}
break;
case LM_LAP_DISCONNECT_REQUEST:
- IRDA_DEBUG(4, __FUNCTION__ "(), LM_LAP_DISCONNECT_REQUEST\n");
+ IRDA_DEBUG(1, __FUNCTION__ "(), LM_LAP_DISCONNECT_REQUEST\n");
- self->refcount--;
- if (self->refcount == 0)
- irlmp_next_lap_state(self, LAP_STANDBY);
+ /* One of the LSAP did timeout or was closed, if it was
+ * the last one, try to get out of here - Jean II */
+ if (HASHBIN_GET_SIZE(self->lsaps) <= 1) {
+ irlap_disconnect_request(self->irlap);
+ }
break;
default:
IRDA_DEBUG(0, __FUNCTION__ "(), Unknown event %s\n",
@@ -352,7 +373,6 @@ static void irlmp_state_active(struct la
switch (event) {
case LM_LAP_CONNECT_REQUEST:
IRDA_DEBUG(4, __FUNCTION__ "(), LS_CONNECT_REQUEST\n");
- self->refcount++;
/*
* LAP connection allready active, just bounce back! Since we
@@ -379,8 +399,6 @@ static void irlmp_state_active(struct la
/* Keep state */
break;
case LM_LAP_DISCONNECT_REQUEST:
- self->refcount--;
-
/*
* Need to find out if we should close IrLAP or not. If there
* is only one LSAP connection left on this link, that LSAP
@@ -419,7 +437,6 @@ static void irlmp_state_active(struct la
break;
case LM_LAP_DISCONNECT_INDICATION:
irlmp_next_lap_state(self, LAP_STANDBY);
- self->refcount = 0;
/* In some case, at this point our side has already closed
* all lsaps, and we are waiting for the idle_timer to
@@ -517,6 +534,8 @@ static int irlmp_state_disconnected(stru
* If we receive this event while our LAP is closing down,
* the LM_LAP_CONNECT_REQUEST get lost and we get stuck in
* CONNECT_PEND state forever.
+ * The other cause of getting stuck down there is if the
+ * higher layer never reply to the CONNECT_INDICATION.
* Anyway, it make sense to make sure that we always have
* a backup plan. 1 second is plenty (should be immediate).
* Jean II */
@@ -577,9 +596,8 @@ static int irlmp_state_connect(struct ls
* Jean II */
IRDA_DEBUG(0, __FUNCTION__ "() WATCHDOG_TIMEOUT!\n");
- /* Here, we should probably disconnect proper */
+ /* Disconnect, get out... - Jean II */
self->dlsap_sel = LSAP_ANY;
- self->conn_skb = NULL;
irlmp_next_lsap_state(self, LSAP_DISCONNECTED);
break;
default:
@@ -612,15 +630,6 @@ static int irlmp_state_connect_pend(stru
case LM_CONNECT_REQUEST:
/* Keep state */
break;
- case LM_CONNECT_INDICATION:
- /* Will happen in some rare cases when the socket get stuck,
- * the other side retries the connect request.
- * We just unstuck the socket - Jean II */
- IRDA_DEBUG(0, __FUNCTION__ "(), LM_CONNECT_INDICATION, "
- "LSAP stuck in CONNECT_PEND state...\n");
- /* Keep state */
- irlmp_do_lap_event(self->lap, LM_LAP_CONNECT_REQUEST, NULL);
- break;
case LM_CONNECT_RESPONSE:
IRDA_DEBUG(0, __FUNCTION__ "(), LM_CONNECT_RESPONSE, "
"no indication issued yet\n");
@@ -648,6 +657,8 @@ static int irlmp_state_connect_pend(stru
/* Go back to disconnected mode, keep the socket waiting */
self->dlsap_sel = LSAP_ANY;
+ if(self->conn_skb)
+ dev_kfree_skb(self->conn_skb);
self->conn_skb = NULL;
irlmp_next_lsap_state(self, LSAP_DISCONNECTED);
break;
@@ -856,7 +867,7 @@ static int irlmp_state_setup_pend(struct
irlmp_next_lsap_state(self, LSAP_SETUP);
break;
case LM_WATCHDOG_TIMEOUT:
- IRDA_DEBUG(0, __FUNCTION__ "() WATCHDOG_TIMEOUT!\n");
+ IRDA_DEBUG(0, __FUNCTION__ "() : WATCHDOG_TIMEOUT !\n");
ASSERT(self->lap != NULL, return -1;);
irlmp_do_lap_event(self->lap, LM_LAP_DISCONNECT_REQUEST, NULL);
@@ -881,18 +892,4 @@ static int irlmp_state_setup_pend(struct
break;
}
return ret;
-}
-
-void irlmp_next_lap_state(struct lap_cb *self, IRLMP_STATE state)
-{
- IRDA_DEBUG(4, __FUNCTION__ "(), LMP LAP = %s\n", irlmp_state[state]);
- self->lap_state = state;
-}
-
-void irlmp_next_lsap_state(struct lsap_cb *self, LSAP_STATE state)
-{
- ASSERT(self != NULL, return;);
-
- IRDA_DEBUG(4, __FUNCTION__ "(), LMP LSAP = %s\n", irlsap_state[state]);
- self->lsap_state = state;
}
diff -u -p linux/net/irda/timer.d2.c linux/net/irda/timer.c
--- linux/net/irda/timer.d2.c Fri Nov 2 18:04:22 2001
+++ linux/net/irda/timer.c Fri Nov 2 19:03:02 2001
@@ -106,17 +106,13 @@ void irlap_stop_mbusy_timer(struct irlap
if(timer_pending(&self->media_busy_timer))
del_timer(&self->media_busy_timer);
-#ifdef CONFIG_IRDA_ULTRA
- /* Send any pending Ultra frames if any */
- if (!skb_queue_empty(&self->txq_ultra))
- /* Note : we don't send the frame, just post an event.
- * Frames will be sent only if we are in NDM mode (see
- * irlap_event.c).
- * Also, moved this code from irlap_media_busy_expired()
- * to here to catch properly all cases...
- * Jean II */
- irlap_do_event(self, SEND_UI_FRAME, NULL, NULL);
-#endif /* CONFIG_IRDA_ULTRA */
+ /* If we are in NDM, there is a bunch of events in LAP that
+ * that be pending due to the media_busy condition, such as
+ * CONNECT_REQUEST and SEND_UI_FRAME. If we don't generate
+ * an event, they will wait forever...
+ * Jean II */
+ if (self->state == LAP_NDM)
+ irlap_do_event(self, MEDIA_BUSY_TIMER_EXPIRED, NULL, NULL);
}
void irlmp_start_watchdog_timer(struct lsap_cb *self, int timeout)
@@ -237,5 +233,6 @@ void irlap_media_busy_expired(void* data
ASSERT(self != NULL, return;);
irda_device_set_media_busy(self->netdev, FALSE);
- /* Note : will deal with Ultra frames */
+ /* Note : the LAP event will be send in irlap_stop_mbusy_timer(),
+ * to catch other cases and do it after clearing the mbusy flag */
}
diff -u -p linux/net/irda/irlap_event.d2.c linux/net/irda/irlap_event.c
--- linux/net/irda/irlap_event.d2.c Fri Nov 2 16:28:13 2001
+++ linux/net/irda/irlap_event.c Fri Nov 2 19:15:59 2001
@@ -114,6 +114,7 @@ static const char *irlap_event[] = {
"DISCOVERY_TIMER_EXPIRED",
"WD_TIMER_EXPIRED",
"BACKOFF_TIMER_EXPIRED",
+ "MEDIA_BUSY_TIMER_EXPIRED",
};
const char *irlap_state[] = {
@@ -264,6 +265,15 @@ void irlap_do_event(struct irlap_cb *sel
}
break;
case LAP_NDM:
+ /* This one *should* not pend in this state, except if a
+ * socket try to connect and immediately disconnected.
+ * clear - Jean II */
+ if (self->disconnect_pending) {
+ IRDA_DEBUG(0, __FUNCTION__ "(), Clear pending disconnect!\n");
+ self->disconnect_pending = FALSE;
+ self->connect_pending = FALSE;
+ irlap_disconnect_indication(self, LAP_DISC_INDICATION);
+ }
/* Check if we should try to connect */
if ((self->connect_pending) && !self->media_busy) {
self->connect_pending = FALSE;
@@ -440,6 +450,20 @@ static int irlap_state_ndm(struct irlap_
irlap_discovery_indication(self, info->discovery);
}
break;
+ case MEDIA_BUSY_TIMER_EXPIRED:
+ /* A bunch of events may be postponed because the media is busy
+ * This event indicate the the media is free, therefore
+ * we can process those events... - Jean II */
+#ifdef CONFIG_IRDA_ULTRA
+ /* Send any pending Ultra frames if any */
+ if (!skb_queue_empty(&self->txq_ultra))
+ /* We don't send the frame, just post an event.
+ * Also, previously this code was in timer.c...
+ * Jean II */
+ ret = (*state[self->state])(self, SEND_UI_FRAME,
+ NULL, NULL);
+#endif /* CONFIG_IRDA_ULTRA */
+ break;
#ifdef CONFIG_IRDA_ULTRA
case SEND_UI_FRAME:
/* Only allowed to repeat an operation twice */
@@ -735,8 +759,10 @@ static int irlap_state_conn(struct irlap
break;
case DISCONNECT_REQUEST:
+ IRDA_DEBUG(0, __FUNCTION__ "(), Disconnect request!\n");
irlap_send_dm_frame(self);
- irlap_next_state( self, LAP_CONN);
+ irlap_next_state( self, LAP_NDM);
+ irlap_disconnect_indication(self, LAP_DISC_INDICATION);
break;
default:
IRDA_DEBUG(1, __FUNCTION__ "(), Unknown event %d, %s\n", event,
@@ -1417,13 +1443,12 @@ static int irlap_state_nrm_p(struct irla
irlap_start_final_timer(self, self->final_timeout);
break;
case RECV_RD_RSP:
- IRDA_DEBUG(0, __FUNCTION__ "(), RECV_RD_RSP\n");
+ IRDA_DEBUG(1, __FUNCTION__ "(), RECV_RD_RSP\n");
- irlap_next_state(self, LAP_PCLOSE);
- irlap_send_disc_frame(self);
irlap_flush_all_queues(self);
- irlap_start_final_timer(self, self->final_timeout);
- self->retry_count = 0;
+ irlap_next_state(self, LAP_XMIT_P);
+ /* Call back the LAP state machine to do a proper disconnect */
+ irlap_disconnect_request(self);
break;
default:
IRDA_DEBUG(1, __FUNCTION__ "(), Unknown event %s\n",
