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,
