netdev - Re: [PATCH] qmi_wwan: support modify usbnet's rx_urb

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: <5a3c7567-da1a-7676-2516-de7681652643@gmail.com>
Date:   Mon, 3 Aug 2020 11:38:47 +0300
From:   Sergei Shtylyov <sergei.shtylyov@...il.com>
To:     yzc666@...ease.com, bjorn@...k.no
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org,
        linux-usb@...r.kernel.org, carl <carl.yin@...ctel.com>
Subject: Re: [PATCH] qmi_wwan: support modify usbnet's rx_urb_size

On 03.08.2020 9:51, yzc666@...ease.com wrote:
> From: carl <carl.yin@...ctel.com>

    Prefrrably a full name, matching that one in the signoff tag.

> 
>      When QMUX enabled, the 'dl-datagram-max-size' can be 4KB/16KB/31KB depend on QUALCOMM's chipsets.

    No indentation here please, start at column 1 and respect the length limit 
of 75 columns as script/checkpatch.pl says...

>      User can set 'dl-datagram-max-size' by 'QMI_WDA_SET_DATA_FORMAT'.
>      The usbnet's rx_urb_size must lager than or equal to the 'dl-datagram-max-size'.
>      This patch allow user to modify usbnet's rx_urb_size by next command.
> 
> 		echo 4096 > /sys/class/net/wwan0/qmi/rx_urb_size
> 
> 		Next commnds show how to set and query 'dl-datagram-max-size' by qmicli
> 		# qmicli -d /dev/cdc-wdm1 --wda-set-data-format="link-layer-protocol=raw-ip, ul-protocol=qmap,
> 				dl-protocol=qmap, dl-max-datagrams=32, dl-datagram-max-size=31744, ep-type=hsusb, ep-iface-number=4"
> 		[/dev/cdc-wdm1] Successfully set data format
> 		                        QoS flow header: no
> 		                    Link layer protocol: 'raw-ip'
> 		       Uplink data aggregation protocol: 'qmap'
> 		     Downlink data aggregation protocol: 'qmap'
> 		                          NDP signature: '0'
> 		Downlink data aggregation max datagrams: '10'
> 		     Downlink data aggregation max size: '4096'
> 
> 	    # qmicli -d /dev/cdc-wdm1 --wda-get-data-format
> 		[/dev/cdc-wdm1] Successfully got data format
> 		                   QoS flow header: no
> 		               Link layer protocol: 'raw-ip'
> 		  Uplink data aggregation protocol: 'qmap'
> 		Downlink data aggregation protocol: 'qmap'
> 		                     NDP signature: '0'
> 		Downlink data aggregation max datagrams: '10'
> 		Downlink data aggregation max size: '4096'
> 
> Signed-off-by: carl <carl.yin@...ctel.com>

    Need your full name.

> ---
>   drivers/net/usb/qmi_wwan.c | 39 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 07c42c0719f5b..8ea57fd99ae43 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -400,6 +400,44 @@ static ssize_t raw_ip_store(struct device *d,  struct device_attribute *attr, co
>   	return ret;
>   }
>   
> +static ssize_t rx_urb_size_show(struct device *d, struct device_attribute *attr, char *buf)
> +{
> +	struct usbnet *dev = netdev_priv(to_net_dev(d));
> +
> +	return sprintf(buf, "%zd\n", dev->rx_urb_size);

   Is dev->rx_urb_size really declared as size_t?

> +}
> +
> +static ssize_t rx_urb_size_store(struct device *d,  struct device_attribute *attr,
> +				 const char *buf, size_t len)
> +{
> +	struct usbnet *dev = netdev_priv(to_net_dev(d));
> +	u32 rx_urb_size;

    ... in this case, sholdn't this variable be also declared as size_t?

> +	int ret;
> +
> +	if (kstrtou32(buf, 0, &rx_urb_size))
> +		return -EINVAL;
> +
> +	/* no change? */
> +	if (rx_urb_size == dev->rx_urb_size)
> +		return len;
> +
> +	if (!rtnl_trylock())
> +		return restart_syscall();
> +
> +	/* we don't want to modify a running netdev */
> +	if (netif_running(dev->net)) {
> +		netdev_err(dev->net, "Cannot change a running device\n");
> +		ret = -EBUSY;
> +		goto err;
> +	}
> +
> +	dev->rx_urb_size = rx_urb_size;
> +	ret = len;
> +err:
> +	rtnl_unlock();
> +	return ret;
> +}
> +
>   static ssize_t add_mux_show(struct device *d, struct device_attribute *attr, char *buf)
>   {
>   	struct net_device *dev = to_net_dev(d);
> @@ -505,6 +543,7 @@ static DEVICE_ATTR_RW(add_mux);
>   static DEVICE_ATTR_RW(del_mux);
>   
>   static struct attribute *qmi_wwan_sysfs_attrs[] = {
> +	&dev_attr_rx_urb_size.attr,
>   	&dev_attr_raw_ip.attr,
>   	&dev_attr_add_mux.attr,
>   	&dev_attr_del_mux.attr,
>