[PATCH v2 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[PATCH v2 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[Łukasz Spintzyk]

From: Dominik Czerwik <dominik.czerwik@synaptics.com>

This improves performance and stability of
DL-3xxx/DL-5xxx/DL-6xxx device series.

Specifically prevents device from temporary network dropouts when
playing video from the web and network traffic going through is high.

Signed-off-by: Bernice Chen <Bernice.Chen@synaptics.com>
Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
---

v2: Added Bernice Chen as company lawyer.

 drivers/net/usb/cdc_ncm.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d55f59ce4a31..4594bf2982ee 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -2,6 +2,7 @@
  * cdc_ncm.c
  *
  * Copyright (C) ST-Ericsson 2010-2012
+ * Copyright (C) 2022 Synaptics Incorporated. All rights reserved.
  * Contact: Alexey Orishko <alexey.orishko@stericsson.com>
  * Original author: Hans Petter Selasky <hans.petter.selasky@stericsson.com>
  *
@@ -1904,6 +1905,19 @@ static const struct driver_info cdc_ncm_info = {
 	.set_rx_mode = usbnet_cdc_update_filter,
 };
 
+/* Same as cdc_ncm_info, but with FLAG_SEND_ZLP  */
+static const struct driver_info cdc_ncm_zlp_info = {
+	.description = "CDC NCM (SEND ZLP)",
+	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+			| FLAG_LINK_INTR | FLAG_ETHER | FLAG_SEND_ZLP,
+	.bind = cdc_ncm_bind,
+	.unbind = cdc_ncm_unbind,
+	.manage_power = usbnet_manage_power,
+	.status = cdc_ncm_status,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
 /* Same as cdc_ncm_info, but with FLAG_WWAN */
 static const struct driver_info wwan_info = {
 	.description = "Mobile Broadband Network Device",
@@ -2010,6 +2024,16 @@ static const struct usb_device_id cdc_devs[] = {
 	  .driver_info = (unsigned long)&wwan_info,
 	},
 
+	/* DisplayLink docking stations */
+	{ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO
+		| USB_DEVICE_ID_MATCH_VENDOR,
+	  .idVendor = 0x17e9,
+	  .bInterfaceClass = USB_CLASS_COMM,
+	  .bInterfaceSubClass = USB_CDC_SUBCLASS_NCM,
+	  .bInterfaceProtocol = USB_CDC_PROTO_NONE,
+	  .driver_info = (unsigned long)&cdc_ncm_zlp_info,
+	},
+
 	/* Generic CDC-NCM devices */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM,
 		USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
-- 
2.36.1

[PATCH v2 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb

[Łukasz Spintzyk]

DisplayLink ethernet devices require NTB buffers larger then 32kb
in order to run with highest performance.

This patch is changing upper limit of the rx and tx buffers.
Those buffers are initialized with CDC_NCM_NTB_DEF_SIZE_RX and
CDC_NCM_NTB_DEF_SIZE_TX which is 16kb so by default no device is
affected by increased limit.

Rx and tx buffer is increased under two conditions:
 - Device need to advertise that it supports higher buffer size in
   dwNtbMaxInMaxSize and dwNtbMaxOutMaxSize.
 - cdc_ncm/rx_max and cdc_ncm/tx_max driver parameters must be adjusted
   with udev rule or ethtool.

Summary of testing and performance results:
Tests were performed on following devices:
 - DisplayLink DL-3xxx family device
 - DisplayLink DL-6xxx family device
 - ASUS USB-C2500 2.5G USB3 ethernet adapter
 - Plugable USB3 1G USB3 ethernet adapter
 - EDIMAX EU-4307 USB-C ethernet adapter
 - Dell DBQBCBC064 USB-C ethernet adapter

Performance measurements were done with:
 - iperf3 between two linux boxes
 - http://openspeedtest.com/ instance running on local test machine

Insights from tests results:
 - All except one from third party usb adapters were not affected by
   increased buffer size to their advertised dwNtbOutMaxSize and
   dwNtbInMaxSize.
   Devices were generally reaching 912-940Mbps both download and upload.

   Only EDIMAX adapter experienced decreased download size from
   929Mbps to 827Mbps with iper3, with openspeedtest decrease was from
   968Mbps to 886Mbps.

 - DisplayLink DL-3xxx family devices experienced performance increase
   with iperf3 download from 300Mbps to 870Mbps and
   upload from 782Mbps to 844Mbps.
   With openspeedtest download increased from 556Mbps to 873Mbps
   and upload from 727Mbps to 973Mbps

 - DiplayLink DL-6xxx family devices are not affected by
   increased buffer size.

Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
---

v2:
 - Information how to change tx,rx buffer size
 - Added performance tests results to the commit description.


 include/linux/usb/cdc_ncm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index f7cb3ddce7fb..2d207cb4837d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -53,8 +53,8 @@
 #define USB_CDC_NCM_NDP32_LENGTH_MIN		0x20
 
 /* Maximum NTB length */
-#define	CDC_NCM_NTB_MAX_SIZE_TX			32768	/* bytes */
-#define	CDC_NCM_NTB_MAX_SIZE_RX			32768	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_TX			65536	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_RX			65536	/* bytes */
 
 /* Initial NTB length */
 #define	CDC_NCM_NTB_DEF_SIZE_TX			16384	/* bytes */
-- 
2.36.1

Re: [PATCH v2 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb

[Greg KH]

On Thu, Jul 14, 2022 at 02:02:17PM +0200, Łukasz Spintzyk wrote:
> DisplayLink ethernet devices require NTB buffers larger then 32kb
> in order to run with highest performance.
> 
> This patch is changing upper limit of the rx and tx buffers.
> Those buffers are initialized with CDC_NCM_NTB_DEF_SIZE_RX and
> CDC_NCM_NTB_DEF_SIZE_TX which is 16kb so by default no device is
> affected by increased limit.
> 
> Rx and tx buffer is increased under two conditions:
>  - Device need to advertise that it supports higher buffer size in
>    dwNtbMaxInMaxSize and dwNtbMaxOutMaxSize.
>  - cdc_ncm/rx_max and cdc_ncm/tx_max driver parameters must be adjusted
>    with udev rule or ethtool.
> 
> Summary of testing and performance results:
> Tests were performed on following devices:
>  - DisplayLink DL-3xxx family device
>  - DisplayLink DL-6xxx family device
>  - ASUS USB-C2500 2.5G USB3 ethernet adapter
>  - Plugable USB3 1G USB3 ethernet adapter
>  - EDIMAX EU-4307 USB-C ethernet adapter
>  - Dell DBQBCBC064 USB-C ethernet adapter
> 
> Performance measurements were done with:
>  - iperf3 between two linux boxes
>  - http://openspeedtest.com/ instance running on local test machine
> 
> Insights from tests results:
>  - All except one from third party usb adapters were not affected by
>    increased buffer size to their advertised dwNtbOutMaxSize and
>    dwNtbInMaxSize.
>    Devices were generally reaching 912-940Mbps both download and upload.
> 
>    Only EDIMAX adapter experienced decreased download size from
>    929Mbps to 827Mbps with iper3, with openspeedtest decrease was from
>    968Mbps to 886Mbps.
> 
>  - DisplayLink DL-3xxx family devices experienced performance increase
>    with iperf3 download from 300Mbps to 870Mbps and
>    upload from 782Mbps to 844Mbps.
>    With openspeedtest download increased from 556Mbps to 873Mbps
>    and upload from 727Mbps to 973Mbps
> 
>  - DiplayLink DL-6xxx family devices are not affected by
>    increased buffer size.
> 
> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> ---
> 
> v2:
>  - Information how to change tx,rx buffer size
>  - Added performance tests results to the commit description.
> 
> 
>  include/linux/usb/cdc_ncm.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
> index f7cb3ddce7fb..2d207cb4837d 100644
> --- a/include/linux/usb/cdc_ncm.h
> +++ b/include/linux/usb/cdc_ncm.h
> @@ -53,8 +53,8 @@
>  #define USB_CDC_NCM_NDP32_LENGTH_MIN		0x20
>  
>  /* Maximum NTB length */
> -#define	CDC_NCM_NTB_MAX_SIZE_TX			32768	/* bytes */
> -#define	CDC_NCM_NTB_MAX_SIZE_RX			32768	/* bytes */
> +#define	CDC_NCM_NTB_MAX_SIZE_TX			65536	/* bytes */
> +#define	CDC_NCM_NTB_MAX_SIZE_RX			65536	/* bytes */
>  
>  /* Initial NTB length */
>  #define	CDC_NCM_NTB_DEF_SIZE_TX			16384	/* bytes */
> -- 
> 2.36.1
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks for the additional information in the changelog text.

Re: [PATCH v2 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[Greg KH]

On Thu, Jul 14, 2022 at 02:02:16PM +0200, Łukasz Spintzyk wrote:
> From: Dominik Czerwik <dominik.czerwik@synaptics.com>
> 
> This improves performance and stability of
> DL-3xxx/DL-5xxx/DL-6xxx device series.
> 
> Specifically prevents device from temporary network dropouts when
> playing video from the web and network traffic going through is high.
> 
> Signed-off-by: Bernice Chen <Bernice.Chen@synaptics.com>
> Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> ---
> 
> v2: Added Bernice Chen as company lawyer.

You forgot to cc: them, as they obviously want to be involved in this
process.  They do understand what Signed-off-by: means, right?

> 
>  drivers/net/usb/cdc_ncm.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index d55f59ce4a31..4594bf2982ee 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -2,6 +2,7 @@
>   * cdc_ncm.c
>   *
>   * Copyright (C) ST-Ericsson 2010-2012
> + * Copyright (C) 2022 Synaptics Incorporated. All rights reserved.

Bernice, please note that Dominik is only adding 23 lines to a 2039 line
file, making this a change that only affects a very small percentage of
the overall code, and affects the logic in no direct way (they are only
adding new device information.)

Based on that information, you still believe this warrents an addition
of a copyright notice?  Any pointers to legal rulings where this is
backed up would be appreciated as it goes against what I have been told
to allow by my lawyers.

thanks,

greg k-h

[PATCH v3 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[Łukasz Spintzyk]

From: Dominik Czerwik <dominik.czerwik@synaptics.com>

This improves performance and stability of
DL-3xxx/DL-5xxx/DL-6xxx device series.

Specifically prevents device from temporary network dropouts when
playing video from the web and network traffic going through is high.

Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
---

v3: We have decided to remove that copyright as this is not really 
    necessary.

    This is v3 revision of the patch series.
    Please forgive me it is wrongly resubmitted with git-send-mail.

 drivers/net/usb/cdc_ncm.c ssary +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d55f59ce4a31..af84ac0d65c9 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1904,6 +1904,19 @@ static const struct driver_info cdc_ncm_info = {
 	.set_rx_mode = usbnet_cdc_update_filter,
 };
 
+/* Same as cdc_ncm_info, but with FLAG_SEND_ZLP  */
+static const struct driver_info cdc_ncm_zlp_info = {
+	.description = "CDC NCM (SEND ZLP)",
+	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+			| FLAG_LINK_INTR | FLAG_ETHER | FLAG_SEND_ZLP,
+	.bind = cdc_ncm_bind,
+	.unbind = cdc_ncm_unbind,
+	.manage_power = usbnet_manage_power,
+	.status = cdc_ncm_status,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
 /* Same as cdc_ncm_info, but with FLAG_WWAN */
 static const struct driver_info wwan_info = {
 	.description = "Mobile Broadband Network Device",
@@ -2010,6 +2023,16 @@ static const struct usb_device_id cdc_devs[] = {
 	  .driver_info = (unsigned long)&wwan_info,
 	},
 
+	/* DisplayLink docking stations */
+	{ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO
+		| USB_DEVICE_ID_MATCH_VENDOR,
+	  .idVendor = 0x17e9,
+	  .bInterfaceClass = USB_CLASS_COMM,
+	  .bInterfaceSubClass = USB_CDC_SUBCLASS_NCM,
+	  .bInterfaceProtocol = USB_CDC_PROTO_NONE,
+	  .driver_info = (unsigned long)&cdc_ncm_zlp_info,
+	},
+
 	/* Generic CDC-NCM devices */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM,
 		USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
-- 
2.36.1

[PATCH v3 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb

[Łukasz Spintzyk]

DisplayLink ethernet devices require NTB buffers larger then 32kb
in order to run with highest performance.

This patch is changing upper limit of the rx and tx buffers.
Those buffers are initialized with CDC_NCM_NTB_DEF_SIZE_RX and
CDC_NCM_NTB_DEF_SIZE_TX which is 16kb so by default no device is
affected by increased limit.

Rx and tx buffer is increased under two conditions:
 - Device need to advertise that it supports higher buffer size in
   dwNtbMaxInMaxSize and dwNtbMaxOutMaxSize.
 - cdc_ncm/rx_max and cdc_ncm/tx_max driver parameters must be adjusted
   with udev rule or ethtool.

Summary of testing and performance results:
Tests were performed on following devices:
 - DisplayLink DL-3xxx family device
 - DisplayLink DL-6xxx family device
 - ASUS USB-C2500 2.5G USB3 ethernet adapter
 - Plugable USB3 1G USB3 ethernet adapter
 - EDIMAX EU-4307 USB-C ethernet adapter
 - Dell DBQBCBC064 USB-C ethernet adapter

Performance measurements were done with:
 - iperf3 between two linux boxes
 - http://openspeedtest.com/ instance running on local test machine

Insights from tests results:
 - All except one from third party usb adapters were not affected by
   increased buffer size to their advertised dwNtbOutMaxSize and
   dwNtbInMaxSize.
   Devices were generally reaching 912-940Mbps both download and upload.

   Only EDIMAX adapter experienced decreased download size from
   929Mbps to 827Mbps with iper3, with openspeedtest decrease was from
   968Mbps to 886Mbps.

 - DisplayLink DL-3xxx family devices experienced performance increase
   with iperf3 download from 300Mbps to 870Mbps and
   upload from 782Mbps to 844Mbps.
   With openspeedtest download increased from 556Mbps to 873Mbps
   and upload from 727Mbps to 973Mbps

 - DiplayLink DL-6xxx family devices are not affected by
   increased buffer size.

Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
---

 v3: No new changes to v2 2/2.
 It is just rebase on top of changed v3 1/2 patch

 include/linux/usb/cdc_ncm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index f7cb3ddce7fb..2d207cb4837d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -53,8 +53,8 @@
 #define USB_CDC_NCM_NDP32_LENGTH_MIN		0x20
 
 /* Maximum NTB length */
-#define	CDC_NCM_NTB_MAX_SIZE_TX			32768	/* bytes */
-#define	CDC_NCM_NTB_MAX_SIZE_RX			32768	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_TX			65536	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_RX			65536	/* bytes */
 
 /* Initial NTB length */
 #define	CDC_NCM_NTB_DEF_SIZE_TX			16384	/* bytes */
-- 
2.36.1

Re: [PATCH v3 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb

[Greg KH]

On Mon, Jul 18, 2022 at 02:36:18PM +0200, Łukasz Spintzyk wrote:
> DisplayLink ethernet devices require NTB buffers larger then 32kb
> in order to run with highest performance.
> 
> This patch is changing upper limit of the rx and tx buffers.
> Those buffers are initialized with CDC_NCM_NTB_DEF_SIZE_RX and
> CDC_NCM_NTB_DEF_SIZE_TX which is 16kb so by default no device is
> affected by increased limit.
> 
> Rx and tx buffer is increased under two conditions:
>  - Device need to advertise that it supports higher buffer size in
>    dwNtbMaxInMaxSize and dwNtbMaxOutMaxSize.
>  - cdc_ncm/rx_max and cdc_ncm/tx_max driver parameters must be adjusted
>    with udev rule or ethtool.
> 
> Summary of testing and performance results:
> Tests were performed on following devices:
>  - DisplayLink DL-3xxx family device
>  - DisplayLink DL-6xxx family device
>  - ASUS USB-C2500 2.5G USB3 ethernet adapter
>  - Plugable USB3 1G USB3 ethernet adapter
>  - EDIMAX EU-4307 USB-C ethernet adapter
>  - Dell DBQBCBC064 USB-C ethernet adapter
> 
> Performance measurements were done with:
>  - iperf3 between two linux boxes
>  - http://openspeedtest.com/ instance running on local test machine
> 
> Insights from tests results:
>  - All except one from third party usb adapters were not affected by
>    increased buffer size to their advertised dwNtbOutMaxSize and
>    dwNtbInMaxSize.
>    Devices were generally reaching 912-940Mbps both download and upload.
> 
>    Only EDIMAX adapter experienced decreased download size from
>    929Mbps to 827Mbps with iper3, with openspeedtest decrease was from
>    968Mbps to 886Mbps.
> 
>  - DisplayLink DL-3xxx family devices experienced performance increase
>    with iperf3 download from 300Mbps to 870Mbps and
>    upload from 782Mbps to 844Mbps.
>    With openspeedtest download increased from 556Mbps to 873Mbps
>    and upload from 727Mbps to 973Mbps
> 
>  - DiplayLink DL-6xxx family devices are not affected by
>    increased buffer size.
> 
> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> ---
> 
>  v3: No new changes to v2 2/2.
>  It is just rebase on top of changed v3 1/2 patch

Any reason you dropped my reviewed-by?

[PATCH v4 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[Łukasz Spintzyk]

From: Dominik Czerwik <dominik.czerwik@synaptics.com>

This improves performance and stability of
DL-3xxx/DL-5xxx/DL-6xxx device series.

Specifically prevents device from temporary network dropouts when
playing video from the web and network traffic going through is high.

Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
---
 drivers/net/usb/cdc_ncm.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d55f59ce4a31..af84ac0d65c9 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1904,6 +1904,19 @@ static const struct driver_info cdc_ncm_info = {
 	.set_rx_mode = usbnet_cdc_update_filter,
 };
 
+/* Same as cdc_ncm_info, but with FLAG_SEND_ZLP  */
+static const struct driver_info cdc_ncm_zlp_info = {
+	.description = "CDC NCM (SEND ZLP)",
+	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+			| FLAG_LINK_INTR | FLAG_ETHER | FLAG_SEND_ZLP,
+	.bind = cdc_ncm_bind,
+	.unbind = cdc_ncm_unbind,
+	.manage_power = usbnet_manage_power,
+	.status = cdc_ncm_status,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
 /* Same as cdc_ncm_info, but with FLAG_WWAN */
 static const struct driver_info wwan_info = {
 	.description = "Mobile Broadband Network Device",
@@ -2010,6 +2023,16 @@ static const struct usb_device_id cdc_devs[] = {
 	  .driver_info = (unsigned long)&wwan_info,
 	},
 
+	/* DisplayLink docking stations */
+	{ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO
+		| USB_DEVICE_ID_MATCH_VENDOR,
+	  .idVendor = 0x17e9,
+	  .bInterfaceClass = USB_CLASS_COMM,
+	  .bInterfaceSubClass = USB_CDC_SUBCLASS_NCM,
+	  .bInterfaceProtocol = USB_CDC_PROTO_NONE,
+	  .driver_info = (unsigned long)&cdc_ncm_zlp_info,
+	},
+
 	/* Generic CDC-NCM devices */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM,
 		USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
-- 
2.36.1

[PATCH v4 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb

[Łukasz Spintzyk]

DisplayLink ethernet devices require NTB buffers larger then 32kb
in order to run with highest performance.

This patch is changing upper limit of the rx and tx buffers.
Those buffers are initialized with CDC_NCM_NTB_DEF_SIZE_RX and
CDC_NCM_NTB_DEF_SIZE_TX which is 16kb so by default no device is
affected by increased limit.

Rx and tx buffer is increased under two conditions:
 - Device need to advertise that it supports higher buffer size in
   dwNtbMaxInMaxSize and dwNtbMaxOutMaxSize.
 - cdc_ncm/rx_max and cdc_ncm/tx_max driver parameters must be adjusted
   with udev rule or ethtool.

Summary of testing and performance results:
Tests were performed on following devices:
 - DisplayLink DL-3xxx family device
 - DisplayLink DL-6xxx family device
 - ASUS USB-C2500 2.5G USB3 ethernet adapter
 - Plugable USB3 1G USB3 ethernet adapter
 - EDIMAX EU-4307 USB-C ethernet adapter
 - Dell DBQBCBC064 USB-C ethernet adapter

Performance measurements were done with:
 - iperf3 between two linux boxes
 - http://openspeedtest.com/ instance running on local test machine

Insights from tests results:
 - All except one from third party usb adapters were not affected by
   increased buffer size to their advertised dwNtbOutMaxSize and
   dwNtbInMaxSize.
   Devices were generally reaching 912-940Mbps both download and upload.

   Only EDIMAX adapter experienced decreased download size from
   929Mbps to 827Mbps with iper3, with openspeedtest decrease was from
   968Mbps to 886Mbps.

 - DisplayLink DL-3xxx family devices experienced performance increase
   with iperf3 download from 300Mbps to 870Mbps and
   upload from 782Mbps to 844Mbps.
   With openspeedtest download increased from 556Mbps to 873Mbps
   and upload from 727Mbps to 973Mbps

 - DiplayLink DL-6xxx family devices are not affected by
   increased buffer size.

Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

 v4: Added Acked-by from link
    https://lore.kernel.org/netdev/YtAKEyplVDC85EKV@kroah.com/#t

 Greg, Hopefully this is what you meant about missing "Reviewed-by".
 Many thanks for help!

 include/linux/usb/cdc_ncm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index f7cb3ddce7fb..2d207cb4837d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -53,8 +53,8 @@
 #define USB_CDC_NCM_NDP32_LENGTH_MIN		0x20
 
 /* Maximum NTB length */
-#define	CDC_NCM_NTB_MAX_SIZE_TX			32768	/* bytes */
-#define	CDC_NCM_NTB_MAX_SIZE_RX			32768	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_TX			65536	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_RX			65536	/* bytes */
 
 /* Initial NTB length */
 #define	CDC_NCM_NTB_DEF_SIZE_TX			16384	/* bytes */
-- 
2.36.1

Re: [PATCH v4 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb

[Greg KH]

On Tue, Jul 19, 2022 at 08:24:52AM +0200, Łukasz Spintzyk wrote:
> DisplayLink ethernet devices require NTB buffers larger then 32kb
> in order to run with highest performance.
> 
> This patch is changing upper limit of the rx and tx buffers.
> Those buffers are initialized with CDC_NCM_NTB_DEF_SIZE_RX and
> CDC_NCM_NTB_DEF_SIZE_TX which is 16kb so by default no device is
> affected by increased limit.
> 
> Rx and tx buffer is increased under two conditions:
>  - Device need to advertise that it supports higher buffer size in
>    dwNtbMaxInMaxSize and dwNtbMaxOutMaxSize.
>  - cdc_ncm/rx_max and cdc_ncm/tx_max driver parameters must be adjusted
>    with udev rule or ethtool.
> 
> Summary of testing and performance results:
> Tests were performed on following devices:
>  - DisplayLink DL-3xxx family device
>  - DisplayLink DL-6xxx family device
>  - ASUS USB-C2500 2.5G USB3 ethernet adapter
>  - Plugable USB3 1G USB3 ethernet adapter
>  - EDIMAX EU-4307 USB-C ethernet adapter
>  - Dell DBQBCBC064 USB-C ethernet adapter
> 
> Performance measurements were done with:
>  - iperf3 between two linux boxes
>  - http://openspeedtest.com/ instance running on local test machine
> 
> Insights from tests results:
>  - All except one from third party usb adapters were not affected by
>    increased buffer size to their advertised dwNtbOutMaxSize and
>    dwNtbInMaxSize.
>    Devices were generally reaching 912-940Mbps both download and upload.
> 
>    Only EDIMAX adapter experienced decreased download size from
>    929Mbps to 827Mbps with iper3, with openspeedtest decrease was from
>    968Mbps to 886Mbps.
> 
>  - DisplayLink DL-3xxx family devices experienced performance increase
>    with iperf3 download from 300Mbps to 870Mbps and
>    upload from 782Mbps to 844Mbps.
>    With openspeedtest download increased from 556Mbps to 873Mbps
>    and upload from 727Mbps to 973Mbps
> 
>  - DiplayLink DL-6xxx family devices are not affected by
>    increased buffer size.
> 
> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 
>  v4: Added Acked-by from link
>     https://lore.kernel.org/netdev/YtAKEyplVDC85EKV@kroah.com/#t
> 
>  Greg, Hopefully this is what you meant about missing "Reviewed-by".

Yes, but you forgot to list what changed from v1 and v2 here :(

thanks,

greg k-h

Re: [PATCH v4 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[Greg KH]

On Tue, Jul 19, 2022 at 08:24:51AM +0200, Łukasz Spintzyk wrote:
> From: Dominik Czerwik <dominik.czerwik@synaptics.com>
> 
> This improves performance and stability of
> DL-3xxx/DL-5xxx/DL-6xxx device series.
> 
> Specifically prevents device from temporary network dropouts when
> playing video from the web and network traffic going through is high.
> 
> Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> ---
>  drivers/net/usb/cdc_ncm.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index d55f59ce4a31..af84ac0d65c9 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -1904,6 +1904,19 @@ static const struct driver_info cdc_ncm_info = {
>  	.set_rx_mode = usbnet_cdc_update_filter,
>  };
>  
> +/* Same as cdc_ncm_info, but with FLAG_SEND_ZLP  */
> +static const struct driver_info cdc_ncm_zlp_info = {
> +	.description = "CDC NCM (SEND ZLP)",
> +	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
> +			| FLAG_LINK_INTR | FLAG_ETHER | FLAG_SEND_ZLP,
> +	.bind = cdc_ncm_bind,
> +	.unbind = cdc_ncm_unbind,
> +	.manage_power = usbnet_manage_power,
> +	.status = cdc_ncm_status,
> +	.rx_fixup = cdc_ncm_rx_fixup,
> +	.tx_fixup = cdc_ncm_tx_fixup,
> +};
> +
>  /* Same as cdc_ncm_info, but with FLAG_WWAN */
>  static const struct driver_info wwan_info = {
>  	.description = "Mobile Broadband Network Device",
> @@ -2010,6 +2023,16 @@ static const struct usb_device_id cdc_devs[] = {
>  	  .driver_info = (unsigned long)&wwan_info,
>  	},
>  
> +	/* DisplayLink docking stations */
> +	{ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO
> +		| USB_DEVICE_ID_MATCH_VENDOR,
> +	  .idVendor = 0x17e9,
> +	  .bInterfaceClass = USB_CLASS_COMM,
> +	  .bInterfaceSubClass = USB_CDC_SUBCLASS_NCM,
> +	  .bInterfaceProtocol = USB_CDC_PROTO_NONE,
> +	  .driver_info = (unsigned long)&cdc_ncm_zlp_info,
> +	},
> +
>  	/* Generic CDC-NCM devices */
>  	{ USB_INTERFACE_INFO(USB_CLASS_COMM,
>  		USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
> -- 
> 2.36.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/SubmittingPatches for what needs to be done
  here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH v3 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[Greg KH]

On Mon, Jul 18, 2022 at 02:36:17PM +0200, Łukasz Spintzyk wrote:
> From: Dominik Czerwik <dominik.czerwik@synaptics.com>
> 
> This improves performance and stability of
> DL-3xxx/DL-5xxx/DL-6xxx device series.
> 
> Specifically prevents device from temporary network dropouts when
> playing video from the web and network traffic going through is high.
> 
> Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> ---
> 
> v3: We have decided to remove that copyright as this is not really 
>     necessary.

Thank you for working with your lawyers on this, hopefully this is not
confusing anymore.

greg k-h

Re: [PATCH v2 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[Oliver Neukum]

On 14.07.22 14:02, Łukasz Spintzyk wrote:
> From: Dominik Czerwik <dominik.czerwik@synaptics.com>
> 
> This improves performance and stability of
> DL-3xxx/DL-5xxx/DL-6xxx device series.
>
Hi,

may I request that you send an additional patch changing the description
of cdc_ncm_info, so that is clear that it does not send ZLP, in
contrast to the new option?

	Regards
		Oliver

[PATCH v5 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[Łukasz Spintzyk]

From: Dominik Czerwik <dominik.czerwik@synaptics.com>

This improves performance and stability of
DL-3xxx/DL-5xxx/DL-6xxx device series.

Specifically prevents device from temporary network dropouts when
playing video from the web and network traffic going through is high.

Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
---
v2: Added Sign-off-by: Bernice Chen as company lawyer.
v3:
  - Remove copyright as this is not really necessary.
  - Removed Sign-off-by: Bernice Chen
v4: No new changes, just resubmit with changed [PATCH v4 2/2]
v5: As asked by Oliver Neukum.
    Mark in original cdc_ncm_info description that it does not send ZLP

 drivers/net/usb/cdc_ncm.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d55f59ce4a31..8d5cbda33f66 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1892,7 +1892,7 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 }
 
 static const struct driver_info cdc_ncm_info = {
-	.description = "CDC NCM",
+	.description = "CDC NCM (NO ZLP)",
 	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
 			| FLAG_LINK_INTR | FLAG_ETHER,
 	.bind = cdc_ncm_bind,
@@ -1904,6 +1904,19 @@ static const struct driver_info cdc_ncm_info = {
 	.set_rx_mode = usbnet_cdc_update_filter,
 };
 
+/* Same as cdc_ncm_info, but with FLAG_SEND_ZLP  */
+static const struct driver_info cdc_ncm_zlp_info = {
+	.description = "CDC NCM (SEND ZLP)",
+	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+			| FLAG_LINK_INTR | FLAG_ETHER | FLAG_SEND_ZLP,
+	.bind = cdc_ncm_bind,
+	.unbind = cdc_ncm_unbind,
+	.manage_power = usbnet_manage_power,
+	.status = cdc_ncm_status,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
 /* Same as cdc_ncm_info, but with FLAG_WWAN */
 static const struct driver_info wwan_info = {
 	.description = "Mobile Broadband Network Device",
@@ -2010,6 +2023,16 @@ static const struct usb_device_id cdc_devs[] = {
 	  .driver_info = (unsigned long)&wwan_info,
 	},
 
+	/* DisplayLink docking stations */
+	{ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO
+		| USB_DEVICE_ID_MATCH_VENDOR,
+	  .idVendor = 0x17e9,
+	  .bInterfaceClass = USB_CLASS_COMM,
+	  .bInterfaceSubClass = USB_CDC_SUBCLASS_NCM,
+	  .bInterfaceProtocol = USB_CDC_PROTO_NONE,
+	  .driver_info = (unsigned long)&cdc_ncm_zlp_info,
+	},
+
 	/* Generic CDC-NCM devices */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM,
 		USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
-- 
2.36.1

[PATCH v5 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb

[Łukasz Spintzyk]

DisplayLink ethernet devices require NTB buffers larger then 32kb
in order to run with highest performance.

This patch is changing upper limit of the rx and tx buffers.
Those buffers are initialized with CDC_NCM_NTB_DEF_SIZE_RX and
CDC_NCM_NTB_DEF_SIZE_TX which is 16kb so by default no device is
affected by increased limit.

Rx and tx buffer is increased under two conditions:
 - Device need to advertise that it supports higher buffer size in
   dwNtbMaxInMaxSize and dwNtbMaxOutMaxSize.
 - cdc_ncm/rx_max and cdc_ncm/tx_max driver parameters must be adjusted
   with udev rule or ethtool.

Summary of testing and performance results:
Tests were performed on following devices:
 - DisplayLink DL-3xxx family device
 - DisplayLink DL-6xxx family device
 - ASUS USB-C2500 2.5G USB3 ethernet adapter
 - Plugable USB3 1G USB3 ethernet adapter
 - EDIMAX EU-4307 USB-C ethernet adapter
 - Dell DBQBCBC064 USB-C ethernet adapter

Performance measurements were done with:
 - iperf3 between two linux boxes
 - http://openspeedtest.com/ instance running on local test machine

Insights from tests results:
 - All except one from third party usb adapters were not affected by
   increased buffer size to their advertised dwNtbOutMaxSize and
   dwNtbInMaxSize.
   Devices were generally reaching 912-940Mbps both download and upload.

   Only EDIMAX adapter experienced decreased download size from
   929Mbps to 827Mbps with iper3, with openspeedtest decrease was from
   968Mbps to 886Mbps.

 - DisplayLink DL-3xxx family devices experienced performance increase
   with iperf3 download from 300Mbps to 870Mbps and
   upload from 782Mbps to 844Mbps.
   With openspeedtest download increased from 556Mbps to 873Mbps
   and upload from 727Mbps to 973Mbps

 - DiplayLink DL-6xxx family devices are not affected by
   increased buffer size.

Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2:
  - Information how to change tx,rx buffer size
  - Added performance tests results to the commit description.
v3: No new changes to v2 2/2.
    It is just rebase on top of changed [PATCH v3 1/2]
v4: Added Acked-by from link
    https://lore.kernel.org/netdev/YtAKEyplVDC85EKV@kroah.com/#t
v5: No new changes to v4 2/2.
    It is just rebase on top of changed [PATCH v5 1/2]

 include/linux/usb/cdc_ncm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index f7cb3ddce7fb..2d207cb4837d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -53,8 +53,8 @@
 #define USB_CDC_NCM_NDP32_LENGTH_MIN		0x20
 
 /* Maximum NTB length */
-#define	CDC_NCM_NTB_MAX_SIZE_TX			32768	/* bytes */
-#define	CDC_NCM_NTB_MAX_SIZE_RX			32768	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_TX			65536	/* bytes */
+#define	CDC_NCM_NTB_MAX_SIZE_RX			65536	/* bytes */
 
 /* Initial NTB length */
 #define	CDC_NCM_NTB_DEF_SIZE_TX			16384	/* bytes */
-- 
2.36.1

Re: [PATCH v5 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 20 Jul 2022 08:05:17 +0200 you wrote:
> From: Dominik Czerwik <dominik.czerwik@synaptics.com>
> 
> This improves performance and stability of
> DL-3xxx/DL-5xxx/DL-6xxx device series.
> 
> Specifically prevents device from temporary network dropouts when
> playing video from the web and network traffic going through is high.
> 
> [...]

Here is the summary with links:
  - [v5,1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices
    https://git.kernel.org/netdev/net-next/c/266c0190aee3
  - [v5,2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb
    https://git.kernel.org/netdev/net-next/c/5588d6280270

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html