Hi,
This is a followup to :
http://www.pasta.cs.uit.no/pipermail/linux-irda/2001-January/002192.html
I updated my change speed patch to include the skb_free of the
empty packet. The stack doesn't dislike it so far, so I'm happy.
Patch attached...
Things I'm not happy about :
o mtt (min turn time) doesn't seem to be managed properly
around the speed change. In particular, after a immediate speed
change, it gets reset to zero, and the device doesn't seem to wait
until the other end has changed its speed. This is a bit too hairy for
me, Dag might want to look into that.
o nsc-ircc mess the last transmitted packet when returning
from FIR mode to SIR mode (deffered speed change). I looked at the
code, and I can't see what's wrong. I suspect a tricky bug in
nsc-ircc. Dag might rescue us on this one as well.
o Did I mention the xbofs stuff in wrapper.c ?
Have fun...
Jean
diff -u -p linux/include/net/irda/irda_device.d6.h linux/include/net/irda/irda_device.h
--- linux/include/net/irda/irda_device.d6.h Fri Dec 29 16:19:09 2000
+++ linux/include/net/irda/irda_device.h Tue Jan 2 13:16:56 2001
@@ -218,29 +218,54 @@ extern inline __u16 irda_get_mtt(struct
#endif
/*
- * Function irda_get_speed (skb)
+ * Function irda_get_next_speed (skb)
*
- * Extact the speed this frame should be sent out with from the skb
+ * Extract the speed that should be set *after* this frame from the skb
*
+ * Note : return -1 for user space frames
*/
-#define irda_get_speed(skb) ( \
+#define irda_get_next_speed(skb) ( \
(((struct irda_skb_cb*) skb->cb)->magic == LAP_MAGIC) ? \
- ((struct irda_skb_cb *)(skb->cb))->speed : 9600 \
+ ((struct irda_skb_cb *)(skb->cb))->next_speed : -1 \
)
#if 0
-extern inline __u32 irda_get_speed(struct sk_buff *skb)
+extern inline __u32 irda_get_next_speed(struct sk_buff *skb)
{
__u32 speed;
if (((struct irda_skb_cb *)(skb->cb))->magic != LAP_MAGIC)
- speed = 9600;
+ speed = -1;
else
- speed = ((struct irda_skb_cb *)(skb->cb))->speed;
+ speed = ((struct irda_skb_cb *)(skb->cb))->next_speed;
return speed;
}
#endif
+
+/*
+ * Function irda_get_next_xbofs (skb)
+ *
+ * Extract the xbofs that should be set for this frame from the skb
+ *
+ * Note : default to 12 for user space frames
+ */
+#define irda_get_xbofs(skb) ( \
+ (((struct irda_skb_cb*) skb->cb)->magic == LAP_MAGIC) ? \
+ ((struct irda_skb_cb *)(skb->cb))->xbofs : 10 \
+)
+
+/*
+ * Function irda_get_next_xbofs (skb)
+ *
+ * Extract the xbofs that should be set *after* this frame from the skb
+ *
+ * Note : return -1 for user space frames
+ */
+#define irda_get_next_xbofs(skb) ( \
+ (((struct irda_skb_cb*) skb->cb)->magic == LAP_MAGIC) ? \
+ ((struct irda_skb_cb *)(skb->cb))->next_xbofs : -1 \
+)
#endif /* IRDA_DEVICE_H */
diff -u -p linux/include/net/irda/irlap.d6.h linux/include/net/irda/irlap.h
--- linux/include/net/irda/irlap.d6.h Fri Dec 29 17:47:59 2000
+++ linux/include/net/irda/irlap.h Tue Jan 2 13:16:56 2001
@@ -168,7 +168,7 @@ struct irlap_cb {
hashbin_t *discovery_log;
discovery_t *discovery_cmd;
- __u32 speed;
+ __u32 speed; /* Link speed */
struct qos_info qos_tx; /* QoS requested by peer */
struct qos_info qos_rx; /* QoS requested by self */
@@ -179,6 +179,7 @@ struct irlap_cb {
int mtt_required; /* Minumum turnaround time required */
int xbofs_delay; /* Nr of XBOF's used to MTT */
int bofs_count; /* Negotiated extra BOFs */
+ int next_bofs; /* Negotiated extra BOFs after next frame */
#ifdef CONFIG_IRDA_COMPRESSION
struct irda_compressor compressor;
@@ -236,8 +237,8 @@ void irlap_change_speed(struct irlap_cb
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);
+void irlap_apply_default_connection_parameters(struct irlap_cb *self, int now);
+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 */
diff -u -p linux/include/net/irda/irda.d6.h linux/include/net/irda/irda.h
--- linux/include/net/irda/irda.d6.h Fri Dec 29 17:45:19 2000
+++ linux/include/net/irda/irda.h Tue Jan 2 13:16:56 2001
@@ -177,9 +177,10 @@ typedef union {
*/
struct irda_skb_cb {
magic_t magic; /* Be sure that we can trust the information */
- __u32 speed; /* The Speed this frame should be sent with */
+ __u32 next_speed; /* The Speed to be set *after* this frame */
__u16 mtt; /* Minimum turn around time */
__u16 xbofs; /* Number of xbofs required, used by SIR mode */
+ __u16 next_xbofs; /* Number of xbofs required *after* this frame */
void *context; /* May be used by drivers */
void (*destructor)(struct sk_buff *skb); /* Used for flow control */
__u16 xbofs_delay; /* Number of xbofs used for generating the mtt */
diff -u -p linux/net/irda/irlap.d6.c linux/net/irda/irlap.c
--- linux/net/irda/irlap.d6.c Fri Dec 29 17:22:55 2000
+++ linux/net/irda/irlap.c Tue Jan 2 10:31:28 2001
@@ -148,7 +148,7 @@ struct irlap_cb *irlap_open(struct net_d
init_timer(&self->backoff_timer);
init_timer(&self->media_busy_timer);
- irlap_apply_default_connection_parameters(self);
+ irlap_apply_default_connection_parameters(self, TRUE);
self->N3 = 3; /* # connections attemts to try before giving up */
@@ -998,26 +998,29 @@ void irlap_init_qos_capabilities(struct
}
/*
- * Function irlap_apply_default_connection_parameters (void)
+ * Function irlap_apply_default_connection_parameters (void, now)
*
* Use the default connection and transmission parameters
*
*/
-void irlap_apply_default_connection_parameters(struct irlap_cb *self)
+void irlap_apply_default_connection_parameters(struct irlap_cb *self, int now)
{
IRDA_DEBUG(4, __FUNCTION__ "()\n");
ASSERT(self != NULL, return;);
ASSERT(self->magic == LAP_MAGIC, return;);
- irlap_change_speed(self, 9600, TRUE);
+ /* xbofs : Default value in NDM */
+ self->next_bofs = 12;
+ if(now)
+ self->bofs_count = 12;
+
+ /* NDM Speed is 9600 */
+ irlap_change_speed(self, 9600, now);
/* Set mbusy when going to NDM state */
irda_device_set_media_busy(self->netdev, TRUE);
- /* Default value in NDM */
- self->bofs_count = 12;
-
/*
* Generate random connection address for this session, which must
* be 7 bits wide and different from 0x00 and 0xfe
@@ -1056,23 +1059,31 @@ void irlap_apply_default_connection_para
}
/*
- * Function irlap_apply_connection_parameters (qos)
+ * Function irlap_apply_connection_parameters (qos, now)
*
* Initialize IrLAP with the negotiated QoS values
*
+ * If 'now' is false, the speed and xbofs will be changed after the next
+ * frame is sent.
+ * If 'now' is true, the speed and xbofs is changed immediately
*/
-void irlap_apply_connection_parameters(struct irlap_cb *self)
+void irlap_apply_connection_parameters(struct irlap_cb *self, int now)
{
IRDA_DEBUG(4, __FUNCTION__ "()\n");
ASSERT(self != NULL, return;);
ASSERT(self->magic == LAP_MAGIC, return;);
- irlap_change_speed(self, self->qos_tx.baud_rate.value, FALSE);
+ /* Set the negociated xbofs value */
+ self->next_bofs = self->qos_tx.additional_bofs.value;
+ if(now)
+ self->bofs_count = self->next_bofs;
+
+ /* Set the negociated link speed (may need the new xbofs value) */
+ irlap_change_speed(self, self->qos_tx.baud_rate.value, now);
self->window_size = self->qos_tx.window_size.value;
self->window = self->qos_tx.window_size.value;
- self->bofs_count = self->qos_tx.additional_bofs.value;
/*
* Calculate how many bytes it is possible to transmit before the
diff -u -p linux/net/irda/irlap_frame.d6.c linux/net/irda/irlap_frame.c
--- linux/net/irda/irlap_frame.d6.c Fri Dec 29 17:23:23 2000
+++ linux/net/irda/irlap_frame.c Tue Jan 2 10:47:48 2001
@@ -60,7 +60,7 @@ static inline void irlap_insert_info(str
*/
cb->magic = LAP_MAGIC;
cb->mtt = self->mtt_required;
- cb->speed = self->speed;
+ cb->next_speed = self->speed;
/* Reset */
self->mtt_required = 0;
@@ -70,10 +70,13 @@ static inline void irlap_insert_info(str
* force the negotiated minimum turnaround time
*/
cb->xbofs = self->bofs_count;
+ cb->next_xbofs = self->next_bofs;
cb->xbofs_delay = self->xbofs_delay;
/* Reset XBOF's delay (used only for getting min turn time) */
self->xbofs_delay = 0;
+ /* Put the correct xbofs value for the next packet */
+ self->bofs_count = self->next_bofs;
}
/*
diff -u -p linux/net/irda/irlap_event.d6.c linux/net/irda/irlap_event.c
--- linux/net/irda/irlap_event.d6.c Fri Dec 29 17:31:37 2000
+++ linux/net/irda/irlap_event.c Tue Jan 2 10:45:13 2001
@@ -684,25 +684,26 @@ static int irlap_state_conn(struct irlap
irlap_initiate_connection_state(self);
-#if 0
/*
- * We are allowed to send two frames, but this may increase
- * the connect latency, so lets not do it for now.
+ * Applying the parameters now will make sure we change speed
+ * *after* we have sent the next frame
*/
- irlap_send_ua_response_frame(self, &self->qos_rx);
-#endif
+ irlap_apply_connection_parameters(self, FALSE);
/*
- * Applying the parameters now will make sure we change speed
- * after we have sent the next frame
+ * Sending this frame will force a speed change after it has
+ * been sent (i.e. the frame will be sent at 9600).
*/
- irlap_apply_connection_parameters(self);
+ irlap_send_ua_response_frame(self, &self->qos_rx);
+#if 0
/*
- * Sending this frame will force a speed change after it has
- * been sent
+ * We are allowed to send two frames, but this may increase
+ * the connect latency, so lets not do it for now.
*/
+ /* What the hell is this ? - Jean II */
irlap_send_ua_response_frame(self, &self->qos_rx);
+#endif
/*
* The WD-timer could be set to the duration of the P-timer
@@ -794,8 +795,9 @@ static int irlap_state_setup(struct irla
irlap_qos_negotiate(self, skb);
+ /* Send UA frame and then change link settings */
+ irlap_apply_connection_parameters(self, FALSE);
irlap_send_ua_response_frame(self, &self->qos_rx);
- irlap_apply_connection_parameters(self);
irlap_next_state(self, LAP_NRM_S);
irlap_connect_confirm(self, skb);
@@ -827,10 +829,11 @@ static int irlap_state_setup(struct irla
irlap_qos_negotiate(self, skb);
- irlap_apply_connection_parameters(self);
+ /* Set the new link setting *now* (before the rr frame) */
+ irlap_apply_connection_parameters(self, TRUE);
self->retry_count = 0;
- /* This frame will actually force the speed change */
+ /* This frame will actually be sent at the new speed */
irlap_send_rr_frame(self, CMD_FRAME);
irlap_start_final_timer(self, self->final_timeout/2);
@@ -991,8 +994,9 @@ static int irlap_state_pclose(struct irl
case RECV_UA_RSP: /* FALLTHROUGH */
case RECV_DM_RSP:
del_timer(&self->final_timer);
-
- irlap_apply_default_connection_parameters(self);
+
+ /* Set new link parameters */
+ irlap_apply_default_connection_parameters(self, TRUE);
/* Always switch state before calling upper layers */
irlap_next_state(self, LAP_NDM);
@@ -1007,7 +1011,7 @@ static int irlap_state_pclose(struct irl
self->retry_count++;
/* Keep state */
} else {
- irlap_apply_default_connection_parameters(self);
+ irlap_apply_default_connection_parameters(self, TRUE);
/* Always switch state before calling upper layers */
irlap_next_state(self, LAP_NDM);
@@ -1365,7 +1369,7 @@ static int irlap_state_nrm_p(struct irla
self->retry_count);
/* Keep state */
} else if (self->retry_count >= self->N2) {
- irlap_apply_default_connection_parameters(self);
+ irlap_apply_default_connection_parameters(self, TRUE);
/* Always switch state before calling upper layers */
irlap_next_state(self, LAP_NDM);
@@ -1477,7 +1481,7 @@ static int irlap_state_reset(struct irla
case RECV_DISC_CMD:
del_timer(&self->final_timer);
- irlap_apply_default_connection_parameters(self);
+ irlap_apply_default_connection_parameters(self, TRUE);
/* Always switch state before calling upper layers */
irlap_next_state(self, LAP_NDM);
@@ -1512,7 +1516,7 @@ static int irlap_state_reset(struct irla
irlap_start_final_timer(self, self->final_timeout);
irlap_next_state(self, LAP_RESET);
} else if (self->retry_count >= self->N3) {
- irlap_apply_default_connection_parameters(self);
+ irlap_apply_default_connection_parameters(self, TRUE);
/* Always switch state before calling upper layers */
irlap_next_state(self, LAP_NDM);
@@ -1933,7 +1937,7 @@ static int irlap_state_nrm_s(struct irla
irlap_start_wd_timer(self, self->wd_timeout);
self->retry_count++;
} else if ((self->retry_count * 2) >= self->N2) {
- irlap_apply_default_connection_parameters(self);
+ irlap_apply_default_connection_parameters(self, TRUE);
/* Always switch state before calling upper layers */
irlap_next_state(self, LAP_NDM);
@@ -1945,10 +1949,12 @@ static int irlap_state_nrm_s(struct irla
irlap_next_state(self, LAP_NDM);
irlap_wait_min_turn_around(self, &self->qos_tx);
+
+ /* Set default link parameters after sending the response */
+ irlap_apply_default_connection_parameters(self, FALSE);
irlap_send_ua_response_frame(self, NULL);
del_timer(&self->wd_timer);
irlap_flush_all_queues(self);
- irlap_apply_default_connection_parameters(self);
irlap_disconnect_indication(self, LAP_DISC_INDICATION);
break;
@@ -2002,9 +2008,11 @@ static int irlap_state_sclose(struct irl
irlap_next_state(self, LAP_NDM);
irlap_wait_min_turn_around(self, &self->qos_tx);
+
+ /* Set default link parameters after sending the response */
+ irlap_apply_default_connection_parameters(self, FALSE);
irlap_send_ua_response_frame(self, NULL);
del_timer(&self->wd_timer);
- irlap_apply_default_connection_parameters(self);
irlap_disconnect_indication(self, LAP_DISC_INDICATION);
break;
@@ -2013,12 +2021,12 @@ static int irlap_state_sclose(struct irl
irlap_next_state(self, LAP_NDM);
del_timer(&self->wd_timer);
- irlap_apply_default_connection_parameters(self);
+ irlap_apply_default_connection_parameters(self, TRUE);
irlap_disconnect_indication(self, LAP_DISC_INDICATION);
break;
case WD_TIMER_EXPIRED:
- irlap_apply_default_connection_parameters(self);
+ irlap_apply_default_connection_parameters(self, TRUE);
irlap_disconnect_indication(self, LAP_DISC_INDICATION);
break;
diff -u -p linux/drivers/net/irda/irtty.d6.c linux/drivers/net/irda/irtty.c
--- linux/drivers/net/irda/irtty.d6.c Tue Jan 2 11:05:20 2001
+++ linux/drivers/net/irda/irtty.c Thu Jan 4 10:33:35 2001
@@ -629,7 +629,7 @@ static int irtty_hard_xmit(struct sk_buf
{
struct irtty_cb *self;
int actual = 0;
- __u32 speed;
+ __s32 speed;
self = (struct irtty_cb *) dev->priv;
ASSERT(self != NULL, return 0;);
@@ -638,12 +638,14 @@ static int irtty_hard_xmit(struct sk_buf
netif_stop_queue(dev);
/* Check if we need to change the speed */
- if ((speed = irda_get_speed(skb)) != self->io.speed) {
+ speed = irda_get_next_speed(skb);
+ if ((speed != self->io.speed) && (speed != -1)) {
/* Check for empty frame */
if (!skb->len) {
irda_task_execute(self, irtty_change_speed,
irtty_change_speed_complete,
NULL, (void *) speed);
+ dev_kfree_skb(skb);
return 0;
} else
self->new_speed = speed;
diff -u -p linux/drivers/net/irda/irport.d6.c linux/drivers/net/irda/irport.c
--- linux/drivers/net/irda/irport.d6.c Tue Jan 2 11:05:46 2001
+++ linux/drivers/net/irda/irport.c Thu Jan 4 10:34:48 2001
@@ -616,7 +616,7 @@ int irport_hard_xmit(struct sk_buff *skb
struct irport_cb *self;
unsigned long flags;
int iobase;
- __u32 speed;
+ __s32 speed;
ASSERT(dev != NULL, return 0;);
@@ -628,12 +628,14 @@ int irport_hard_xmit(struct sk_buff *skb
netif_stop_queue(dev);
/* Check if we need to change the speed */
- if ((speed = irda_get_speed(skb)) != self->io.speed) {
+ speed = irda_get_next_speed(skb);
+ if ((speed != self->io.speed) && (speed != -1)) {
/* Check for empty frame */
if (!skb->len) {
irda_task_execute(self, __irport_change_speed,
irport_change_speed_complete,
NULL, (void *) speed);
+ dev_kfree_skb(skb);
return 0;
} else
self->new_speed = speed;
diff -u -p linux/drivers/net/irda/nsc-ircc.d6.c linux/drivers/net/irda/nsc-ircc.c
--- linux/drivers/net/irda/nsc-ircc.d6.c Tue Jan 2 11:06:49 2001
+++ linux/drivers/net/irda/nsc-ircc.c Thu Jan 4 10:34:16 2001
@@ -1064,7 +1064,7 @@ static int nsc_ircc_hard_xmit_sir(struct
struct nsc_ircc_cb *self;
unsigned long flags;
int iobase;
- __u32 speed;
+ __s32 speed;
__u8 bank;
self = (struct nsc_ircc_cb *) dev->priv;
@@ -1076,10 +1076,12 @@ static int nsc_ircc_hard_xmit_sir(struct
netif_stop_queue(dev);
/* Check if we need to change the speed */
- if ((speed = irda_get_speed(skb)) != self->io.speed) {
+ speed = irda_get_next_speed(skb);
+ if ((speed != self->io.speed) && (speed != -1)) {
/* Check for empty frame */
if (!skb->len) {
nsc_ircc_change_speed(self, speed);
+ dev_kfree_skb(skb);
return 0;
} else
self->new_speed = speed;
@@ -1116,7 +1118,7 @@ static int nsc_ircc_hard_xmit_fir(struct
struct nsc_ircc_cb *self;
unsigned long flags;
int iobase;
- __u32 speed;
+ __s32 speed;
__u8 bank;
int mtt, diff;
@@ -1126,10 +1128,12 @@ static int nsc_ircc_hard_xmit_fir(struct
netif_stop_queue(dev);
/* Check if we need to change the speed */
- if ((speed = irda_get_speed(skb)) != self->io.speed) {
+ speed = irda_get_next_speed(skb);
+ if ((speed != self->io.speed) && (speed != -1)) {
/* Check for empty frame */
if (!skb->len) {
nsc_ircc_change_speed(self, speed);
+ dev_kfree_skb(skb);
return 0;
} else
self->new_speed = speed;
diff -u -p linux/drivers/net/irda/smc-ircc.d6.c linux/drivers/net/irda/smc-ircc.c
--- linux/drivers/net/irda/smc-ircc.d6.c Tue Jan 2 11:08:19 2001
+++ linux/drivers/net/irda/smc-ircc.c Thu Jan 4 10:35:11 2001
@@ -605,7 +606,7 @@ static int ircc_hard_xmit(struct sk_buff
struct irport_cb *irport;
struct ircc_cb *self;
unsigned long flags;
- __u32 speed;
+ __s32 speed;
int iobase;
int mtt;
@@ -618,10 +619,12 @@ static int ircc_hard_xmit(struct sk_buff
netif_stop_queue(dev);
/* Check if we need to change the speed after this frame */
- if ((speed = irda_get_speed(skb)) != self->io.speed) {
+ speed = irda_get_next_speed(skb);
+ if ((speed != self->io.speed) && (speed != -1)) {
/* Check for empty frame */
if (!skb->len) {
ircc_change_speed(self, speed);
+ dev_kfree_skb(skb);
return 0;
} else
self->new_speed = speed;
diff -u -p linux/drivers/net/irda/toshoboe.d6.c linux/drivers/net/irda/toshoboe.c
--- linux/drivers/net/irda/toshoboe.d6.c Tue Jan 2 11:09:22 2001
+++ linux/drivers/net/irda/toshoboe.c Thu Jan 4 10:35:45 2001
@@ -263,7 +263,7 @@ static int
toshoboe_hard_xmit (struct sk_buff *skb, struct net_device *dev)
{
struct toshoboe_cb *self;
- __u32 speed;
+ __s32 speed;
int mtt, len;
self = (struct toshoboe_cb *) dev->priv;
@@ -272,10 +272,12 @@ toshoboe_hard_xmit (struct sk_buff *skb,
);
/* Check if we need to change the speed */
- if ((speed = irda_get_speed(skb)) != self->io.speed) {
+ speed = irda_get_next_speed(skb);
+ if ((speed != self->io.speed) && (speed != -1)) {
/* Check for empty frame */
if (!skb->len) {
toshoboe_setbaud(self, speed);
+ dev_kfree_skb(skb);
return 0;
} else
self->new_speed = speed;
diff -u -p linux/drivers/net/irda/w83977af_ir.d6.c linux/drivers/net/irda/w83977af_ir.c
--- linux/drivers/net/irda/w83977af_ir.d6.c Tue Jan 2 11:10:11 2001
+++ linux/drivers/net/irda/w83977af_ir.c Thu Jan 4 14:49:36 2001
@@ -497,7 +497,7 @@ void w83977af_change_speed(struct w83977
int w83977af_hard_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct w83977af_ir *self;
- __u32 speed;
+ __s32 speed;
int iobase;
__u8 set;
int mtt;
@@ -513,10 +513,12 @@ int w83977af_hard_xmit(struct sk_buff *s
netif_stop_queue(dev);
/* Check if we need to change the speed */
- if ((speed = irda_get_speed(skb)) != self->io.speed) {
+ speed = irda_get_next_speed(skb);
+ if ((speed != self->io.speed) && (speed != -1)) {
/* Check for empty frame */
if (!skb->len) {
w83977af_change_speed(self, speed);
+ dev_kfree_skb(skb);
return 0;
} else
self->new_speed = speed;