[PATCH][RFC] usbatm.[ch]: cleanup and OAM F5 loopback

Roman Kagan rkagan at mail.ru
Sat Mar 19 06:02:54 EST 2005


On Sun, Mar 13, 2005 at 11:52:26AM +0300, Roman Kagan wrote:
> On Sat, Mar 12, 2005 at 11:08:10PM +0100, Duncan Sands wrote:
> > > [...] So, if the modem is busy
> > > digesting previously sent urbs, you can sit and wait for more data in
> > > the buffer, but if all urbs have completed you have to immediately push
> > > the data to the modem.  That's how it used to work and that's how I did
> > > it too, but using an atomic counter rather than an additional list.
> > > Still I'm curious if it's worth the hassle, and simply flushing the
> > > available data immediately is any slower.
> > 
> > it is probably pointless.
> 
> OK, I'll redo the patch without it and see what happens.

Here's the updated patch.  I did it 5 days ago (and thus I've been
testing it since then :) but did't have the time to send to the list.

Roman.

P.S. Sorry Matthieu it conflicts somewhat with what you do.  I believe
we'll be able to reconcile it.  I'd also appreciate if you can test this
patch with bulk endpoints at high rates, my link speed (256k/128k) is
too low for that.  Also bigger buffer sizes and urb numbers would be
worth trying.

 usbatm.c |  634 +++++++++++++++++++++++++++++----------------------------------
 usbatm.h |   70 ------
 2 files changed, 311 insertions(+), 393 deletions(-)


Index: usbatm.h
===================================================================
RCS file: /home/cvs/usbatm/usbatm.h,v
retrieving revision 1.10
diff -u -p -r1.10 usbatm.h
--- usbatm.h	7 Feb 2005 23:50:41 -0000	1.10
+++ usbatm.h	19 Mar 2005 10:54:24 -0000
@@ -95,52 +95,12 @@ extern int usbatm_usb_probe(struct usb_i
 extern void usbatm_usb_disconnect(struct usb_interface *intf);
 
 
-/* usbatm */
-
-#define UDSL_MAX_RCV_URBS		4
-#define UDSL_MAX_SND_URBS		4
-#define UDSL_MAX_RCV_BUFS		8
-#define UDSL_MAX_SND_BUFS		8
-#define UDSL_MAX_RCV_BUF_SIZE		1024	/* ATM cells */
-#define UDSL_MAX_SND_BUF_SIZE		1024	/* ATM cells */
-#define UDSL_DEFAULT_RCV_URBS		2
-#define UDSL_DEFAULT_SND_URBS		2
-#define UDSL_DEFAULT_RCV_BUFS		4
-#define UDSL_DEFAULT_SND_BUFS		4
-#define UDSL_DEFAULT_RCV_BUF_SIZE	64	/* ATM cells */
-#define UDSL_DEFAULT_SND_BUF_SIZE	64	/* ATM cells */
-
-
-/* receive */
-
-struct usbatm_receive_buffer {
-	struct list_head list;
-	unsigned char *base;
+struct usbatm_transceiver {
+	u8 *buffer;
 	unsigned int filled_cells;
-};
-
-struct usbatm_receiver {
-	struct list_head list;
-	struct usbatm_receive_buffer *buffer;
 	struct urb *urb;
 	struct usbatm_data *instance;
-};
-
-
-/* send */
-
-struct usbatm_send_buffer {
 	struct list_head list;
-	unsigned char *base;
-	unsigned char *free_start;
-	unsigned int free_cells;
-};
-
-struct usbatm_sender {
-	struct list_head list;
-	struct usbatm_send_buffer *buffer;
-	struct urb *urb;
-	struct usbatm_data *instance;
 };
 
 
@@ -183,31 +143,21 @@ struct usbatm_data {
 	/* ATM device */
 	struct list_head vcc_list;
 
-	/* receive */
-	struct usbatm_receiver receivers[UDSL_MAX_RCV_URBS];
-	struct usbatm_receive_buffer receive_buffers[UDSL_MAX_RCV_BUFS];
+	struct usbatm_transceiver *transceivers;
 
-	spinlock_t receive_lock;
-	struct list_head spare_receivers;
-	struct list_head filled_receive_buffers;
-
-	struct tasklet_struct receive_tasklet;
-	struct list_head spare_receive_buffers;
+	/* receive */
+	spinlock_t rx_lock;
+	struct list_head filled_receivers;
+	struct tasklet_struct rx_tasklet;
 
 	/* send */
-	struct usbatm_sender senders[UDSL_MAX_SND_URBS];
-	struct usbatm_send_buffer send_buffers[UDSL_MAX_SND_BUFS];
+	spinlock_t tx_lock;
+	struct list_head spare_senders;
+	struct tasklet_struct tx_tasklet;
 
 	struct sk_buff_head sndqueue;
 
-	spinlock_t send_lock;
-	struct list_head spare_senders;
-	struct list_head spare_send_buffers;
-
-	struct tasklet_struct send_tasklet;
 	struct sk_buff *current_skb;			/* being emptied */
-	struct usbatm_send_buffer *current_buffer;	/* being filled */
-	struct list_head filled_send_buffers;
 };
 
 #endif	/* _USBATM_H_ */
Index: usbatm.c
===================================================================
RCS file: /home/cvs/usbatm/usbatm.c,v
retrieving revision 1.15
diff -u -p -r1.15 usbatm.c
--- usbatm.c	4 Mar 2005 11:30:58 -0000	1.15
+++ usbatm.c	19 Mar 2005 10:54:25 -0000
@@ -95,16 +95,12 @@ static int usbatm_print_packet(const uns
 
 static const char usbatm_driver_name[] = "usbatm";
 
-#define UDSL_MAX_RCV_URBS		4
-#define UDSL_MAX_SND_URBS		4
-#define UDSL_MAX_RCV_BUFS		8
-#define UDSL_MAX_SND_BUFS		8
+#define UDSL_MAX_RCV_URBS		16
+#define UDSL_MAX_SND_URBS		16
 #define UDSL_MAX_RCV_BUF_SIZE		1024	/* ATM cells */
 #define UDSL_MAX_SND_BUF_SIZE		1024	/* ATM cells */
-#define UDSL_DEFAULT_RCV_URBS		2
-#define UDSL_DEFAULT_SND_URBS		2
-#define UDSL_DEFAULT_RCV_BUFS		4
-#define UDSL_DEFAULT_SND_BUFS		4
+#define UDSL_DEFAULT_RCV_URBS		4
+#define UDSL_DEFAULT_SND_URBS		4
 #define UDSL_DEFAULT_RCV_BUF_SIZE	64	/* ATM cells */
 #define UDSL_DEFAULT_SND_BUF_SIZE	64	/* ATM cells */
 
@@ -141,8 +137,6 @@ struct usbatm_control {
 
 static unsigned int num_rcv_urbs = UDSL_DEFAULT_RCV_URBS;
 static unsigned int num_snd_urbs = UDSL_DEFAULT_SND_URBS;
-static unsigned int num_rcv_bufs = UDSL_DEFAULT_RCV_BUFS;
-static unsigned int num_snd_bufs = UDSL_DEFAULT_SND_BUFS;
 static unsigned int rcv_buf_size = UDSL_DEFAULT_RCV_BUF_SIZE;
 static unsigned int snd_buf_size = UDSL_DEFAULT_SND_BUF_SIZE;
 
@@ -158,27 +152,15 @@ MODULE_PARM_DESC(num_snd_urbs,
 		 __MODULE_STRING(UDSL_MAX_SND_URBS) ", default: "
 		 __MODULE_STRING(UDSL_DEFAULT_SND_URBS) ")");
 
-module_param(num_rcv_bufs, uint, 0444);
-MODULE_PARM_DESC(num_rcv_bufs,
-		 "Number of buffers used for reception (range: 0-"
-		 __MODULE_STRING(UDSL_MAX_RCV_BUFS) ", default: "
-		 __MODULE_STRING(UDSL_DEFAULT_RCV_BUFS) ")");
-
-module_param(num_snd_bufs, uint, 0444);
-MODULE_PARM_DESC(num_snd_bufs,
-		 "Number of buffers used for transmission (range: 0-"
-		 __MODULE_STRING(UDSL_MAX_SND_BUFS) ", default: "
-		 __MODULE_STRING(UDSL_DEFAULT_SND_BUFS) ")");
-
 module_param(rcv_buf_size, uint, 0444);
 MODULE_PARM_DESC(rcv_buf_size,
-		 "Size of the buffers used for reception (range: 0-"
+		 "Size of the buffers used for reception in ATM cells (range: 0-"
 		 __MODULE_STRING(UDSL_MAX_RCV_BUF_SIZE) ", default: "
 		 __MODULE_STRING(UDSL_DEFAULT_RCV_BUF_SIZE) ")");
 
 module_param(snd_buf_size, uint, 0444);
 MODULE_PARM_DESC(snd_buf_size,
-		 "Size of the buffers used for transmission (range: 0-"
+		 "Size of the buffers used for transmission in ATM cells (range: 0-"
 		 __MODULE_STRING(UDSL_MAX_SND_BUF_SIZE) ", default: "
 		 __MODULE_STRING(UDSL_DEFAULT_SND_BUF_SIZE) ")");
 
@@ -214,6 +196,155 @@ static inline void usbatm_pop(struct atm
 		dev_kfree_skb(skb);
 }
 
+static u16 crc10_table[256] = {
+	0x000, 0x233, 0x255, 0x066, 0x299, 0x0aa, 0x0cc, 0x2ff, 0x301, 0x132, 0x154, 0x367, 0x198, 0x3ab, 0x3cd, 0x1fe,
+	0x031, 0x202, 0x264, 0x057, 0x2a8, 0x09b, 0x0fd, 0x2ce, 0x330, 0x103, 0x165, 0x356, 0x1a9, 0x39a, 0x3fc, 0x1cf,
+	0x062, 0x251, 0x237, 0x004, 0x2fb, 0x0c8, 0x0ae, 0x29d, 0x363, 0x150, 0x136, 0x305, 0x1fa, 0x3c9, 0x3af, 0x19c,
+	0x053, 0x260, 0x206, 0x035, 0x2ca, 0x0f9, 0x09f, 0x2ac, 0x352, 0x161, 0x107, 0x334, 0x1cb, 0x3f8, 0x39e, 0x1ad,
+	0x0c4, 0x2f7, 0x291, 0x0a2, 0x25d, 0x06e, 0x008, 0x23b, 0x3c5, 0x1f6, 0x190, 0x3a3, 0x15c, 0x36f, 0x309, 0x13a,
+	0x0f5, 0x2c6, 0x2a0, 0x093, 0x26c, 0x05f, 0x039, 0x20a, 0x3f4, 0x1c7, 0x1a1, 0x392, 0x16d, 0x35e, 0x338, 0x10b,
+	0x0a6, 0x295, 0x2f3, 0x0c0, 0x23f, 0x00c, 0x06a, 0x259, 0x3a7, 0x194, 0x1f2, 0x3c1, 0x13e, 0x30d, 0x36b, 0x158,
+	0x097, 0x2a4, 0x2c2, 0x0f1, 0x20e, 0x03d, 0x05b, 0x268, 0x396, 0x1a5, 0x1c3, 0x3f0, 0x10f, 0x33c, 0x35a, 0x169,
+	0x188, 0x3bb, 0x3dd, 0x1ee, 0x311, 0x122, 0x144, 0x377, 0x289, 0x0ba, 0x0dc, 0x2ef, 0x010, 0x223, 0x245, 0x076,
+	0x1b9, 0x38a, 0x3ec, 0x1df, 0x320, 0x113, 0x175, 0x346, 0x2b8, 0x08b, 0x0ed, 0x2de, 0x021, 0x212, 0x274, 0x047,
+	0x1ea, 0x3d9, 0x3bf, 0x18c, 0x373, 0x140, 0x126, 0x315, 0x2eb, 0x0d8, 0x0be, 0x28d, 0x072, 0x241, 0x227, 0x014,
+	0x1db, 0x3e8, 0x38e, 0x1bd, 0x342, 0x171, 0x117, 0x324, 0x2da, 0x0e9, 0x08f, 0x2bc, 0x043, 0x270, 0x216, 0x025,
+	0x14c, 0x37f, 0x319, 0x12a, 0x3d5, 0x1e6, 0x180, 0x3b3, 0x24d, 0x07e, 0x018, 0x22b, 0x0d4, 0x2e7, 0x281, 0x0b2,
+	0x17d, 0x34e, 0x328, 0x11b, 0x3e4, 0x1d7, 0x1b1, 0x382, 0x27c, 0x04f, 0x029, 0x21a, 0x0e5, 0x2d6, 0x2b0, 0x083,
+	0x12e, 0x31d, 0x37b, 0x148, 0x3b7, 0x184, 0x1e2, 0x3d1, 0x22f, 0x01c, 0x07a, 0x249, 0x0b6, 0x285, 0x2e3, 0x0d0,
+	0x11f, 0x32c, 0x34a, 0x179, 0x386, 0x1b5, 0x1d3, 0x3e0, 0x21e, 0x02d, 0x04b, 0x278, 0x087, 0x2b4, 0x2d2, 0x0e1,
+};
+
+static u16 inline crc10(u16 init, u8 *data, int len)
+{
+	while (len--)
+		init = ((init << 8) & 0x3ff) ^ crc10_table[(init >> 2) & 0xff] ^ *data++;
+	return init;
+}
+
+/* buffer management */
+static inline struct usbatm_transceiver *usbatm_pop_transceiver(struct list_head *trx_list,
+								spinlock_t *lock)
+{
+	struct usbatm_transceiver *trx;
+
+	spin_lock_irq(lock);
+	if (list_empty(trx_list)) {
+		spin_unlock_irq(lock);
+		return NULL;
+	}
+	
+	trx = list_entry(trx_list->next, struct usbatm_transceiver, list);
+	list_del(&trx->list);
+	spin_unlock_irq(lock);
+	return trx;
+}
+
+static inline int usbatm_submit(struct usbatm_transceiver *trx, int pipe, int size,
+				usb_complete_t complete)
+{
+	int ret;
+	struct usbatm_data *instance = trx->instance;
+
+	usb_fill_bulk_urb(trx->urb, instance->usb_dev,
+			  pipe, trx->buffer, size, complete, trx);
+
+	vdbg("usbatm_submit: submitting urb 0x%p, trx 0x%p", trx->urb, trx);
+
+	ret = usb_submit_urb(trx->urb, GFP_ATOMIC);
+	if (ret)
+		dbg("usbatm_submit: urb 0x%p submission failed (%d)!", trx->urb, ret);
+	return ret;
+}
+
+/* URB completion handlers */
+
+static void usbatm_rx_complete(struct urb *urb, struct pt_regs *regs)
+{
+	struct usbatm_transceiver *rx = urb->context;
+	struct usbatm_data *instance = rx->instance;
+	unsigned long flags;
+
+	rx->filled_cells = urb->actual_length / (ATM_CELL_SIZE + instance->rx_padding);
+
+	vdbg("usbatm_rx_complete: urb 0x%p, status %d, actual_length %d, filled_cells %u, rx 0x%p",
+	     urb, urb->status, urb->actual_length, rx->filled_cells, rx);
+
+	UDSL_ASSERT(rx->filled_cells <= rcv_buf_size);
+
+	/* may be in_interrupt() */
+	spin_lock_irqsave(&instance->rx_lock, flags);
+	list_add_tail(&rx->list, &instance->filled_receivers);
+	spin_unlock_irqrestore(&instance->rx_lock, flags);
+
+	tasklet_schedule(&instance->rx_tasklet);
+}
+
+static void usbatm_tx_complete(struct urb *urb, struct pt_regs *regs)
+{
+	struct usbatm_transceiver *tx = urb->context;
+	struct usbatm_data *instance = tx->instance;
+	unsigned long flags;
+
+	vdbg("usbatm_tx_complete: urb 0x%p, status %d, tx 0x%p",
+	     urb, urb->status, tx);
+
+	tx->filled_cells = 0;
+
+	/* may be in_interrupt() */
+	spin_lock_irqsave(&instance->tx_lock, flags);
+	list_add_tail(&tx->list, &instance->spare_senders);
+	spin_unlock_irqrestore(&instance->tx_lock, flags);
+
+	tasklet_schedule(&instance->tx_tasklet);
+}
+
+/* OAM loopback reply (fire-and-forget) */
+static int usbatm_oam_reply(struct usbatm_data *instance, u8 *source)
+{
+	struct usbatm_transceiver *tx;
+	u16 crc;
+	int ret;
+
+	if (source[ATM_CELL_HEADER] != 0x18 || source[ATM_CELL_HEADER + 1] != 0x01) {
+		dbg("usbatm_oam_reply: unexpected OAM type/function %x direction %x !", source[5], source[6]);
+		return -EPROTO;
+	}
+
+	if (crc10(0, source + ATM_CELL_HEADER, ATM_CELL_PAYLOAD)) {
+		dbg("usbatm_oam_reply: OAM CRC10 error!");
+		return -EIO;
+	}
+
+	tx = usbatm_pop_transceiver(&instance->spare_senders, &instance->tx_lock);
+	if (!tx)
+		return -ENOMEM;
+	
+	memcpy(tx->buffer, source, ATM_CELL_SIZE);
+
+	source[ATM_CELL_HEADER + 1] = 0;	/* update the direction field */
+	
+	memset(tx->buffer + ATM_CELL_SIZE - 2, 0, instance->tx_padding + 2);
+	
+	crc = crc10(0, tx->buffer + ATM_CELL_HEADER, ATM_CELL_PAYLOAD);
+	tx->buffer[ATM_CELL_SIZE - 2] = (crc & 0x3) << 8;
+	tx->buffer[ATM_CELL_SIZE - 1] = crc & 0xff;
+
+	tx->filled_cells = 1;
+	ret = usbatm_submit(tx, instance->tx_endpoint, ATM_CELL_SIZE + instance->tx_padding,
+			    usbatm_tx_complete);
+	if (ret) {
+		/* consider all errors transient and return the buffer back to the queue */
+		/* FIXME: may result in deadlock */
+		tx->filled_cells = 0;
+		spin_lock_irq(&instance->tx_lock);
+		list_add(&tx->list, &instance->spare_senders);
+		spin_unlock_irq(&instance->tx_lock);
+	}
+
+	return ret;
+}
+
 /*************
 **  decode  **
 *************/
@@ -230,24 +361,22 @@ static inline struct usbatm_vcc_data *us
 }
 
 static void usbatm_extract_cells(struct usbatm_data *instance,
-			       unsigned char *source, unsigned int howmany)
+				 unsigned char *source, unsigned int howmany)
 {
 	struct usbatm_vcc_data *cached_vcc = NULL;
 	struct atm_vcc *vcc;
 	struct sk_buff *sarb;
 	struct usbatm_vcc_data *vcc_data;
-	int cached_vci = 0;
 	unsigned int i;
-	int pti;
-	int vci;
-	short cached_vpi = 0;
-	short vpi;
+	u8 pti;
+	u32 vci, cached_vci = 0;
+	u16 vpi, cached_vpi = 0;
 
 	for (i = 0; i < howmany;
 	     i++, source += ATM_CELL_SIZE + instance->rx_padding) {
 		vpi = ((source[0] & 0x0f) << 4) | (source[1] >> 4);
 		vci = ((source[1] & 0x0f) << 12) | (source[2] << 4) | (source[3] >> 4);
-		pti = (source[3] & 0x2) != 0;
+		pti = ((source[3] & 0xe) >> 1);
 
 		vdbg("usbatm_extract_cells: vpi %hd, vci %d, pti %d", vpi, vci, pti);
 
@@ -263,6 +392,16 @@ static void usbatm_extract_cells(struct 
 		}
 
 		vcc = vcc_data->vcc;
+
+		/* OAM F5 end-to-end */
+		if (pti == ATM_PTI_E2EF5) {
+			if (usbatm_oam_reply(instance, source)) {
+				dbg("usbatm_extract_cells: OAM reply failed (vcc: 0x%p)!", vcc);
+				atomic_inc(&vcc->stats->rx_err);
+			}
+			continue;
+		}
+
 		sarb = vcc_data->sarb;
 
 		if (sarb->tail + ATM_CELL_PAYLOAD > sarb->end) {
@@ -274,7 +413,7 @@ static void usbatm_extract_cells(struct 
 		memcpy(sarb->tail, source + ATM_CELL_HEADER, ATM_CELL_PAYLOAD);
 		__skb_put(sarb, ATM_CELL_PAYLOAD);
 
-		if (pti) {
+		if (pti & 1) {
 			struct sk_buff *skb;
 			unsigned int length;
 			unsigned int pdu_length;
@@ -384,11 +523,10 @@ static void usbatm_groom_skb(struct atm_
 }
 
 static unsigned int usbatm_write_cells(struct usbatm_data *instance,
-				     unsigned int howmany, struct sk_buff *skb,
-				     unsigned char **target_p)
+				       unsigned int howmany, struct sk_buff *skb,
+				       unsigned char *target)
 {
 	struct usbatm_control *ctrl = UDSL_SKB(skb);
-	unsigned char *target = *target_p;
 	unsigned int nc, ne, i;
 
 	vdbg("usbatm_write_cells: howmany=%u, skb->len=%d, num_cells=%u, num_entire=%u, pdu_padding=%u", howmany, skb->len, ctrl->num_cells, ctrl->num_entire, ctrl->pdu_padding);
@@ -449,7 +587,6 @@ static unsigned int usbatm_write_cells(s
 		target += instance->tx_padding;
 	}
  out:
-	*target_p = target;
 	return nc - ctrl->num_cells;
 }
 
@@ -457,223 +594,94 @@ static unsigned int usbatm_write_cells(s
 **  receive  **
 **************/
 
-static void usbatm_complete_receive(struct urb *urb, struct pt_regs *regs)
-{
-	struct usbatm_receive_buffer *buf;
-	struct usbatm_data *instance;
-	struct usbatm_receiver *rcv;
-	unsigned long flags;
-
-	if (!urb || !(rcv = urb->context)) {
-		dbg("usbatm_complete_receive: bad urb!");
-		return;
-	}
-
-	instance = rcv->instance;
-	buf = rcv->buffer;
-
-	buf->filled_cells = urb->actual_length / (ATM_CELL_SIZE + instance->rx_padding);
-
-	vdbg("usbatm_complete_receive: urb 0x%p, status %d, actual_length %d, filled_cells %u, rcv 0x%p, buf 0x%p", urb, urb->status, urb->actual_length, buf->filled_cells, rcv, buf);
-
-	UDSL_ASSERT(buf->filled_cells <= rcv_buf_size);
-
-	/* may not be in_interrupt() */
-	spin_lock_irqsave(&instance->receive_lock, flags);
-	list_add(&rcv->list, &instance->spare_receivers);
-	list_add_tail(&buf->list, &instance->filled_receive_buffers);
-	if (likely(!urb->status))
-		tasklet_schedule(&instance->receive_tasklet);
-	spin_unlock_irqrestore(&instance->receive_lock, flags);
-}
-
-static void usbatm_process_receive(unsigned long data)
+static void usbatm_rx_process(unsigned long data)
 {
-	struct usbatm_receive_buffer *buf;
 	struct usbatm_data *instance = (struct usbatm_data *)data;
-	struct usbatm_receiver *rcv;
-	int err;
+	struct usbatm_transceiver *rx;
 
- made_progress:
-	while (!list_empty(&instance->spare_receive_buffers)) {
-		spin_lock_irq(&instance->receive_lock);
-		if (list_empty(&instance->spare_receivers)) {
-			spin_unlock_irq(&instance->receive_lock);
-			break;
-		}
-		rcv = list_entry(instance->spare_receivers.next,
-				 struct usbatm_receiver, list);
-		list_del(&rcv->list);
-		spin_unlock_irq(&instance->receive_lock);
-
-		buf = list_entry(instance->spare_receive_buffers.next,
-				 struct usbatm_receive_buffer, list);
-		list_del(&buf->list);
-
-		rcv->buffer = buf;
-
-		usb_fill_bulk_urb(rcv->urb, instance->usb_dev,
-				  instance->rx_endpoint,
-				  buf->base,
-				  rcv_buf_size * (ATM_CELL_SIZE + instance->rx_padding),
-				  usbatm_complete_receive, rcv);
+	while ((rx = usbatm_pop_transceiver(&instance->filled_receivers, &instance->rx_lock))) {
+		vdbg("usbatm_rx_process: processing rx 0x%p", rx);
+		usbatm_extract_cells(instance, rx->buffer, rx->filled_cells);
 
-		vdbg("usbatm_process_receive: sending urb 0x%p, rcv 0x%p, buf 0x%p",
-		     rcv->urb, rcv, buf);
+		rx->filled_cells = 0;
 
-		if ((err = usb_submit_urb(rcv->urb, GFP_ATOMIC)) < 0) {
-			dbg("usbatm_process_receive: urb submission failed (%d)!", err);
-			list_add(&buf->list, &instance->spare_receive_buffers);
-			spin_lock_irq(&instance->receive_lock);
-			list_add(&rcv->list, &instance->spare_receivers);
-			spin_unlock_irq(&instance->receive_lock);
-			break;
+		if (usbatm_submit(rx, instance->rx_endpoint,
+				  rcv_buf_size * (ATM_CELL_SIZE + instance->rx_padding),
+				  usbatm_rx_complete)) {
+			/* consider all errors transient and return the buffer back to the queue */
+			/* FIXME: if there are no more pending URBs nobody will
+			 * reschedule rx_tasklet => deadlock */
+			spin_lock_irq(&instance->rx_lock);
+			list_add(&rx->list, &instance->filled_receivers);
+			spin_unlock_irq(&instance->rx_lock);
+			return;
 		}
 	}
-
-	spin_lock_irq(&instance->receive_lock);
-	if (list_empty(&instance->filled_receive_buffers)) {
-		spin_unlock_irq(&instance->receive_lock);
-		return;		/* done - no more buffers */
-	}
-	buf = list_entry(instance->filled_receive_buffers.next,
-			 struct usbatm_receive_buffer, list);
-	list_del(&buf->list);
-	spin_unlock_irq(&instance->receive_lock);
-
-	vdbg("usbatm_process_receive: processing buf 0x%p", buf);
-	usbatm_extract_cells(instance, buf->base, buf->filled_cells);
-	list_add(&buf->list, &instance->spare_receive_buffers);
-	goto made_progress;
 }
 
 /***********
 **  send  **
 ***********/
 
-static void usbatm_complete_send(struct urb *urb, struct pt_regs *regs)
+static void usbatm_tx_process(unsigned long data)
 {
-	struct usbatm_data *instance;
-	struct usbatm_sender *snd;
-	unsigned long flags;
+	struct usbatm_data *instance = (struct usbatm_data *)data;
+	struct sk_buff *skb = instance->current_skb;
+	struct usbatm_transceiver *tx = NULL;
+	int num_written;
+
+	if (!skb)
+		skb = skb_dequeue(&instance->sndqueue);
+
+	while (skb) {
+		if (!tx)
+			tx = usbatm_pop_transceiver(&instance->spare_senders,
+						    &instance->tx_lock);
+		if (!tx)
+			break;		/* no more senders */
+
+		num_written = usbatm_write_cells(instance, snd_buf_size - tx->filled_cells, skb,
+						 tx->buffer + tx->filled_cells *
+						 (ATM_CELL_SIZE + instance->tx_padding));
+
+		vdbg("usbatm_tx_process: wrote %u cells from skb 0x%p to sender 0x%p",
+		     num_written, skb, tx);
 
-	if (!urb || !(snd = urb->context) || !(instance = snd->instance)) {
-		dbg("usbatm_complete_send: bad urb!");
-		return;
-	}
+		tx->filled_cells += num_written;
 
-	vdbg("usbatm_complete_send: urb 0x%p, status %d, snd 0x%p, buf 0x%p", urb,
-	     urb->status, snd, snd->buffer);
+		vdbg("usbatm_tx_process: buffer contains %d cells, %d left",
+		     tx->filled_cells, snd_buf_size - tx->filled_cells);
 
-	/* may not be in_interrupt() */
-	spin_lock_irqsave(&instance->send_lock, flags);
-	list_add(&snd->list, &instance->spare_senders);
-	list_add(&snd->buffer->list, &instance->spare_send_buffers);
-	tasklet_schedule(&instance->send_tasklet);
-	spin_unlock_irqrestore(&instance->send_lock, flags);
-}
+		if (!UDSL_SKB(skb)->num_cells) {
+			struct atm_vcc *vcc = UDSL_SKB(skb)->atm_data.vcc;
 
-static void usbatm_process_send(unsigned long data)
-{
-	struct usbatm_send_buffer *buf;
-	struct usbatm_data *instance = (struct usbatm_data *)data;
-	struct sk_buff *skb;
-	struct usbatm_sender *snd;
-	int err;
-	unsigned int num_written;
+			usbatm_pop(vcc, skb);
+			atomic_inc(&vcc->stats->tx);
 
- made_progress:
-	spin_lock_irq(&instance->send_lock);
-	while (!list_empty(&instance->spare_senders)) {
-		if (!list_empty(&instance->filled_send_buffers)) {
-			buf = list_entry(instance->filled_send_buffers.next,
-					 struct usbatm_send_buffer, list);
-			list_del(&buf->list);
-		} else if ((buf = instance->current_buffer)) {
-			instance->current_buffer = NULL;
-		} else		/* all buffers empty */
-			break;
-
-		snd = list_entry(instance->spare_senders.next,
-				 struct usbatm_sender, list);
-		list_del(&snd->list);
-		spin_unlock_irq(&instance->send_lock);
-
-		snd->buffer = buf;
-		usb_fill_bulk_urb(snd->urb, instance->usb_dev,
-				  instance->tx_endpoint,
-				  buf->base,
-				  (snd_buf_size - buf->free_cells) * (ATM_CELL_SIZE + instance->tx_padding),
-				  usbatm_complete_send, snd);
-
-		vdbg("usbatm_process_send: submitting urb 0x%p (%d cells), snd 0x%p, buf 0x%p",
-		     snd->urb, snd_buf_size - buf->free_cells, snd, buf);
-
-		if ((err = usb_submit_urb(snd->urb, GFP_ATOMIC)) < 0) {
-			dbg("usbatm_process_send: urb submission failed (%d)!", err);
-			spin_lock_irq(&instance->send_lock);
-			list_add(&snd->list, &instance->spare_senders);
-			spin_unlock_irq(&instance->send_lock);
-			list_add(&buf->list, &instance->filled_send_buffers);
-			return;	/* bail out */
+			skb = skb_dequeue(&instance->sndqueue);
 		}
 
-		spin_lock_irq(&instance->send_lock);
-	}			/* while */
-	spin_unlock_irq(&instance->send_lock);
-
-	if (!instance->current_skb)
-		instance->current_skb = skb_dequeue(&instance->sndqueue);
-	if (!instance->current_skb)
-		return;		/* done - no more skbs */
-
-	skb = instance->current_skb;
-
-	if (!(buf = instance->current_buffer)) {
-		spin_lock_irq(&instance->send_lock);
-		if (list_empty(&instance->spare_send_buffers)) {
-			instance->current_buffer = NULL;
-			spin_unlock_irq(&instance->send_lock);
-			return;	/* done - no more buffers */
+		if (tx->filled_cells == snd_buf_size || !skb) {
+			if (usbatm_submit(tx, instance->tx_endpoint,
+					  tx->filled_cells * (ATM_CELL_SIZE + instance->tx_padding),
+					  usbatm_tx_complete)) {
+				/* consider all errors transient and return the buffer back to the queue */
+				/* FIXME: may result in deadlock */
+				spin_lock_irq(&instance->tx_lock);
+				list_add(&tx->list, &instance->spare_senders);
+				spin_unlock_irq(&instance->tx_lock);
+				break;
+			}
+			tx = NULL;
 		}
-		buf = list_entry(instance->spare_send_buffers.next,
-			       struct usbatm_send_buffer, list);
-		list_del(&buf->list);
-		spin_unlock_irq(&instance->send_lock);
-
-		buf->free_start = buf->base;
-		buf->free_cells = snd_buf_size;
-
-		instance->current_buffer = buf;
-	}
-
-	num_written = usbatm_write_cells(instance, buf->free_cells, skb, &buf->free_start);
-
-	vdbg("usbatm_process_send: wrote %u cells from skb 0x%p to buffer 0x%p",
-	     num_written, skb, buf);
-
-	if (!(buf->free_cells -= num_written)) {
-		list_add_tail(&buf->list, &instance->filled_send_buffers);
-		instance->current_buffer = NULL;
-	}
-
-	vdbg("usbatm_process_send: buffer contains %d cells, %d left",
-	     snd_buf_size - buf->free_cells, buf->free_cells);
-
-	if (!UDSL_SKB(skb)->num_cells) {
-		struct atm_vcc *vcc = UDSL_SKB(skb)->atm_data.vcc;
-
-		usbatm_pop(vcc, skb);
-		instance->current_skb = NULL;
 
-		atomic_inc(&vcc->stats->tx);
 	}
 
-	goto made_progress;
+	instance->current_skb = skb;
 }
 
 static void usbatm_cancel_send(struct usbatm_data *instance,
-			     struct atm_vcc *vcc)
+			       struct atm_vcc *vcc)
 {
 	struct sk_buff *skb, *n;
 
@@ -689,13 +697,13 @@ static void usbatm_cancel_send(struct us
 		}
 	spin_unlock_irq(&instance->sndqueue.lock);
 
-	tasklet_disable(&instance->send_tasklet);
+	tasklet_disable(&instance->tx_tasklet);
 	if ((skb = instance->current_skb) && (UDSL_SKB(skb)->atm_data.vcc == vcc)) {
 		dbg("usbatm_cancel_send: popping current skb (0x%p)", skb);
 		instance->current_skb = NULL;
 		usbatm_pop(vcc, skb);
 	}
-	tasklet_enable(&instance->send_tasklet);
+	tasklet_enable(&instance->tx_tasklet);
 	dbg("usbatm_cancel_send done");
 }
 
@@ -729,7 +737,7 @@ static int usbatm_atm_send(struct atm_vc
 
 	usbatm_groom_skb(vcc, skb);
 	skb_queue_tail(&instance->sndqueue, skb);
-	tasklet_schedule(&instance->send_tasklet);
+	tasklet_schedule(&instance->tx_tasklet);
 
 	return 0;
 
@@ -749,8 +757,8 @@ static void usbatm_destroy_instance(stru
 
 	dbg("usbatm_destroy_instance");
 
-	tasklet_kill(&instance->receive_tasklet);
-	tasklet_kill(&instance->send_tasklet);
+	tasklet_kill(&instance->rx_tasklet);
+	tasklet_kill(&instance->tx_tasklet);
 	usb_put_dev(instance->usb_dev);
 	kfree(instance);
 }
@@ -880,9 +888,9 @@ static int usbatm_atm_open(struct atm_vc
 
 	vcc->dev_data = new;
 
-	tasklet_disable(&instance->receive_tasklet);
+	tasklet_disable(&instance->rx_tasklet);
 	list_add(&new->list, &instance->vcc_list);
-	tasklet_enable(&instance->receive_tasklet);
+	tasklet_enable(&instance->rx_tasklet);
 
 	set_bit(ATM_VF_ADDR, &vcc->flags);
 	set_bit(ATM_VF_PARTIAL, &vcc->flags);
@@ -890,7 +898,7 @@ static int usbatm_atm_open(struct atm_vc
 
 	up(&instance->serialize);
 
-	tasklet_schedule(&instance->receive_tasklet);
+	tasklet_schedule(&instance->rx_tasklet);
 
 	dbg("usbatm_atm_open: allocated vcc data 0x%p (max_pdu: %u)", new, max_pdu);
 
@@ -916,9 +924,9 @@ static void usbatm_atm_close(struct atm_
 
 	down(&instance->serialize);	/* vs self, usbatm_atm_open */
 
-	tasklet_disable(&instance->receive_tasklet);
+	tasklet_disable(&instance->rx_tasklet);
 	list_del(&vcc_data->list);
-	tasklet_enable(&instance->receive_tasklet);
+	tasklet_enable(&instance->rx_tasklet);
 
 	kfree_skb(vcc_data->sarb);
 	vcc_data->sarb = NULL;
@@ -951,7 +959,7 @@ static int usbatm_atm_ioctl(struct atm_d
 static int usbatm_atm_init(struct usbatm_data *instance)
 {
 	struct atm_dev *atm_dev;
-	int ret;
+	int ret, i;
 
 	/* ATM init */
 	atm_dev = atm_dev_register(instance->driver_name, &usbatm_atm_devops, -1, NULL);
@@ -979,6 +987,12 @@ static int usbatm_atm_init(struct usbatm
 	mb();
 	atm_dev->dev_data = instance;
 
+	/* submit all rx URBs */
+	for (i = 0; i < num_rcv_urbs; i++)
+		usbatm_submit(instance->transceivers + i, instance->rx_endpoint,
+			      rcv_buf_size * (ATM_CELL_SIZE + instance->rx_padding),
+			      usbatm_rx_complete);
+
 	return 0;
 
  fail:
@@ -1107,76 +1121,54 @@ int usbatm_usb_probe (struct usb_interfa
 
 	INIT_LIST_HEAD(&instance->vcc_list);
 
-	spin_lock_init(&instance->receive_lock);
-	INIT_LIST_HEAD(&instance->spare_receivers);
-	INIT_LIST_HEAD(&instance->filled_receive_buffers);
-
-	tasklet_init(&instance->receive_tasklet, usbatm_process_receive, (unsigned long)instance);
-	INIT_LIST_HEAD(&instance->spare_receive_buffers);
+	spin_lock_init(&instance->rx_lock);
+	INIT_LIST_HEAD(&instance->filled_receivers);
+	tasklet_init(&instance->rx_tasklet, usbatm_rx_process, (unsigned long)instance);
 
-	skb_queue_head_init(&instance->sndqueue);
-
-	spin_lock_init(&instance->send_lock);
+	spin_lock_init(&instance->tx_lock);
 	INIT_LIST_HEAD(&instance->spare_senders);
-	INIT_LIST_HEAD(&instance->spare_send_buffers);
-
-	tasklet_init(&instance->send_tasklet, usbatm_process_send, (unsigned long)instance);
-	INIT_LIST_HEAD(&instance->filled_send_buffers);
-
-	/* receive init */
-	for (i = 0; i < num_rcv_urbs; i++) {
-		struct usbatm_receiver *rcv = &(instance->receivers[i]);
+	tasklet_init(&instance->tx_tasklet, usbatm_tx_process, (unsigned long)instance);
 
-		if (!(rcv->urb = usb_alloc_urb(0, GFP_KERNEL))) {
-			dev_dbg(&intf->dev, "no memory for receive urb %d!\n", i);
-			goto fail_unbind;
-		}
-
-		rcv->instance = instance;
+	skb_queue_head_init(&instance->sndqueue);
 
-		list_add(&rcv->list, &instance->spare_receivers);
+	instance->transceivers = kmalloc(sizeof(*instance->transceivers) * (num_rcv_urbs + num_snd_urbs),
+					 GFP_KERNEL);
+	if (!instance->transceivers) {
+		dev_dbg(&intf->dev, "no memory for receive urb %d!\n", i);
+		goto fail_unbind;
 	}
 
-	for (i = 0; i < num_rcv_bufs; i++) {
-		struct usbatm_receive_buffer *buf = &(instance->receive_buffers[i]);
-
-		buf->base = kmalloc(rcv_buf_size * (ATM_CELL_SIZE + instance->rx_padding),
-				    GFP_KERNEL);
-		if (!buf->base) {
-			dev_dbg(&intf->dev, "no memory for receive buffer %d!\n", i);
-			goto fail_unbind;
-		}
-
-		list_add(&buf->list, &instance->spare_receive_buffers);
-	}
+	memset(instance->transceivers, 0, sizeof(*instance->transceivers) * (num_rcv_urbs + num_snd_urbs));
 
-	/* send init */
-	for (i = 0; i < num_snd_urbs; i++) {
-		struct usbatm_sender *snd = &(instance->senders[i]);
+	for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
+		struct usbatm_transceiver *trx = instance->transceivers + i;
 
-		if (!(snd->urb = usb_alloc_urb(0, GFP_KERNEL))) {
-			dev_dbg(&intf->dev, "usbatm_usb_probe: no memory for send urb %d!\n", i);
+		trx->buffer = kmalloc(i < num_rcv_urbs ?
+				      rcv_buf_size * (ATM_CELL_SIZE + instance->rx_padding) :
+				      snd_buf_size * (ATM_CELL_SIZE + instance->tx_padding),
+				      GFP_KERNEL);
+		if (!trx->buffer) {
+			dev_dbg(&intf->dev, "no memory for buffer %d!\n", i);
 			goto fail_unbind;
 		}
 
-		snd->instance = instance;
-
-		list_add(&snd->list, &instance->spare_senders);
-	}
 
-	for (i = 0; i < num_snd_bufs; i++) {
-		struct usbatm_send_buffer *buf = &(instance->send_buffers[i]);
-
-		buf->base = kmalloc(snd_buf_size * (ATM_CELL_SIZE + instance->tx_padding),
-				    GFP_KERNEL);
-		if (!buf->base) {
-			dev_dbg(&intf->dev, "no memory for send buffer %d!\n", i);
+		trx->urb = usb_alloc_urb(0, GFP_KERNEL);
+		if (!trx->urb) {
+			dev_dbg(&intf->dev, "no memory for urb %d!\n", i);
 			goto fail_unbind;
 		}
+		
+		vdbg("alloced trx 0x%p buffer 0x%p urb 0x%p",
+		     trx, trx->buffer, trx->urb);
 
-		list_add(&buf->list, &instance->spare_send_buffers);
+		trx->instance = instance;
 	}
 
+	/* put all tx URBs on the list of spares */
+	for (i = 0; i < num_snd_urbs; i++)
+		list_add(&instance->transceivers[num_rcv_urbs + i].list, &instance->spare_senders);
+
 	if (need_heavy && driver->heavy_init) {
 		error = usbatm_heavy_init(instance);
 	} else {
@@ -1196,18 +1188,13 @@ int usbatm_usb_probe (struct usb_interfa
 	if (instance->driver->unbind)
 		instance->driver->unbind(instance, intf);
  fail_free:
-	for (i = 0; i < num_snd_bufs; i++)
-		kfree(instance->send_buffers[i].base);
-
-	for (i = 0; i < num_snd_urbs; i++)
-		usb_free_urb(instance->senders[i].urb);
-
-	for (i = 0; i < num_rcv_bufs; i++)
-		kfree(instance->receive_buffers[i].base);
-
-	for (i = 0; i < num_rcv_urbs; i++)
-		usb_free_urb(instance->receivers[i].urb);
+	if (instance->transceivers)
+		for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
+			kfree(instance->transceivers[i].buffer);
+			usb_free_urb(instance->transceivers[i].urb);
+		}
 
+	kfree(instance->transceivers);
 	kfree (instance);
 
 	return error;
@@ -1235,49 +1222,32 @@ void usbatm_usb_disconnect(struct usb_in
 
 	wait_for_completion(&instance->thread_exited);
 
-	tasklet_disable(&instance->receive_tasklet);
-	tasklet_disable(&instance->send_tasklet);
+	tasklet_disable(&instance->rx_tasklet);
+	tasklet_disable(&instance->tx_tasklet);
 
 	if (instance->atm_dev && instance->driver->atm_stop)
 		instance->driver->atm_stop(instance, instance->atm_dev);
 
+	for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++)
+		usb_kill_urb(instance->transceivers[i].urb);
+
 	if (instance->driver->unbind)
 		instance->driver->unbind(instance, intf);
 
-	/* receive finalize */
-
-	for (i = 0; i < num_rcv_urbs; i++)
-		usb_kill_urb(instance->receivers[i].urb);
-
-	/* no need to take the spinlock */
-	INIT_LIST_HEAD(&instance->filled_receive_buffers);
-	INIT_LIST_HEAD(&instance->spare_receive_buffers);
-
-	tasklet_enable(&instance->receive_tasklet);
-
-	for (i = 0; i < num_rcv_urbs; i++)
-		usb_free_urb(instance->receivers[i].urb);
-
-	for (i = 0; i < num_rcv_bufs; i++)
-		kfree(instance->receive_buffers[i].base);
-
-	/* send finalize */
-
-	for (i = 0; i < num_snd_urbs; i++)
-		usb_kill_urb(instance->senders[i].urb);
-
+	/* turn usbatm_[rt]x_process into noop */
 	/* no need to take the spinlock */
+	INIT_LIST_HEAD(&instance->filled_receivers);
 	INIT_LIST_HEAD(&instance->spare_senders);
-	INIT_LIST_HEAD(&instance->spare_send_buffers);
-	instance->current_buffer = NULL;
 
-	tasklet_enable(&instance->send_tasklet);
+	tasklet_enable(&instance->rx_tasklet);
+	tasklet_enable(&instance->tx_tasklet);
 
-	for (i = 0; i < num_snd_urbs; i++)
-		usb_free_urb(instance->senders[i].urb);
+	for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
+		usb_free_urb(instance->transceivers[i].urb);
+		kfree(instance->transceivers[i].buffer);
+	}
 
-	for (i = 0; i < num_snd_bufs; i++)
-		kfree(instance->send_buffers[i].base);
+	kfree(instance->transceivers);
 
 	/* ATM finalize */
 	if (instance->atm_dev)
@@ -1303,8 +1273,6 @@ static int __init usbatm_usb_init(void)
 
 	if ((num_rcv_urbs > UDSL_MAX_RCV_URBS)
 	    || (num_snd_urbs > UDSL_MAX_SND_URBS)
-	    || (num_rcv_bufs > UDSL_MAX_RCV_BUFS)
-	    || (num_snd_bufs > UDSL_MAX_SND_BUFS)
 	    || (rcv_buf_size > UDSL_MAX_RCV_BUF_SIZE)
 	    || (snd_buf_size > UDSL_MAX_SND_BUF_SIZE))
 		return -EINVAL;



More information about the Usbatm mailing list