[PATCH] : ir256_lap_icmd_fix-4.diff

From: Jean Tourrilhes (jt@bougret.hpl.hp.com)
Date: Tue Mar 05 2002 - 17:36:03 EST


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

diff -u -p linux/include/net/irda/irlap.d4.h linux/include/net/irda/irlap.h
--- linux/include/net/irda/irlap.d4.h Tue Feb 26 09:49:07 2002
+++ linux/include/net/irda/irlap.h Mon Mar 4 14:46:16 2002
@@ -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 i
 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
diff -u -p linux/include/net/irda/irlap_event.d4.h linux/include/net/irda/irlap_event.h
--- linux/include/net/irda/irlap_event.d4.h Tue Feb 26 09:49:16 2002
+++ linux/include/net/irda/irlap_event.h Mon Mar 4 14:46:16 2002
@@ -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);
diff -u -p linux/net/irda/irlap.d4.c linux/net/irda/irlap.c
--- linux/net/irda/irlap.d4.c Tue Feb 26 09:49:38 2002
+++ linux/net/irda/irlap.c Tue Feb 26 09:51:26 2002
@@ -131,7 +131,7 @@ struct irlap_cb *irlap_open(struct net_d
         /* 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_d
 
         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
         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(s
         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(s
         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 */
 
         
         /*
@@ -1080,24 +1079,6 @@ void irlap_apply_connection_parameters(s
         self->N2 = self->qos_tx.link_disc_time.value * 1000 /
                 self->qos_rx.max_turn_time.value;
         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
diff -u -p linux/net/irda/irlap_event.d4.c linux/net/irda/irlap_event.c
--- linux/net/irda/irlap_event.d4.c Tue Feb 26 09:49:55 2002
+++ linux/net/irda/irlap_event.c Tue Feb 26 09:51:26 2002
@@ -252,6 +252,9 @@ void irlap_do_event(struct irlap_cb *sel
                  * 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 *sel
                                 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 *sel
  * 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 irl
                 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 irl
                 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 irl
                          */
                         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 */
@@ -1687,6 +1697,10 @@ static int irlap_state_xmit_s(struct irl
                 irlap_flush_all_queues(self);
                 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",
diff -u -p linux/net/irda/irlap_frame.d4.c linux/net/irda/irlap_frame.c
--- linux/net/irda/irlap_frame.d4.c Tue Feb 26 09:50:14 2002
+++ linux/net/irda/irlap_frame.c Tue Feb 26 09:51:26 2002
@@ -768,6 +768,9 @@ void irlap_send_data_primary_poll(struct
 {
         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
                  * 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
                         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(str
                 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(str
                         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);
 }
 
 /*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Mar 07 2002 - 21:00:49 EST