netdev - RE: [PATCH 1/2] usbnet: changes for upcoming cdc

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] [day] [month] [year] [list]

Message-ID: <2AC7D4AD8BA1C640B4C60C61C8E520153BA8799667@EXDCVYMBSTM006.EQ1STM.local>
Date:	Wed, 24 Nov 2010 16:14:44 +0100
From:	Alexey ORISHKO <alexey.orishko@...ricsson.com>
To:	David Brownell <david-b@...bell.net>
Cc:	"gregkh@...e.de" <gregkh@...e.de>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"oliver@...kum.org" <oliver@...kum.org>,
	"yauheni.kaliuta@...ia.com" <yauheni.kaliuta@...ia.com>,
	"balbi@...com" <balbi@...com>,
	Sjur BRENDELAND <sjur.brandeland@...ricsson.com>,
	Alexey ORISHKO <alexey.orishko@...ricsson.com>
Subject: RE: [PATCH 1/2] usbnet: changes for upcoming cdc_ncm driver

Hi David,

> > -		skb_queue_tail (&dev->done, skb);
> > +	if (skb->len) {
> > +		/* all data was already cloned inside NCM driver */
> 
> Fix this comment.  NCM isn't the only framing policy which un-batches
> RX packets ... RNDIS has done so for a number of years already, and
> more recently EEM needs it too ... plus at least one hardware driver.

Will do.

> Also, check pending patches, since I seem to recall one that supports
> some hardware (SMSC?) that batches, and needed to update the calling
> convention you're using here (i.e. the original one).

Patch was based on linux-next git and I don't see any difference from
net-next. Any particular git you'd like me to look at pending patches?

> 
> 
> > +		if (dev->driver_info->flags & FLAG_MULTI_PACKET)
> 
> except ... you documented this flag as affecting TX paths not RX...

This flag carries two purposes:
- help to correct statistic (Tx/Rx counting)
- handle NCM feature to avoid sending short packets in certain
conditions: cdc_ncm driver has to send short packet (or ZLP) only
if data size is not equal to negotiated data size (dwNtbOutMaxSize).

The other option (instead of dedicated flag) would be to add
additional field in the usbnet context structure, which will keep
dwNtbOutMaxSize value (it still would be better to use
dedicated flag in addition to the value). Thus usbnet would possess
the knowledge when ZLP or short packet should be skipped.
By doing this approach we will eliminate a need to generate short
packet inside the driver and use generic code in usbnet.

Could you comment, what would be the preferable way to get it in the
next version of this patch?

Regards,
Alexey

--
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