On Wed, Aug 18, 2021 at 04:12:03PM +0000, Christian Ludwig wrote:
> Hi,
>
> when plugging and unplugging an ure(4) adapter continuously, I came
> across the following error with URE_DEBUG set:
>
> ure0 at uhub0 port 21 configuration 1 interface 0 "Realtek USB 10/100/1000
> LAN" rev 3.00/30.00 addr 4
> ure0: RTL8153 (0x5c10)ure_ctl: error 15
> ure_ctl: error 15
> ure_ctl: error 15
>
> It waits for a timeout over and over again. The problem is that the
> device has been unplugged, but the attach handler still issues USB
> requests via ure_ctl(). While there is an early abort check with
> usbd_is_dying() in that function, it does not work during attach. The
> reason is that the attach handler runs from the USB exploration task.
> But that task also handles device detach. So while the hardware port
> change event has been noticed, it is up to the USB exploration task to
> actually detach (and deactivate) the device. But it is still running
> the ure(4) attach handler, which now happily waits for each USB request
> to time out. And the ure(4) attach handler has a bunch of loops calling
> ure_ctl(), which takes a really long time to complete altogether when
> it waits for a timeout for each of them. During that time no other USB
> device will get attached/detached either.
>
> The following diff helps with debugging. Plug a ure(4) device and
> unplug it within 10 seconds.
>
> --- 8< ---
> diff --git a/sys/dev/usb/if_ure.c b/sys/dev/usb/if_ure.c
> index ea73db00954..93b76b421b3 100644
> --- a/sys/dev/usb/if_ure.c
> +++ b/sys/dev/usb/if_ure.c
> @@ -1702,6 +1704,8 @@ ure_attach(struct device *parent, struct device *self,
> void *aux)
> break;
> }
>
> + usbd_delay_ms(sc->ure_udev, 10000);
> +
> if (sc->ure_flags & URE_FLAG_8152)
> ure_rtl8152_init(sc);
> else if (sc->ure_flags & (URE_FLAG_8153B | URE_FLAG_8156))
> --- 8< ---
>
> The problem is that most of the callers of ure_ctl() do not check the
> error return code, especially during attach. One solution could be to
> propagate the error and handle it during attach. OTOH other USB device
> drivers have the same problem. So one might argue that the USB stack is
> at fault then. If we wanted to fix it in the USB stack, we would need
> to decouple how port change events are handled. Maybe we could get away
> with sneaking it into usbd_do_request(). But all of that looks rather
> fragile and is well beyond me.
>
> The easiest solution is to deactivate the device proactively in
> ure_ctl() when we see that error there. Linux's rtl8152 does something
> similar and checks if the device is unplugged [1][2].
>
> [1]
> https://elixir.bootlin.com/linux/v5.14-rc5/source/drivers/net/usb/r8152.c#L1249
> [2]
> https://elixir.bootlin.com/linux/v5.14-rc5/source/drivers/net/usb/r8152.c#L1293
>
> Thanks to kevlo@ for helping me debug this.
Committed. Thanks for spending the time in debugging the issue.
>
> So long,
>
>
> - Christian
Kevin
