[Qemu-devel] Re: [PATCH, RESEND] usb: increase buffer for USB control requests

[Christian Krause <chkr@plauener.de>]

Hello Michael,

On 01/24/2010 10:20 PM, Michael S. Tsirkin wrote:
> On Sun, Jan 24, 2010 at 05:34:52PM +0100, Christian Krause wrote:
>> Resend. The patch was already sent to the list on 2009-12-11. It would
>> be great if it could be reviewed and applied. Thank you very much
>> in advance.
>>
>> The WLAN USB stick ZyXEL NWD271N (0586:3417) uses very large
>> usb control transfers of more than 2048 bytes which won't fit
>> into the buffer of the ctrl_struct. This results in an error message
>> "husb: ctrl buffer too small" and a non-working device.
>> Increasing the buffer size to 8192 seems to be a safe choice.
>>
>> Signed-off-by: Christian Krause <chkr@plauener.de>
> 
> Are there any drawbacks to make\ing the buffer larger?

I've roughly looked into the source code of usb-linux.c and I don't see
an issue if the buffer would be larger:

- if the buffer is used to get data via USBDEVFS_SUBMITURB from the
kernel then the check:

    if (buffer_len > sizeof(s->ctrl.buffer)) {
        fprintf(stderr, "husb: ctrl buffer too small (%u > %zu)\n",
                buffer_len, sizeof(s->ctrl.buffer));
        return USB_RET_STALL;
    }

already ensures that the we only supply a buffer_len which matches the
buffer

- on the other hand, when we copy data out of the buffer into the data
structures from the HC, then there is also a check that we only copy as
much data as the HC requests:

            if (len > p->len)
                len = p->len;
            memcpy(p->data, s->ctrl.buffer + s->ctrl.offset, len);


> If no, let's just make it 64K? IIUC that's a maximum
> length for control transfers as length is a 16 bit field.

I think that's OK since it looks like that the increase of memory would
only be 64k per usb host device.


Best regards,
Christian


>> ---
>>  usb-linux.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/usb-linux.c b/usb-linux.c
>> index 285ac22..d205bd3 100644
>> --- a/usb-linux.c
>> +++ b/usb-linux.c
>> @@ -113,7 +113,7 @@ struct ctrl_struct {
>>      uint16_t offset;
>>      uint8_t  state;
>>      struct   usb_ctrlrequest req;
>> -    uint8_t  buffer[2048];
>> +    uint8_t  buffer[8192];
>>  };
>>  
>>  struct USBAutoFilter {
>> -- 
>> 1.6.2.5
>>
>>
>