Re: Back-out USB data toggle fix

[Marcus Glocker]

On Sun, Feb 14, 2021 at 04:43:17PM +0000, Stuart Henderson wrote:

> On 2021/02/14 15:22, Marcus Glocker wrote:
> > Unfortunately I'm seeing more and more USB device breakages reported
> > the last few days related to the USB data toggle fix which we did
> > commit 2-3 weeks ago.
> 
> Do you think this could this be implicated in a keyboard with 'stuck'
> keys i.e. keep repeating? I've hit it a few times recently, but didn't
> look that far back. (Nothing amiss in logs when it happens).

I don't think so since the issue also has been reported for some fido(4)
devices.
 
> > +/*
> > + * Workaround for OpenBSD <=6.6-current (as of 201910) bug that loses
> > + * sync of DATA0/DATA1 sequence bit across uhid open/close.
> > + * Send pings until we get a response - early pings with incorrect
> > + * sequence bits will be ignored as duplicate packets by the device.
> > + */
> > +static int
> > +terrible_ping_kludge(struct hid_openbsd *ctx)
> 
> it will need to be put back in chromium too (and port revision will
> need to be bumped)

Right, the chromium diff was included, but I did forget to bump.


Index: www/chromium/Makefile
===================================================================
RCS file: /cvs/ports/www/chromium/Makefile,v
retrieving revision 1.546
diff -u -p -u -p -r1.546 Makefile
--- www/chromium/Makefile       6 Feb 2021 07:33:27 -0000       1.546
+++ www/chromium/Makefile       14 Feb 2021 19:24:08 -0000
@@ -18,6 +18,7 @@ COMMENT=              Chromium browser
 V=                     88.0.4324.150
 
 DISTNAME=              chromium-${V}
+REVISION=              0
 
 DISTFILES+=            chromium-${V}${EXTRACT_SUFX}
 
Index: www/chromium/files/hid_service_fido.cc
===================================================================
RCS file: /cvs/ports/www/chromium/files/hid_service_fido.cc,v
retrieving revision 1.4
diff -u -p -u -p -r1.4 hid_service_fido.cc
--- www/chromium/files/hid_service_fido.cc      6 Feb 2021 05:05:04 -0000       
1.4
+++ www/chromium/files/hid_service_fido.cc      14 Feb 2021 19:24:09 -0000
@@ -11,7 +11,10 @@
 #include <sys/un.h>
 #include <unistd.h>
 
+// TODO: remove once the missing guard in fido.h is fixed upstream.
+extern "C" {
 #include <fido.h>
+}
 
 #include <set>
 #include <string>
@@ -72,6 +75,55 @@ void FinishOpen(std::unique_ptr<ConnectP
                         base::Bind(&CreateConnection, base::Passed(&params)));
 }
 
+bool terrible_ping_kludge(int fd, const std::string &path) {
+  u_char data[256];
+  int i, n;
+  struct pollfd pfd;
+
+  for (i = 0; i < 4; i++) {
+    memset(data, 0, sizeof(data));
+    /* broadcast channel ID */
+    data[1] = 0xff;
+    data[2] = 0xff;
+    data[3] = 0xff;
+    data[4] = 0xff;
+    /* Ping command */
+    data[5] = 0x81;
+    /* One byte ping only, Vasili */
+    data[6] = 0;
+    data[7] = 1;
+    HID_LOG(EVENT) << "send ping " << i << " " << path;
+    if (write(fd, data, 64) == -1) {
+      HID_PLOG(ERROR) << "write " << path;
+      return false;
+    }
+    HID_LOG(EVENT) << "wait reply " << path;
+    memset(&pfd, 0, sizeof(pfd));
+    pfd.fd = fd;
+    pfd.events = POLLIN;
+    if ((n = poll(&pfd, 1, 100)) == -1) {
+      HID_PLOG(EVENT) << "poll " << path;
+      return false;
+    } else if (n == 0) {
+      HID_LOG(EVENT) << "timed out " << path;
+      continue;
+    }
+    if (read(fd, data, 64) == -1) {
+      HID_PLOG(ERROR) << "read " << path;
+      return false;
+    }
+    /*
+     * Ping isn't always supported on the broadcast channel,
+     * so we might get an error, but we don't care - we're
+     * synched now.
+     */
+    HID_LOG(EVENT) << "got reply " << path;
+    return true;
+  }
+  HID_LOG(ERROR) << "no response " << path;
+  return false;
+}
+
 void OpenOnBlockingThread(std::unique_ptr<ConnectParams> params) {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
@@ -85,6 +137,11 @@ void OpenOnBlockingThread(std::unique_pt
   if (!device_file.IsValid()) {
     HID_LOG(EVENT) << "Failed to open '" << device_node << "': "
                    << base::File::ErrorToString(device_file.error_details());
+    task_runner->PostTask(FROM_HERE, 
base::BindOnce(std::move(params->callback), nullptr));
+    return;
+  }
+  if (!terrible_ping_kludge(device_file.GetPlatformFile(), device_node)) {
+    HID_LOG(EVENT) << "Failed to ping " << device_node;
     task_runner->PostTask(FROM_HERE, 
base::BindOnce(std::move(params->callback), nullptr));
     return;
   }