* [PATCH 0/2] DisplayLink USB-ethernet improvements
@ 2022-07-04 7:04 Łukasz Spintzyk
2022-07-04 7:04 ` [PATCH 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices Łukasz Spintzyk
2022-07-04 7:04 ` [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb Łukasz Spintzyk
0 siblings, 2 replies; 8+ messages in thread
From: Łukasz Spintzyk @ 2022-07-04 7:04 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, oliver, kuba, ppd-posix
Hi all,
I am resending that two changes again.
This are two patches to cdc_ncm driver used in our DisplayLink USB docking stations.
They are independent, however both of them are improving performance and stability so it matches Windows experience.
It is improving the experience for users of millions of DisplayLink-based docking stations that are in the wild.
That tweaks of NTB TX/RX results in approximately 4-5x available bandwidth improvement in extreme cases.
Please take a look.
Regards,
Łukasz Spintzyk
Software Developer at Synaptics Inc.
Dominik Czerwik (1):
net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices
Łukasz Spintzyk (1):
net/cdc_ncm: Increase NTB max RX/TX values to 64kb
drivers/net/usb/cdc_ncm.c | 24 ++++++++++++++++++++++++
include/linux/usb/cdc_ncm.h | 4 ++--
2 files changed, 26 insertions(+), 2 deletions(-)
--
2.36.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices
2022-07-04 7:04 [PATCH 0/2] DisplayLink USB-ethernet improvements Łukasz Spintzyk
@ 2022-07-04 7:04 ` Łukasz Spintzyk
2022-07-04 7:25 ` Greg KH
2022-07-04 7:04 ` [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb Łukasz Spintzyk
1 sibling, 1 reply; 8+ messages in thread
From: Łukasz Spintzyk @ 2022-07-04 7:04 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, oliver, kuba, ppd-posix
From: Dominik Czerwik <dominik.czerwik@synaptics.com>
This improves performance and stability of
DL-3xxx/DL-5xxx/DL-6xxx device series.
Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb
2022-07-04 7:04 [PATCH 0/2] DisplayLink USB-ethernet improvements Łukasz Spintzyk
2022-07-04 7:04 ` [PATCH 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices Łukasz Spintzyk
@ 2022-07-04 7:04 ` Łukasz Spintzyk
2022-07-04 7:24 ` Greg KH
1 sibling, 1 reply; 8+ messages in thread
From: Łukasz Spintzyk @ 2022-07-04 7:04 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, oliver, kuba, ppd-posix
DisplayLink ethernet devices require NTB buffers larger then 32kb in order to run with highest performance.
Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb
2022-07-04 7:04 ` [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb Łukasz Spintzyk
@ 2022-07-04 7:24 ` Greg KH
2022-07-08 7:40 ` Lukasz Spintzyk
0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2022-07-04 7:24 UTC (permalink / raw)
To: Łukasz Spintzyk; +Cc: netdev, linux-usb, oliver, kuba, ppd-posix
On Mon, Jul 04, 2022 at 09:04:07AM +0200, Łukasz Spintzyk wrote:
> DisplayLink ethernet devices require NTB buffers larger then 32kb in order to run with highest performance.
>
> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> ---
> 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 */
Does this mess with the throughput of older devices that are not on
displaylink connections?
What devices did you test this on, and what is the actual performance
changes? You offer no real information here at all, and large buffer
sizes does have other downsides, so determining how you tested this is
key.
Also, please wrap your changelogs at 72 columns like git asks you to do.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices
2022-07-04 7:04 ` [PATCH 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices Łukasz Spintzyk
@ 2022-07-04 7:25 ` Greg KH
2022-07-08 7:42 ` Lukasz Spintzyk
0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2022-07-04 7:25 UTC (permalink / raw)
To: Łukasz Spintzyk; +Cc: netdev, linux-usb, oliver, kuba, ppd-posix
On Mon, Jul 04, 2022 at 09:04:06AM +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.
>
> Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> ---
> 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.
As I ask many times for other copyright additions, when making a change
like this, for such a tiny patch, I want to see a lawyer from your
company also sign off on the patch proving that they agree that this
line should be added.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb
2022-07-04 7:24 ` Greg KH
@ 2022-07-08 7:40 ` Lukasz Spintzyk
2022-07-08 12:37 ` Greg KH
0 siblings, 1 reply; 8+ messages in thread
From: Lukasz Spintzyk @ 2022-07-08 7:40 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, linux-usb, oliver, kuba, ppd-posix
On 04/07/2022 09:24, Greg KH wrote:
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On Mon, Jul 04, 2022 at 09:04:07AM +0200, Łukasz Spintzyk wrote:
>> DisplayLink ethernet devices require NTB buffers larger then 32kb in order to run with highest performance.
>>
>> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
>> ---
>> 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 */
>
> Does this mess with the throughput of older devices that are not on
> displaylink connections?
>
> What devices did you test this on, and what is the actual performance
> changes? You offer no real information here at all, and large buffer
> sizes does have other downsides, so determining how you tested this is
> key.
>
> Also, please wrap your changelogs at 72 columns like git asks you to do.
>
> thanks,
>
> greg k-h
Hi Greg,
To my best knowledge that patch should not affect other devices because:
- tx,rx buffers size is initialized to 16kb with
CDC_NCM_NTB_DEF_SIZE_RX, and CDC_NCM_NTB_DEF_SIZE_TX
So all existing devices should not be affected by default.
- In order to change tx and rx buffer max size you need to
additionally modify cdc_ncm/tx_max and cdc_ncm/rx_max parameters. This
can be done with udev rules and ethtool. So if you want to use higher
buffer sizes you need to specially request that.
For DisplayLink devices this will be done with udev rule that will be
installed with other DisplayLink drivers.
- This tx,rx buffer sizes are always capped to dwNtbMaxInMaxSize and
dwNtbMaxOutMaxSize that are advertised by the device itself. So in
theory that values should be acceptable by the device.
Here is summary of my tests I have done on such devices:
- DisplayLink DL-3xxx family device
- DisplayLink DL-6xxx family device
- ASUS USB-C2500 2.5G USB3 eth adapter
- Plugable USB3 1G USB3 adapter
- EDIMAX EU-4307 USB-C adapter
- Dell DBQBCBC064 USB-C adapter
Unfortunately I was not able to find more or older than USB-3 device on
company's shelf.
I was doing measurements with:
- iperf3 between two linux boxes
- http://openspeedtest.com/ instance running on local testing machine
I can provide you with detailed results, but I think they are quite
verbose so I will stay with some high level results:
- All except one from third party usb adapters were not affected by
increased buffer size. (I have forced them to use tx,rx size as big as
their advertised dwNtbOutMaxSize and dwNtbInMaxSize).
They were generally reaching 912 - 940Mbps (download/upload)
Only Edimax adapter experienced decreased download size from 929Mbps
to 827 with iper3. In openspeedtest this was decrease from 968Mbps to
886Mbps
- DisplayLink DL-3xxx family devices experienced increase of performance
Iperf3:
Download from (300Mbps to 870Mbps), also from historical
measurements on other setup this was (90Mbps to 500Mbps)
Upload from 782Mbps to 844Mbps
Openspeedtest:
Download from 556Mbps to 873Mbps
Upload from 727Mbps to 973Mbps
- DiplayLink DL-6xxx family devices are not affected greatly by buffer
size. It is more affected by patch enabling ZLP which prevents device
from temporary network dropouts when playing video from web and network
traffic going through it is high.
thanks
Łukasz Spintzyk
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices
2022-07-04 7:25 ` Greg KH
@ 2022-07-08 7:42 ` Lukasz Spintzyk
0 siblings, 0 replies; 8+ messages in thread
From: Lukasz Spintzyk @ 2022-07-08 7:42 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, linux-usb, oliver, kuba, ppd-posix
On 04/07/2022 09:25, Greg KH wrote:
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On Mon, Jul 04, 2022 at 09:04:06AM +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.
>>
>> Signed-off-by: Dominik Czerwik <dominik.czerwik@synaptics.com>
>> Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
>> ---
>> 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.
>
> As I ask many times for other copyright additions, when making a change
> like this, for such a tiny patch, I want to see a lawyer from your
> company also sign off on the patch proving that they agree that this
> line should be added.
>
> thanks,
>
> greg k-h
Ok, I will contact company's layer and will be back with updated patches
if needed.
thanks
Łukasz Spintzyk
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb
2022-07-08 7:40 ` Lukasz Spintzyk
@ 2022-07-08 12:37 ` Greg KH
0 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2022-07-08 12:37 UTC (permalink / raw)
To: Lukasz Spintzyk; +Cc: netdev, linux-usb, oliver, kuba, ppd-posix
On Fri, Jul 08, 2022 at 09:40:49AM +0200, Lukasz Spintzyk wrote:
> On 04/07/2022 09:24, Greg KH wrote:
> > CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> >
> > On Mon, Jul 04, 2022 at 09:04:07AM +0200, Łukasz Spintzyk wrote:
> > > DisplayLink ethernet devices require NTB buffers larger then 32kb in order to run with highest performance.
> > >
> > > Signed-off-by: Łukasz Spintzyk <lukasz.spintzyk@synaptics.com>
> > > ---
> > > 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 */
> >
> > Does this mess with the throughput of older devices that are not on
> > displaylink connections?
> >
> > What devices did you test this on, and what is the actual performance
> > changes? You offer no real information here at all, and large buffer
> > sizes does have other downsides, so determining how you tested this is
> > key.
> >
> > Also, please wrap your changelogs at 72 columns like git asks you to do.
> >
> > thanks,
> >
> > greg k-h
>
> Hi Greg,
> To my best knowledge that patch should not affect other devices because:
> - tx,rx buffers size is initialized to 16kb with CDC_NCM_NTB_DEF_SIZE_RX,
> and CDC_NCM_NTB_DEF_SIZE_TX
> So all existing devices should not be affected by default.
> - In order to change tx and rx buffer max size you need to additionally
> modify cdc_ncm/tx_max and cdc_ncm/rx_max parameters. This can be done with
> udev rules and ethtool. So if you want to use higher buffer sizes you need
> to specially request that.
> For DisplayLink devices this will be done with udev rule that will be
> installed with other DisplayLink drivers.
> - This tx,rx buffer sizes are always capped to dwNtbMaxInMaxSize and
> dwNtbMaxOutMaxSize that are advertised by the device itself. So in theory
> that values should be acceptable by the device.
>
> Here is summary of my tests I have done on such devices:
> - DisplayLink DL-3xxx family device
> - DisplayLink DL-6xxx family device
> - ASUS USB-C2500 2.5G USB3 eth adapter
> - Plugable USB3 1G USB3 adapter
> - EDIMAX EU-4307 USB-C adapter
> - Dell DBQBCBC064 USB-C adapter
>
> Unfortunately I was not able to find more or older than USB-3 device on
> company's shelf.
>
> I was doing measurements with:
> - iperf3 between two linux boxes
> - http://openspeedtest.com/ instance running on local testing machine
>
> I can provide you with detailed results, but I think they are quite verbose
> so I will stay with some high level results:
>
> - All except one from third party usb adapters were not affected by
> increased buffer size. (I have forced them to use tx,rx size as big as their
> advertised dwNtbOutMaxSize and dwNtbInMaxSize).
> They were generally reaching 912 - 940Mbps (download/upload)
> Only Edimax adapter experienced decreased download size from 929Mbps to
> 827 with iper3. In openspeedtest this was decrease from 968Mbps to 886Mbps
>
> - DisplayLink DL-3xxx family devices experienced increase of performance
> Iperf3:
> Download from (300Mbps to 870Mbps), also from historical measurements
> on other setup this was (90Mbps to 500Mbps)
> Upload from 782Mbps to 844Mbps
> Openspeedtest:
> Download from 556Mbps to 873Mbps
> Upload from 727Mbps to 973Mbps
>
> - DiplayLink DL-6xxx family devices are not affected greatly by buffer size.
> It is more affected by patch enabling ZLP which prevents device from
> temporary network dropouts when playing video from web and network traffic
> going through it is high.
Lots of this information needs to go into the changelog text so that we
understand how you tested this, what you tested this on, and why you are
making this change.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-07-08 12:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-04 7:04 [PATCH 0/2] DisplayLink USB-ethernet improvements Łukasz Spintzyk
2022-07-04 7:04 ` [PATCH 1/2] net/cdc_ncm: Enable ZLP for DisplayLink ethernet devices Łukasz Spintzyk
2022-07-04 7:25 ` Greg KH
2022-07-08 7:42 ` Lukasz Spintzyk
2022-07-04 7:04 ` [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb Łukasz Spintzyk
2022-07-04 7:24 ` Greg KH
2022-07-08 7:40 ` Lukasz Spintzyk
2022-07-08 12:37 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.