Re: [patch] urndis: uncomment watchdog

[Gerhard Roth]

On 12/8/21 15:13, Mikhail wrote:

On Wed, Dec 08, 2021 at 02:43:04PM +0100, Gerhard Roth wrote:

Well, the RNDIS device doesn't respond to REMOTE_NDIS_KEEPALIVE_MSG
messages anymore, but now you hope that it'll still process the
REMOTE_NDIS_RESET_MSG we are sending? Sounds like wishful thinking.
I'd say a usbd_reset_port() might be more effective.

BTW: I was wrong about the 5 seconds. In fact its 10 seconds since the
same timeout applies to the reset message.

I think if the device don't ack the keepalive message the driver will
just output an error to the log and return, there should be no second 5
sec timeout:

  748         rval = urndis_ctrl_send(sc, keep, sizeof(*keep));
  749         free(keep, M_TEMP, sizeof *keep);
  750
  751         if (rval != RNDIS_STATUS_SUCCESS) {
  752                 printf("%s: keepalive failed\n", DEVNAME(sc));
  753                 return rval;
  754         }
  755
  756         if ((hdr = urndis_ctrl_recv(sc)) == NULL) {
  757                 printf("%s: unable to get keepalive response\n", 
DEVNAME(sc));
  758                 return RNDIS_STATUS_FAILURE;
  759         }

As you see it calls urndis_ctrl_recv() which in turn calls
urndis_ctrl_msg() which in turn calls usbd_do_request().

usbd_do_request() calls usbd_do_request_flags() with a timeout
of USBD_DEFAULT_TIMEOUT (which is 5 seconds). And
usbd_do_request_flags() sets up an xfer with the USBD_SYNCHRONOUS
flag. Hence usbd_transfer() will wait for the completion up to
USBD_DEFAULT_TIMEOUT seconds.

It may be that the transfer fails with USBD_IOERROR. In that case
the I/O completes faster.

I don't want to fight for this diff, if you think that it's too naive to

expect proper reset from unresponsive device - that's fine, I'm ready to
drop the diff, but who knows how those devices are engineered and trade
of of not being able to run other watchdogs comparing to possible
network recovery does look reasonable to me.

I don't blame the idea of revitializing urndis_watchdog(). But that
code has been disabled for more than 10 years. And its quite different
for what all the other watchdog routines of USB network interface
drivers do. Maybe the code needs rethinking.

smime.p7s
Description: S/MIME cryptographic signature