netdev - Re: usbnet transmit path problems

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:	Wed, 11 Sep 2013 23:11:57 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	David Laight <David.Laight@...lab.com>
Cc:	Oliver Neukum <oliver@...kum.org>,
	Network Development <netdev@...r.kernel.org>,
	linux-usb <linux-usb@...r.kernel.org>
Subject: Re: usbnet transmit path problems

On Wed, Sep 11, 2013 at 8:56 PM, David Laight <David.Laight@...lab.com> wrote:
>> > > 2) If 'length % dev->maxpacket == 0' for a multi-fragment packet then
>> > >    the extra byte isn't added correctly (the code probably falls off
>> > >    the end of the scatter-gather list).
>> >
>> > Indeed. Ming Lei, should usbnet handle this in the sg case or better
>> > leave it to the subdriver you introduced this for?
>
> Is the ZLP issue a problem with the host or with the target?

Sorry, what do you mean the ZLP issue here? I understand Oliver
thinks one commit from me may break ZLP handling, are you discussing
this problem? If not, could you explain it in a bit detail?

> If it is a host problem then the necessity comes from the host,
> but the fix needs to be target dependant.
> If it is a common target problem then generic code can apply
> a common fix.

All usbnet device should have sent one ZLP in case the size of
bulk out transfer can be divided by max packet size, but the one
byte transfer might be introduced for avoiding some target problem
(can't deal with zlp well), as said by David, see below discussion:

   http://marc.info/?l=linux-usb&m=127067487604112&w=2

>
> AFICT there are at least 3 fixes:
> 1) Extend the ethernet frame by one byte and hope the receiving
>    system doesn't object to the padding.
>    This is probably the only option if tx_fixup() doesn't
>    add a header.
> 2) Put the ethernet frame length in the header and have the
>    target discard the added pad byte (ax88179_178a.c).
> 3) Add a second zero-length frame in the same USB data block
>    (ax88172a.c).

Why do we need the above 3 fixes? The patch in my last email can
fix the problem which is introduced recently, can't it?

>
> Only the third requires that tx_fixup() append to the packet.
> For the other two actual pad can be added by usbnet.
>
>> IMO, it should be handled by usbnet, could you comment on below patch?
>
> Seems excessive to kmalloc() one byte!

It isn't strange, many usb drivers have to do that(maybe kmalloc
two or three, or four bytes) since the buffer is involved into DMA.

> If you can't assume that the 'dev' structure itself can be dma'd from
> allocate the extra byte in the sg list.

It is better to always obey rule of DMA-API, so don't do that since
one extra kmalloc() per device is needed, even though we can allocate
only one global buffer for this purpose.


Thanks,
--
Ming Lei
--
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