netdev - Re: RFC: [PATCH 3/3] usb: cdc_ncm: MirrorLink booster for N60x,70x

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]

Date:	Sat, 09 Mar 2013 12:22:42 +0100
From:	Bjørn Mork <bjorn@...k.no>
To:	Loic Domaigne <loic.domaigne@...glemail.com>
Cc:	netdev@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: RFC: [PATCH 3/3] usb: cdc_ncm: MirrorLink booster for N60x,70x

Loic Domaigne <loic.domaigne@...glemail.com> writes:

> This patch allows to boost the performance if used for MirrorLink(TM). It
> is derived from a patch provided by Nokia-LC/Berlin. It optimizes the transfer
> speed for the Nokia N60x,70x. which requires special settings (jumbo frame, 
> no batching)

This could probably be reworked to a real device specific quirk if
necessary.  But it will need some refinement and justification.

> +			/* The Nokia N60x,70x (productId 0x1419) needs:
> +			 * - Jumbo Frame (MTU 8kB)
> +			 * - Disable TX batching to improve latency
> +			 */
> +			if (dev != NULL)
> +				/* called during NCM bind */
> +				dev->hard_mtu = 8192+ETH_HLEN;
> +			else {
> +				/* called during NCM setup */
> +				pr_info(KBUILD_MODNAME ":  jambit MirrorLink booster for "
> +					"Nokia 60x,70x family");
> +				ctx->tx_max_datagrams  = 1;
> +				ctx->max_datagram_size = 8192+ETH_HLEN;
> +			}

This looks really strange to me at first glance.  An important part of
the NCM protocol is the ability for the host to request these values
from the device, and the only real point of using NCM in the first place
is the bundling.  If the device has higher performance without bundling,
then why the h is it using NCM?

And if it really wants jumbo datagrams, then why doesn't it say so when
the driver ask?

Are the above values based on some perceived performance, or are there
actual measurements behind this?   Yes, sure the latency will drop with
no bundling, but that is sort of the trade-off you chose when you chose
NCM...

> @@ -1269,5 +1320,7 @@ module_exit(cdc_ncm_exit);
>  #endif
>  
>  MODULE_AUTHOR("Hans Petter Selasky");
> +MODULE_AUTHOR("Loic Domaigne");
>  MODULE_DESCRIPTION("USB CDC NCM host driver");
> +MODULE_DESCRIPTION("MirrorLink Booster by jambit GmbH");
>  MODULE_LICENSE("Dual BSD/GPL");

I do not think this is appropriate at all for a device specific bug
workaround...

Bjørn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html