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

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

[Christian Krause]

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

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

[Michael S. Tsirkin]

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?
If no, let's just make it 64K? IIUC that's a maximum
length for control transfers as length is a 16 bit field.


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

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

[Christian Krause]

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

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

[Aurelien Jarno]

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.

Thanks, applied.

 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net