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

Roman Kagan rkagan at mail.ru
Fri Mar 11 14:39:27 EST 2005


  Hi,

Having spent several days reparing my parent's kitchen, with my hands
very busy and my head free, I came up with an idea to put my nose into
the guts of usbatm transmittion and reception code.

Below is the patch to rearrange it a bit.  More specifically, it

 1) removes the distinction between sending and receiving buffers, and
    sticks the buffer with the urb to serve it onto the struct
    usbatm_transceiver, which is to replace structs usbatm_receiver,
    usbatm_sender, usbatm_receive_buffer and usbatm_send_buffer;

 2) maintains only one list of transceivers per direction:
    spare_senders and filled_receivers;

 3) moves some common code to a separate functions.

As a result it IMHO makes the logic in the transmittion and
reception paths more explicit, namely

reception:
  All spare receivers are kept submitted; the completion handler puts
  the receiver onto the filled_receivers queue and reschedules the
  rx_tasklet; the latter pops the receivers from the queue, copies on
  the data to the ATM layer and immediately resubmits them.

transmittion:
  The tx_tasklet pulls the ATM data from the skbqueue, pops the senders
  from the spare_senders list, copies the skbdata there and submits
  them; the completion handler returns senders onto the spare_senders
  list.

  One point worth mentioning here is that the patch reproduces the
  behavior of the original code, such that if there are any sending urbs
  which haven't yet completed, the submission of the current_sender is
  delayed and the ATM data gets accumulated on it.  Presumably this is
  meant to optimize the slow part by merging quickly arriving small skbs
  into bigger urbs.  I'm not certain this really makes anything faster,
  and I've no idea how to test it, but if it doesn't the code can be
  simplified further. 


In addition, I've put in some code to reply to OAM F5 Loopback cells
without passing them to the ATM layer (F4 should be easy too).
Unfortunatly I have no way to test it, my ISP doesn't use it.  I had to
include crc10 lookup table, which may eventually be factored out into a
library module; this explains why the diffstat is not that cool :)

Please comment / consider applying.

Cheers,
  Roman.

 usbatm.c |  648 ++++++++++++++++++++++++++++++---------------------------------
 usbatm.h |   72 +------
 2 files changed, 324 insertions(+), 396 deletions(-)


--- a/usbatm.h	2005-02-08 02:50:41.000000000 +0300
+++ b/usbatm.h	2005-03-11 20:05:17.000000000 +0300
@@ -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,23 @@ 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;
+	struct usbatm_transceiver *current_sender;	/* being filled */
+	atomic_t n_submitted_senders;
 };
 
 #endif	/* _USBATM_H_ */
--- a/usbatm.c	2005-02-08 02:50:41.000000000 +0300
+++ b/usbatm.c	2005-03-11 21:28:27.000000000 +0300
@@ -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,158 @@ 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);
+
+	atomic_dec(&instance->n_submitted_senders);
+	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);
+	}
+	else
+		atomic_inc(&instance->n_submitted_senders);
+
+	return ret;
+}
+
 /*************
 **  decode  **
 *************/
@@ -230,24 +364,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 +395,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 +416,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 +526,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 +590,6 @@ static unsigned int usbatm_write_cells(s
 		target += instance->tx_padding;
 	}
  out:
-	*target_p = target;
 	return nc - ctrl->num_cells;
 }
 
@@ -457,223 +597,95 @@ 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;
-
-	if (!urb || !(snd = urb->context) || !(instance = snd->instance)) {
-		dbg("usbatm_complete_send: bad urb!");
-		return;
-	}
-
-	vdbg("usbatm_complete_send: urb 0x%p, status %d, snd 0x%p, buf 0x%p", urb,
-	     urb->status, snd, snd->buffer);
-
-	/* 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);
-}
-
-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;
+	struct usbatm_transceiver *tx;
+	int num_written;
 
- 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 */
-		}
+	while (1) {
+		if (!instance->current_skb)
+			instance->current_skb = skb_dequeue(&instance->sndqueue);
+		if (!instance->current_skb)
+			return;		/* done - no more skbs */
+
+		if (!instance->current_sender)
+			instance->current_sender = usbatm_pop_transceiver(&instance->spare_senders,
+									  &instance->tx_lock);
+		if (!instance->current_sender)
+			return;		/* no spare senders */
+
+		skb = instance->current_skb;
+		tx = instance->current_sender;
+
+		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);
+
+		tx->filled_cells += num_written;
+
+		vdbg("usbatm_tx_process: buffer contains %d cells, %d left",
+		     tx->filled_cells, snd_buf_size - tx->filled_cells);
+
+		if (!atomic_read(&instance->n_submitted_senders) ||	/* no pending senders: send immediately */
+		    tx->filled_cells == snd_buf_size) {
+			if (usbatm_submit(tx, instance->tx_endpoint,
+					  tx->filled_cells * (ATM_CELL_SIZE + instance->tx_padding),
+					  usbatm_tx_complete))
+				/* consider all errors transient */
+				/* FIXME: may result in deadlock */
+				return;
 
-		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 */
+			instance->current_sender = NULL;
+			atomic_inc(&instance->n_submitted_senders);
 		}
-		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;
-	}
+		if (!UDSL_SKB(skb)->num_cells) {
+			struct atm_vcc *vcc = UDSL_SKB(skb)->atm_data.vcc;
 
-	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;
+			usbatm_pop(vcc, skb);
+			instance->current_skb = NULL;
 
-		atomic_inc(&vcc->stats->tx);
+			atomic_inc(&vcc->stats->tx);
+		}
 	}
-
-	goto made_progress;
 }
 
 static void usbatm_cancel_send(struct usbatm_data *instance,
-			     struct atm_vcc *vcc)
+			       struct atm_vcc *vcc)
 {
 	struct sk_buff *skb, *n;
 
@@ -689,13 +701,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 +741,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 +761,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 +892,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 +902,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 +928,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 +963,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 +991,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:
@@ -1043,7 +1061,9 @@ int usbatm_usb_probe (struct usb_interfa
 	int need_heavy;
 
 	dev_dbg(&intf->dev, "trying driver %s with vendor=0x%x, product=0x%x, ifnum %d\n",
-			driver->driver_name, dev->descriptor.idVendor, dev->descriptor.idProduct,
+			driver->driver_name,
+			le16_to_cpu(dev->descriptor.idVendor),
+			le16_to_cpu(dev->descriptor.idProduct),
 			intf->altsetting->desc.bInterfaceNumber);
 
 	/* instance init */
@@ -1105,76 +1125,55 @@ 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);
+	spin_lock_init(&instance->rx_lock);
+	INIT_LIST_HEAD(&instance->filled_receivers);
+	tasklet_init(&instance->rx_tasklet, usbatm_rx_process, (unsigned long)instance);
 
-	tasklet_init(&instance->receive_tasklet, usbatm_process_receive, (unsigned long)instance);
-	INIT_LIST_HEAD(&instance->spare_receive_buffers);
-
-	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);
+	tasklet_init(&instance->tx_tasklet, usbatm_tx_process, (unsigned long)instance);
 
-	/* receive init */
-	for (i = 0; i < num_rcv_urbs; i++) {
-		struct usbatm_receiver *rcv = &(instance->receivers[i]);
-
-		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);
+	atomic_set(&instance->n_submitted_senders, 0);
 
-		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]);
+	memset(instance->transceivers, 0, sizeof(*instance->transceivers) * (num_rcv_urbs + num_snd_urbs));
 
-		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);
-	}
+	for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
+		struct usbatm_transceiver *trx = instance->transceivers + i;
 
-	/* send init */
-	for (i = 0; i < num_snd_urbs; i++) {
-		struct usbatm_sender *snd = &(instance->senders[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 {
@@ -1194,18 +1193,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;
@@ -1233,49 +1227,33 @@ 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;
+	instance->current_sender = 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)
@@ -1301,8 +1279,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