Re: [PATCH 2/2] usb: gadget: ncm: Add support to update wMaxSegmentSize via configfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/10/2023 10:08 AM, Krishna Kurapati PSSNV wrote:



^ is this a problem now if we have >1 gadget?
how does it work then?


You are right. This would effect unwrap call and the wMaxSegmentSize is used directly. Thanks for the catch. I didn't test with 2 NCM interfaces and hence I wasn't able to find this bug. Perhaps changing this to opts->max_segment_size would fix the implementation as unwrap would anyways be called after bind.

Hi Maciej,

 How about the below diff:

---------

+/*
+ * Allow max segment size to be in parity with max_mtu possible
+ * for the interface.
+ */
+#define MAX_DATAGRAM_SIZE      GETHER_MAX_ETH_FRAME_LEN
+
 #define FORMATS_SUPPORTED      (USB_CDC_NCM_NTB16_SUPPORTED |  \
                                 USB_CDC_NCM_NTB32_SUPPORTED)

@@ -194,7 +200,6 @@ static struct usb_cdc_ether_desc ecm_desc = {
        /* this descriptor actually adds value, surprise! */
        /* .iMACAddress = DYNAMIC */
        .bmEthernetStatistics = cpu_to_le32(0), /* no statistics */
-       .wMaxSegmentSize =      cpu_to_le16(ETH_FRAME_LEN),
        .wNumberMCFilters =     cpu_to_le16(0),
        .bNumberPowerFilters =  0,
 };
@@ -1180,10 +1185,15 @@ static int ncm_unwrap_ntb(struct gether *port,
        struct sk_buff  *skb2;
        int             ret = -EINVAL;
unsigned ntb_max = le32_to_cpu(ntb_parameters.dwNtbOutMaxSize);
-       unsigned        frame_max = le16_to_cpu(ecm_desc.wMaxSegmentSize);
+       unsigned int    frame_max;
        const struct ndp_parser_opts *opts = ncm->parser_opts;
        unsigned        crc_len = ncm->is_crc ? sizeof(uint32_t) : 0;
        int             dgram_counter;
+       struct f_ncm_opts *ncm_opts;
+       const struct usb_function_instance *fi = port->func.fi;
+
+       ncm_opts = container_of(fi, struct f_ncm_opts, func_inst);
+       frame_max = ncm_opts->max_segment_size;

        /* dwSignature */
        if (get_unaligned_le32(tmp) != opts->nth_sign) {
@@ -1440,6 +1450,7 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
         */
        if (!ncm_opts->bound) {
                mutex_lock(&ncm_opts->lock);
+ ncm_opts->net->mtu = (ncm_opts->max_segment_size - ETH_HLEN);
                gether_set_gadget(ncm_opts->net, cdev->gadget);
                status = gether_register_netdev(ncm_opts->net);
                mutex_unlock(&ncm_opts->lock);
@@ -1484,6 +1495,8 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)

        status = -ENODEV;

+       ecm_desc.wMaxSegmentSize = (__le16)ncm_opts->max_segment_size;
+

------

I can limit the max segment size to (Max MTU + ETH_HELN) and this would be logical to do. Also we can set the frame_max from ncm_opts itself while initializing it to 1514 (default value) during alloc_inst callback and nothing would break while still being backward compatible.

Let me know your thoughts on this.

Regards,
Krishna,




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux