[PATCH] : ir256_lap_icmd_fix-4.diff

ir256_lap_icmd_fix-4.diff :
 -------------------------
	o [CORRECT] Fix Tx queue handling (remove race, keep packets in order)
	o [CORRECT] Synchronise window_size & line_capacity and make sure
	  we never forget to increase them (would stall Tx queue)
	o [FEATURE] Group common code out of if-then-else
	o [FEATURE] Don't harcode LAP header size, use proper constant
	o [FEATURE] Inline irlap_next_state() to decrease bloat

include/net/irda/irlap.h

@@ -11,6 +11,7 @@
  * 
  *     Copyright (c) 1998-1999 Dag Brattli <dagb@cs.uit.no>, 
  *     All Rights Reserved.
+ *     Copyright (c) 2000-2001 Jean Tourrilhes <jt@hpl.hp.com>
  *     
  *     This program is free software; you can redistribute it and/or 
  *     modify it under the terms of the GNU General Public License as 
@@ -44,6 +45,7 @@
 #define LAP_ADDR_HEADER 1  /* IrLAP Address Header */
 #define LAP_CTRL_HEADER 1  /* IrLAP Control Header */
 
+/* May be different when we get VFIR */
 #define LAP_MAX_HEADER (LAP_ADDR_HEADER + LAP_CTRL_HEADER)
 
 #define BROADCAST  0xffffffff /* Broadcast device address */
@@ -210,9 +212,8 @@ void irlap_wait_min_turn_around(struct irlap_cb *, struct qos_info *);
 void irlap_init_qos_capabilities(struct irlap_cb *, struct qos_info *);
 void irlap_apply_default_connection_parameters(struct irlap_cb *self);
 void irlap_apply_connection_parameters(struct irlap_cb *self, int now);
-void irlap_set_local_busy(struct irlap_cb *self, int status);
 
-#define IRLAP_GET_HEADER_SIZE(self) 2 /* Will be different when we get VFIR */
+#define IRLAP_GET_HEADER_SIZE(self) (LAP_MAX_HEADER)
 #define IRLAP_GET_TX_QUEUE_LEN(self) skb_queue_len(&self->txq)
 
 #endif

include/net/irda/irlap_event.h

@@ -134,7 +134,6 @@ extern const char *irlap_state[];
 
 void irlap_do_event(struct irlap_cb *self, IRLAP_EVENT event, 
 		    struct sk_buff *skb, struct irlap_info *info);
-void irlap_next_state(struct irlap_cb *self, IRLAP_STATE state);
 void irlap_print_event(IRLAP_EVENT event);
 
 extern int irlap_qos_negotiate(struct irlap_cb *self, struct sk_buff *skb);

net/irda/irlap.c

@@ -131,7 +131,7 @@ struct irlap_cb *irlap_open(struct net_device *dev, struct qos_info *qos,
 	/* FIXME: should we get our own field? */
 	dev->atalk_ptr = self;
 
-	irlap_next_state(self, LAP_OFFLINE);
+	self->state = LAP_OFFLINE;
 
 	/* Initialize transmit queue */
 	skb_queue_head_init(&self->txq);
@@ -155,7 +155,7 @@ struct irlap_cb *irlap_open(struct net_device *dev, struct qos_info *qos,
 
 	self->N3 = 3; /* # connections attemts to try before giving up */
 	
-	irlap_next_state(self, LAP_NDM);
+	self->state = LAP_NDM;
 
 	hashbin_insert(irlap, (irda_queue_t *) self, self->saddr, NULL);
 
@@ -346,25 +346,21 @@ void irlap_data_request(struct irlap_cb *self, struct sk_buff *skb,
 	else
 		skb->data[1] = I_FRAME;
 
+	/* Add at the end of the queue (keep ordering) - Jean II */
+	skb_queue_tail(&self->txq, skb);
+
 	/* 
 	 *  Send event if this frame only if we are in the right state 
 	 *  FIXME: udata should be sent first! (skb_queue_head?)
 	 */
   	if ((self->state == LAP_XMIT_P) || (self->state == LAP_XMIT_S)) {
-		/*
-		 *  Check if the transmit queue contains some unsent frames,
-		 *  and if so, make sure they are sent first
-		 */
-		if (!skb_queue_empty(&self->txq)) {
-			skb_queue_tail(&self->txq, skb);
-			skb = skb_dequeue(&self->txq);
-			
-			ASSERT(skb != NULL, return;);
-		}
-		irlap_do_event(self, SEND_I_CMD, skb, NULL);
-		kfree_skb(skb);
-	} else
-		skb_queue_tail(&self->txq, skb);
+		/* If we are not already processing the Tx queue, trigger
+		 * transmission immediately - Jean II */
+		if((skb_queue_len(&self->txq) <= 1) && (!self->local_busy))
+			irlap_do_event(self, DATA_REQUEST, skb, NULL);
+		/* Otherwise, the packets will be sent normally at the
+		 * next pf-poll - Jean II */
+	}
 }
 
 /*
@@ -1013,6 +1009,7 @@ void irlap_apply_connection_parameters(struct irlap_cb *self, int now)
 	self->window_size = self->qos_tx.window_size.value;
 	self->window      = self->qos_tx.window_size.value;
 
+#ifdef CONFIG_IRDA_DYNAMIC_WINDOW
 	/*
 	 *  Calculate how many bytes it is possible to transmit before the
 	 *  link must be turned around
@@ -1020,6 +1017,8 @@ void irlap_apply_connection_parameters(struct irlap_cb *self, int now)
 	self->line_capacity = 
 		irlap_max_line_capacity(self->qos_tx.baud_rate.value,
 					self->qos_tx.max_turn_time.value);
+	self->bytes_left = self->line_capacity;
+#endif /* CONFIG_IRDA_DYNAMIC_WINDOW */
 
 	
 	/* 
@@ -1082,24 +1081,6 @@ void irlap_apply_connection_parameters(struct irlap_cb *self, int now)
 	IRDA_DEBUG(4, "Setting N2 = %d\n", self->N2);
 }
 
-/*
- * Function irlap_set_local_busy (self, status)
- *
- *    
- *
- */
-void irlap_set_local_busy(struct irlap_cb *self, int status)
-{
-	IRDA_DEBUG(0, __FUNCTION__ "()\n");
-
-	self->local_busy = status;
-	
-	if (status)
-		IRDA_DEBUG(0, __FUNCTION__ "(), local busy ON\n");
-	else
-		IRDA_DEBUG(0, __FUNCTION__ "(), local busy OFF\n");
-}
-
 #ifdef CONFIG_PROC_FS
 /*
  * Function irlap_proc_read (buf, start, offset, len, unused)

net/irda/irlap_event.c

@@ -252,6 +252,9 @@ void irlap_do_event(struct irlap_cb *self, IRLAP_EVENT event,
 		 * that will change the state away form XMIT
 		 */
 		if (skb_queue_len(&self->txq)) {
+			/* Prevent race conditions with irlap_data_request() */
+			self->local_busy = TRUE;
+
 			/* Try to send away all queued data frames */
 			while ((skb = skb_dequeue(&self->txq)) != NULL) {
 				ret = (*state[self->state])(self, SEND_I_CMD,
@@ -260,6 +263,8 @@ void irlap_do_event(struct irlap_cb *self, IRLAP_EVENT event,
 				if (ret == -EPROTO)
 					break; /* Try again later! */
 			}
+			/* Finished transmitting */
+			self->local_busy = FALSE;
 		} else if (self->disconnect_pending) {
 			self->disconnect_pending = FALSE;
 			
@@ -282,25 +287,15 @@ void irlap_do_event(struct irlap_cb *self, IRLAP_EVENT event,
  *    Switches state and provides debug information
  *
  */
-void irlap_next_state(struct irlap_cb *self, IRLAP_STATE state) 
+static inline void irlap_next_state(struct irlap_cb *self, IRLAP_STATE state) 
 {	
+	/*
 	if (!self || self->magic != LAP_MAGIC)
 		return;
 	
 	IRDA_DEBUG(4, "next LAP state = %s\n", irlap_state[state]);
-
+	*/
 	self->state = state;
-
-#ifdef CONFIG_IRDA_DYNAMIC_WINDOW
-	/*
-	 *  If we are swithing away from a XMIT state then we are allowed to 
-	 *  transmit a maximum number of bytes again when we enter the XMIT 
-	 *  state again. Since its possible to "switch" from XMIT to XMIT,
-	 *  we cannot do this when swithing into the XMIT state :-)
-	 */
-	if ((state != LAP_XMIT_P) && (state != LAP_XMIT_S))
-		self->bytes_left = self->line_capacity;
-#endif /* CONFIG_IRDA_DYNAMIC_WINDOW */
 }
 
 /*
@@ -1017,6 +1012,12 @@ static int irlap_state_xmit_p(struct irlap_cb *self, IRLAP_EVENT event,
 		IRDA_DEBUG(3, __FUNCTION__ "(), POLL_TIMER_EXPIRED (%ld)\n",
 			   jiffies);
 		irlap_send_rr_frame(self, CMD_FRAME);
+		/* Return to NRM properly - Jean II  */
+		self->window = self->window_size;
+#ifdef CONFIG_IRDA_DYNAMIC_WINDOW
+		/* Allowed to transmit a maximum number of bytes again. */
+		self->bytes_left = self->line_capacity;
+#endif /* CONFIG_IRDA_DYNAMIC_WINDOW */
 		irlap_start_final_timer(self, self->final_timeout);
 		irlap_next_state(self, LAP_NRM_P);
 		break;
@@ -1029,6 +1030,10 @@ static int irlap_state_xmit_p(struct irlap_cb *self, IRLAP_EVENT event,
 		self->retry_count = 0;
 		irlap_next_state(self, LAP_PCLOSE);
 		break;
+	case DATA_REQUEST:
+		/* Nothing to do, irlap_do_event() will send the packet
+		 * when we return... - Jean II */
+		break;
 	default:
 		IRDA_DEBUG(0, __FUNCTION__ "(), Unknown event %s\n", 
 			   irlap_event[event]);
@@ -1645,12 +1650,17 @@ static int irlap_state_xmit_s(struct irlap_cb *self, IRLAP_EVENT event,
 			 */
 			if (skb->len > self->bytes_left) {
 				skb_queue_head(&self->txq, skb_get(skb));
+
 				/*
 				 *  Switch to NRM_S, this is only possible
 				 *  when we are in secondary mode, since we 
 				 *  must be sure that we don't miss any RR
 				 *  frames
 				 */
+				self->window = self->window_size;
+				self->bytes_left = self->line_capacity;
+				irlap_start_wd_timer(self, self->wd_timeout);
+
 				irlap_next_state(self, LAP_NRM_S);
 
 				return -EPROTO; /* Try again later */
@@ -1688,6 +1698,10 @@ static int irlap_state_xmit_s(struct irlap_cb *self, IRLAP_EVENT event,
 		irlap_start_wd_timer(self, self->wd_timeout);
 		irlap_next_state(self, LAP_SCLOSE);
 		break;
+	case DATA_REQUEST:
+		/* Nothing to do, irlap_do_event() will send the packet
+		 * when we return... - Jean II */
+		break;
 	default:
 		IRDA_DEBUG(2, __FUNCTION__ "(), Unknown event %s\n", 
 			   irlap_event[event]);

net/irda/irlap_frame.c

@@ -768,6 +768,9 @@ void irlap_send_data_primary_poll(struct irlap_cb *self, struct sk_buff *skb)
 {
 	struct sk_buff *tx_skb;
 
+	/* Stop P timer */
+	del_timer(&self->poll_timer);
+		
 	/* Is this reliable or unreliable data? */
 	if (skb->data[1] == I_FRAME) {
 		
@@ -793,23 +796,15 @@ void irlap_send_data_primary_poll(struct irlap_cb *self, struct sk_buff *skb)
 		 *  skb, since retransmitted need to set or clear the poll
 		 *  bit depending on when they are sent.  
 		 */
-		/* Stop P timer */
-		del_timer(&self->poll_timer);
-		
 		tx_skb->data[1] |= PF_BIT;
 		
 		self->vs = (self->vs + 1) % 8;
 		self->ack_required = FALSE;
-		self->window = self->window_size;
-
-		irlap_start_final_timer(self, self->final_timeout);
 
 		irlap_send_i_frame(self, tx_skb, CMD_FRAME);
 	} else {
 		IRDA_DEBUG(4, __FUNCTION__ "(), sending unreliable frame\n");
 
-		del_timer(&self->poll_timer);
-
 		if (self->ack_required) {
 			irlap_send_ui_frame(self, skb_get(skb), self->caddr, CMD_FRAME);
 			irlap_send_rr_frame(self, CMD_FRAME);
@@ -818,9 +813,15 @@ void irlap_send_data_primary_poll(struct irlap_cb *self, struct sk_buff *skb)
 			skb->data[1] |= PF_BIT;
 			irlap_send_ui_frame(self, skb_get(skb), self->caddr, CMD_FRAME);
 		}
-		self->window = self->window_size;
-		irlap_start_final_timer(self, self->final_timeout);
 	}
+
+	self->window = self->window_size;
+#ifdef CONFIG_IRDA_DYNAMIC_WINDOW
+	/* We are allowed to transmit a maximum number of bytes again. */
+	self->bytes_left = self->line_capacity;
+#endif /* CONFIG_IRDA_DYNAMIC_WINDOW */
+
+	irlap_start_final_timer(self, self->final_timeout);
 }
 
 /*
@@ -858,11 +859,8 @@ void irlap_send_data_secondary_final(struct irlap_cb *self,
 		tx_skb->data[1] |= PF_BIT;
 		
 		self->vs = (self->vs + 1) % 8; 
-		self->window = self->window_size;
 		self->ack_required = FALSE;
 		
-		irlap_start_wd_timer(self, self->wd_timeout);
-
 		irlap_send_i_frame(self, tx_skb, RSP_FRAME); 
 	} else {
 		if (self->ack_required) {
@@ -873,10 +871,15 @@ void irlap_send_data_secondary_final(struct irlap_cb *self,
 			skb->data[1] |= PF_BIT;
 			irlap_send_ui_frame(self, skb_get(skb), self->caddr, RSP_FRAME);
 		}
-		self->window = self->window_size;
-
-		irlap_start_wd_timer(self, self->wd_timeout);
 	}
+
+	self->window = self->window_size;
+#ifdef CONFIG_IRDA_DYNAMIC_WINDOW
+	/* We are allowed to transmit a maximum number of bytes again. */
+	self->bytes_left = self->line_capacity;
+#endif /* CONFIG_IRDA_DYNAMIC_WINDOW */
+
+	irlap_start_wd_timer(self, self->wd_timeout);
 }
 
 /*