netdev - RE: [PATCH v2 net-next 3/8] net: cdc_ncm: inform usbnet when rx buffers are reduced

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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 30 May 2014 09:18:37 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Bjørn Mork' <bjorn@...k.no>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	Alexey Orishko <alexey.orishko@...il.com>,
	Oliver Neukum <oliver@...kum.org>,
	Enrico Mioso <mrkiko.rs@...il.com>,
	Lars Melin <larsm17@...il.com>, Peter Stuge <peter@...ge.se>,
	Greg Suarez <gsuarez@...thmicro.com>
Subject: RE: [PATCH v2 net-next 3/8] net: cdc_ncm: inform usbnet when rx
 buffers are reduced

From: Bjørn Mork 
> It doesn't matter whether the buffer size goes up or down.  We have to
> keep usbnet and device syncronized to be able to split transfers at the
> correct boundaries. The spec allow skipping short packets when using
> max sized transfers.  If we don't tell usbnet about our new expected rx
> buffer size, then it will merge and/or split NTBs.  The driver does not
> support this, and the result will be lots of framing errors.
> 
> Fix by always reallocating usbnet rx buffers when the rx_max value
> changes.

I'm guessing that the rx_max value is the maximum size of the USB bulk
data 'message' that the device generates?

As such the URB only need to be longer that it.
(Or multiples of the USB packet size, and the driver then merge URB
when generating skbs.)

Since you are now copying the data out of the URB's skb before
passing the ethernet packet upstream, is there ever any real
requirement to use a small rx_max? or ever change rx_max?

	David

> Fixes: 68864abf08f0 ("net: cdc_ncm: support rx_max/tx_max updates when running")
> Signed-off-by: Bjørn Mork <bjorn@...k.no>
> ---
>  drivers/net/usb/cdc_ncm.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index ff5b3a854898..5bce86a0d063 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -215,19 +215,12 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
>  			min, max, val);
>  	}
> 
> -	/* usbnet use these values for sizing rx queues */
> -	dev->rx_urb_size = val;
> -
>  	/* inform device about NTB input size changes */
>  	if (val != ctx->rx_max) {
>  		__le32 dwNtbInMaxSize = cpu_to_le32(val);
> 
>  		dev_info(&dev->intf->dev, "setting rx_max = %u\n", val);
> 
> -		/* need to unlink rx urbs before increasing buffer size */
> -		if (netif_running(dev->net) && dev->rx_urb_size > ctx->rx_max)
> -			usbnet_unlink_rx_urbs(dev);
> -
>  		/* tell device to use new size */
>  		if (usbnet_write_cmd(dev, USB_CDC_SET_NTB_INPUT_SIZE,
>  				     USB_TYPE_CLASS | USB_DIR_OUT
> @@ -238,6 +231,13 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
>  			ctx->rx_max = val;
>  	}
> 
> +	/* usbnet use these values for sizing rx queues */
> +	if (dev->rx_urb_size != ctx->rx_max) {
> +		dev->rx_urb_size = ctx->rx_max;
> +		if (netif_running(dev->net))
> +			usbnet_unlink_rx_urbs(dev);
> +	}
> +
>  	/* clamp new_tx to sane values */
>  	min = ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct usb_cdc_ncm_nth16);
>  	max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize));
> --
> 2.0.0.rc4