Re: Handling incoming ZLP in cdc-wdm

[Robert Hodaszi <robert.hodaszi@digi.com>]

>
> Hi,
>
> I see what you mean, but I am afraid this introduces an error case.
>
> Suppose we have the following scenario with your patch applied:
>
> wdm_in_callback() gets a ZLP -> we schedule service_interrupt_work()
> service_interrupt_work() calls service_outstanding_interrupt()
> service_outstanding_interrupt() decrements resp_count to zero
> service_interrupt_work() sets WDM_READ and wakes the waiting task
>
> wdm_read() is woken up and finds an empty buffer
> If reslength == 0 all is well and it returns -EAGAIN to user space
> If reslength != 0 it will return EOF
>
> We must not return EOF.
> Hence the longer I think about it it seems to me that
>
> 1. reslength is necessary
> 2. You must set reslength to 0 whenever you get a ZLP
>
>        Regards
>                Oliver
>
That's a good point! So that's the case, when we receive only a single 
ZLP, and nothing else.

But we cannot return with -EAGAIN. If we do that, we're back to sqrt(1), 
and get stuck again.

So what about modifying the service_interrupt_work to no simply set 
WDM_READ if resp_count is 0, but instead to check if there's any real 
message in the buffer, to not confuse consumers. Something like this:

    diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
    index 37873acd18f4..9037379f3603 100644
    --- a/drivers/usb/class/cdc-wdm.c
    +++ b/drivers/usb/class/cdc-wdm.c
    @@ -1010,7 +1010,7 @@ static void service_interrupt_work(struct
    work_struct *work)

             spin_lock_irq(&desc->iuspin);
             service_outstanding_interrupt(desc);
    -       if (!desc->resp_count) {
    +       if (!desc->resp_count && (desc->length || desc->rerr)) {
                     set_bit(WDM_READ, &desc->flags);
                     wake_up(&desc->wait);
             }


Best regards,
Robert