The NCM specification as per
http://www.usb.org/developers/docs/devclass_docs/NCM10_012011.zip
does not define a fixed position for different frame parts.
The NCM header is effectively the head of a list of NDPs (NCM datagram
pointers), each of them pointing to a set of ethernet frames.
In general, the NDP can be anywhere after the header.
Some devices however are not quite respecting the specs - as they mandate
the NDP pointers to be after the payload, at the end of the NCM package.
This patch aims to support this scenario, introducing a module parameter
enabling this funcitonality.

Is this approach acceptable?

What does work:
- the module compiles, and seems to cause no crash
What doesn't:
- the device for now will ignore our frames
- I would need some guidance on flags to use with kzalloc.

thank you for the patience, and the review.

Signed-off-by: Enrico Mioso <mrkiko...@gmail.com>
---
 drivers/net/usb/cdc_ncm.c   | 30 ++++++++++++++++++++++++++++--
 include/linux/usb/cdc_ncm.h |  1 +
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 8067b8f..a6d0666b 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -60,6 +60,10 @@ static bool prefer_mbim;
 module_param(prefer_mbim, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(prefer_mbim, "Prefer MBIM setting on dual NCM/MBIM 
functions");
 
+static bool move_ndp_to_end;
+module_param(move_ndp_to_end, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(move_ndp_to_end, "Move NDP block to the end of the NCM 
aggregate");
+
 static void cdc_ncm_txpath_bh(unsigned long param);
 static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
 static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
@@ -684,6 +688,8 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
                ctx->tx_curr_skb = NULL;
        }
 
+       kfree(ctx->delayed_ndp);
+
        kfree(ctx);
 }
 
@@ -994,6 +1000,9 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct 
cdc_ncm_ctx *ctx, struct sk_
                ndpoffset = le16_to_cpu(ndp16->wNextNdpIndex);
        }
 
+       if ((move_ndp_to_end) && (ctx->delayed_ndp->dwSignature == sign))
+               return ndp16;
+
        /* align new NDP */
        cdc_ncm_align_tail(skb, ctx->tx_ndp_modulus, 0, ctx->tx_max);
 
@@ -1008,7 +1017,13 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct 
cdc_ncm_ctx *ctx, struct sk_
                nth16->wNdpIndex = cpu_to_le16(skb->len);
 
        /* push a new empty NDP */
-       ndp16 = (struct usb_cdc_ncm_ndp16 *)memset(skb_put(skb, 
ctx->max_ndp_size), 0, ctx->max_ndp_size);
+       if (!move_ndp_to_end) {
+               ndp16 = (struct usb_cdc_ncm_ndp16 *)memset(skb_put(skb, 
ctx->max_ndp_size), 0, ctx->max_ndp_size);
+       } else {
+               ndp16 = kzalloc(sizeof(*ndp16), GFP_KERNEL);
+               ctx->delayed_ndp = ndp16;
+       }
+
        ndp16->dwSignature = sign;
        ndp16->wLength = cpu_to_le16(sizeof(struct usb_cdc_ncm_ndp16) + 
sizeof(struct usb_cdc_ncm_dpe16));
        return ndp16;
@@ -1023,6 +1038,8 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff 
*skb, __le32 sign)
        struct sk_buff *skb_out;
        u16 n = 0, index, ndplen;
        u8 ready2send = 0;
+       unsigned int skb_prev_len;
+       char *delayed_alloc_ptr;
 
        /* if there is a remaining skb, it gets priority */
        if (skb != NULL) {
@@ -1074,7 +1091,8 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff 
*skb, __le32 sign)
                ndp16 = cdc_ncm_ndp(ctx, skb_out, sign, skb->len + 
ctx->tx_modulus + ctx->tx_remainder);
 
                /* align beginning of next frame */
-               cdc_ncm_align_tail(skb_out,  ctx->tx_modulus, 
ctx->tx_remainder, ctx->tx_max);
+               if (!move_ndp_to_end)
+                       cdc_ncm_align_tail(skb_out,  ctx->tx_modulus, 
ctx->tx_remainder, ctx->tx_max);
 
                /* check if we had enough room left for both NDP and frame */
                if (!ndp16 || skb_out->len + skb->len > ctx->tx_max) {
@@ -1111,6 +1129,14 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff 
*skb, __le32 sign)
                dev_kfree_skb_any(skb);
                skb = NULL;
 
+               if ((move_ndp_to_end) && (ctx->delayed_ndp)) {
+                       skb_prev_len = skb_out->len;
+                       delayed_alloc_ptr = memset(skb_put(skb_out, 
ctx->max_ndp_size), 0, ctx->max_ndp_size);
+                       memcpy(ctx->delayed_ndp, delayed_alloc_ptr, 
sizeof(struct usb_cdc_ncm_ndp16));
+                       kfree(ctx->delayed_ndp);
+                       cdc_ncm_align_tail(skb_out,  ctx->tx_modulus, 
ctx->tx_remainder, ctx->tx_max);
+               }
+
                /* send now if this NDP is full */
                if (index >= CDC_NCM_DPT_DATAGRAMS_MAX) {
                        ready2send = 1;
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 7c9b484..cc02a0d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -93,6 +93,7 @@ struct cdc_ncm_ctx {
        const struct usb_cdc_mbim_desc *mbim_desc;
        const struct usb_cdc_mbim_extended_desc *mbim_extended_desc;
        const struct usb_cdc_ether_desc *ether_desc;
+       struct usb_cdc_ncm_ndp16 *delayed_ndp;
 
        struct usb_interface *control;
        struct usb_interface *data;
-- 
2.4.2

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to