Re: [PATCH] usbnet: allow rx_process() to ignore packets

[David Brownell <david-b@pacbell.net>]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

--- On Tue, 9/7/10, Ondrej Zary <linux@rainbow-software.org> wrote:
> 
> It's not a garbage, just a packet that is not yet
> complete.

Sorry, I guess I was being dense.

I thought packets were by definition complete... :)

I think you're saying this is one of N fragments
which the minidriver will be re-assembling into
a bigger packet, before somehow passing it up
the stack...

I attach my doc update patch.  (should be the
same as what I sent before.  Consider my apology
for using an attachment as repeated here; my email
setup remains partly broken).  Can you send
a version of your defrag-enabling patch which
applies on top of it, and documents your new
calling convention (with a third return case
for rx_process() too?

[-- Attachment #2: rxfix --]
[-- Type: application/octet-stream, Size: 2492 bytes --]

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Subject: usbnet: doc updates

Provide more documentation about the rx_fixup() calling convention,
to help reduce some recently-observed confusion.

Also summarize the equivalence of USB and network TX/RX queues, to
help highlight the roles of the fixup() routines with link protocols
using the CPU-intensive packet batching models.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---


 include/linux/usb/usbnet.h |   31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -22,8 +22,22 @@
 #ifndef	__LINUX_USB_USBNET_H
 #define	__LINUX_USB_USBNET_H
 
-/* interface from usbnet core to each USB networking link we handle */
+/** interface from usbnet core to each USB networking link we handle.
+ *
+ * Note that the design center for usbnet has one IEEE packet per URB,
+ * and maps network TX and RX queues directly to the
+ * USB hardware TX/RX queues, minimizing software and hardware overhead.
+ * Some link protocols (like RNDIS and CDC NCM) promote  less efficient
+ * designs which involve batching multiple IEEE packets in each URB,
+ * with increased TX overhead to copy IEEE packets into TX URBs' data
+ * buffers in tx_fixup() routines, plus unbatching work in rx_fixup() too.
+ * Sometimes the TX overhead can be minimized on Linux by not batching, but
+ * this usually won't work for RX because of the need to interoperate with
+ * implementations which batch on TX.
+ */
+
 struct usbnet {
+
 	/* housekeeping */
 	struct usb_device	*udev;
 	struct usb_interface	*intf;
@@ -113,7 +127,20 @@ struct driver_info {
 	/* link reset handling, called from defer_kevent */
 	int	(*link_reset)(struct usbnet *);
 
-	/* fixup rx packet (strip framing) */
+	/* fixup rx packet ( by stripping framing from URB/SKB.
+	 *
+	 * If SKB batches one or more packets, pass all but one of them
+	 * up the stack (clone SKB and fixup each clone before netif_rx)).
+	 *
+
+	 * When you leave one packet in SKB (intended usage), return
+	 * nonzero and ensure skb-len is nonzero.  the packet will be
+	 * passed up the network stack and accounted properly.
+	 *
+	 * return zero to indicate errors such as too much header or
+	 * data, or a bad checksum; the packet will be dropped
+	 * and accounted as an error.
+	*/
 	int	(*rx_fixup)(struct usbnet *dev, struct sk_buff *skb);
 
 	/* fixup tx packet (add framing) */