linux-kernel - Re: [PATCH] [RFC] introduce cx82310_eth: Conexant CX82310-based ADSL router USB ethernet driver

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: <4C827002.6060808@simon.arlott.org.uk>
Date:	Sat, 04 Sep 2010 17:12:50 +0100
From:	Simon Arlott <simon@...e.lp0.eu>
To:	Ondrej Zary <linux@...nbow-software.org>
CC:	David Brownell <dbrownell@...rs.sourceforge.net>,
	netdev@...r.kernel.org,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [RFC] introduce cx82310_eth: Conexant CX82310-based ADSL
 router USB ethernet driver

On 04/09/10 12:57, Ondrej Zary wrote:
> On Saturday 04 September 2010 00:14:54 Simon Arlott wrote:
>> On 03/09/10 22:17, Ondrej Zary wrote:
>> > +	/* send command packet */
>> > +	ret = usb_bulk_msg(udev, usb_sndbulkpipe(udev, CMD_EP), buf,
>> > +			   CMD_PACKET_SIZE, &actual_len, CMD_TIMEOUT);
>>
>> From your previous lsusb output this is an interrupt endpoint, although
>> usb_bulk_msg will auto-detect the type.
> 
> There's also usb_interrupt_msg() function but it calls usb_bulk_msg() directly 
> so it probably does not matter...
> Otherwise, I already found out that sending interrupt URBs to bulk endpoints 
> is a very bad idea (when programming Nexio support for usbtouchscreen). Alan 
> Stern then created a patch that checks for this.

I know... it broke cxacru when the device has a bulk command endpoint.

>> >
>> > +#define rx_incomplete	(dev->data[0])
>> > +#define rx_remainder	(dev->data[1])
>> > +#define incomplete_data	(dev->data[2])
>>
>> This doesn't make the rest of the code particularly readable.
> 
> I agree. Maybe I should abuse data[0] as a pointer to private data...
> This data[] array does not seem like a good thing. Various usbnet drivers 
> abuse it in various ways. It should probably be removed and replaced by one 
> priv pointer.

There's driver_priv, added in 2008 and used only by rndis_wlan.

>> > +	/* we can send at most 1514 bytes of data (+ 2-byte header) per URB */
>> > +	dev->hard_mtu = CX82310_MTU + dev->net->hard_header_len;
>>
>> Have you tried sending larger packets?
>> With another 8 bytes it would support 802.1Q (VLAN).
> 
> Larger packets seem to be dropped. No surprise as the router firmware does not 
> support VLANs either.

I hadn't considered that it'd need to be able to receive/transmit the
larger packets over the built-in ethernet ports too.

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/