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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cb2d33f6-1b6d-57bf-55b1-59d48cacf492@synaptics.com>
Date:   Fri, 8 Jul 2022 09:40:49 +0200
From:   Lukasz Spintzyk <lukasz.spintzyk@...aptics.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     netdev@...r.kernel.org, linux-usb@...r.kernel.org,
        oliver@...kum.org, kuba@...nel.org, ppd-posix@...aptics.com
Subject: Re: [PATCH 2/2] net/cdc_ncm: Increase NTB max RX/TX values to 64kb

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@...aptics.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