usbus: Add support for endpoint halt condition

[bergzand]

Contribution description

This PR adds support for the endpoint halt condition on the USBUS side.

Instead of directly stalling an endpoint, handlers should enable the halt condition on an usbus endpoint to signal error condition. This can then be cleared via a CLEAR FEATURE request from the host.

This PR also extends support for the GET STATUS requests to support endpoints and interfaces as recipient. It also adds the SET and CLEAR FEATURE requests for the endpoints with support to set and clear the halt condition on an endpoint.

Testing procedure

The feature can be shown when adding the following patch to the CDC_ECM code:

diff --git a/sys/usb/usbus/cdc/ecm/cdc_ecm.c b/sys/usb/usbus/cdc/ecm/cdc_ecm.c
index f580209e03..3d87a67087 100644
--- a/sys/usb/usbus/cdc/ecm/cdc_ecm.c
+++ b/sys/usb/usbus/cdc/ecm/cdc_ecm.c
@@ -31,7 +31,7 @@
 
 #include <string.h>
 
-#define ENABLE_DEBUG 0
+#define ENABLE_DEBUG 1
 #include "debug.h"
 
 static void _event_handler(usbus_t *usbus, usbus_handler_t *handler,
@@ -335,7 +335,12 @@ static void _transfer_handler(usbus_t *usbus, usbus_handler_t *handler,
         size_t len = 0;
         usbdev_ep_get(ep, USBOPT_EP_AVAILABLE, &len, sizeof(size_t));
         _store_frame_chunk(cdcecm);
-        if (len == USBUS_CDCECM_EP_DATA_SIZE) {
+        DEBUG("Length: %u\n", cdcecm->len);
+        if (cdcecm->len > 1000) {
+            usbus_endpoint_halt(cdcecm->ep_out);
+            _handle_rx_flush(cdcecm);
+        }
+        else if (len == USBUS_CDCECM_EP_DATA_SIZE) {
             usbdev_ep_ready(ep, 0);
         }
     }
diff --git a/sys/usb/usbus/usbus.c b/sys/usb/usbus/usbus.c
index 826966c929..1bed707e2d 100644
--- a/sys/usb/usbus/usbus.c
+++ b/sys/usb/usbus/usbus.c
@@ -40,7 +40,7 @@
 #include <string.h>
 #include <errno.h>
 
-#define ENABLE_DEBUG             0
+#define ENABLE_DEBUG             1
 #include "debug.h"
 
 #define _USBUS_MSG_QUEUE_SIZE    (16)

With this patch, the CDC ECM code will trigger the halt condition after receiving more than 1000 bytes for a single network packet. For example with a ping 2001:db8::2 -s 1200 (assuming 2001:db8::2 is the RIOT device). On a nRF52840dk this looks like the following:

nRF52840dk

2021-10-31 21:24:26,271 # usbus: starting thread 3
2021-10-31 21:24:26,275 # usbus: Adding string descriptor number 1 for: "USB config"
2021-10-31 21:24:26,281 # usbus: Adding string descriptor number 2 for: "USB device"
2021-10-31 21:24:26,286 # usbus: Adding string descriptor number 3 for: "RIOT-os.org"
2021-10-31 21:24:26,292 # usbus: Adding string descriptor number 4 for: "AC1B17D1BE432280"
2021-10-31 21:24:26,294 # CDC ECM: initialization
2021-10-31 21:24:26,299 # usbus: Adding string descriptor number 5 for: "AAEC5F69468B"
2021-10-31 21:24:26,305 # NETOPT_RX_END_IRQ not implemented byusbus: USB suspend detected
2021-10-31 21:24:26,312 # main(): This is RIOT! (Version: 2022.01-devel-281-g7da001-pr/usbus/halt_endpoint)
2021-10-31 21:24:26,316 # Test application for the USBUS CDC ECM interface
2021-10-31 21:24:26,317 # 
2021-10-31 21:24:26,321 # This test pulls in parts of the GNRC network stack, use the
2021-10-31 21:24:26,327 # provided shell commands (i.e. ifconfig, ping6) to interact with
2021-10-31 21:24:26,330 # the CDC ECM based network interface.
2021-10-31 21:24:26,330 # 
2021-10-31 21:24:26,332 # Starting the shell now...
> 2021-10-31 21:24:26,481 # CDC ECM: Reset
2021-10-31 21:24:26,545 # CDC ECM: Reset
2021-10-31 21:24:26,631 # CDC ECM: Request: 0xb
2021-10-31 21:24:26,635 # CDC ECM: Changing active interface to alt 1
2021-10-31 21:24:26,638 # CDC ECM: sending link up indication
2021-10-31 21:24:26,645 # CDC ECM: Request: 0x43
2021-10-31 21:24:26,649 # CDC ECM: Not modifying filter to 0xc
2021-10-31 21:24:29,889 # CDC ECM: Request: 0x43
2021-10-31 21:24:29,893 # CDC ECM: Not modifying filter to 0xc
2021-10-31 21:24:29,901 # CDC ECM: Request: 0x43
2021-10-31 21:24:29,904 # CDC ECM: Not modifying filter to 0xe
2021-10-31 21:24:29,907 # CDC ECM: sending link speed indication
> ifconfig 4 add 2001:db8::2/64
2021-10-31 21:24:35,296 # ifconfig 4 add 2001:db8::2/64
2021-10-31 21:24:35,300 # CDC_ECM: Handling TX xmit from netdev
2021-10-31 21:24:35,305 # success: added 2001:db8::2/64 to interface 4
2021-10-31 21:24:35,307 # CDC_ECM: Handling TX xmit from netdev
2021-10-31 21:24:51,075 # CDC_ECM: Handling TX xmit from netdev
2021-10-31 21:24:51,078 # CDC_ECM: Handling TX xmit from netdev
2021-10-31 21:25:06,851 # CDC_ECM: Handling TX xmit from netdev
2021-10-31 21:25:06,855 # CDC_ECM: Handling TX xmit from netdev
2021-10-31 21:25:13,271 # CDC ECM: Request: 0x43
2021-10-31 21:25:13,274 # CDC ECM: Not modifying filter to 0xe
2021-10-31 21:25:13,275 # Length: 64
2021-10-31 21:25:13,276 # Length: 90
2021-10-31 21:25:13,521 # Length: 64
2021-10-31 21:25:13,522 # Length: 86
2021-10-31 21:25:13,569 # Length: 64
2021-10-31 21:25:13,570 # Length: 90
2021-10-31 21:25:14,927 # Length: 64
2021-10-31 21:25:14,927 # Length: 86
2021-10-31 21:25:14,931 # CDC_ECM: Handling TX xmit from netdev
2021-10-31 21:25:14,935 # CDC_ECM: Handling TX xmit from netdev
2021-10-31 21:25:14,936 # Length: 64
2021-10-31 21:25:14,937 # Length: 128
2021-10-31 21:25:14,938 # Length: 192
2021-10-31 21:25:14,939 # Length: 256
2021-10-31 21:25:14,940 # Length: 320
2021-10-31 21:25:14,941 # Length: 384
2021-10-31 21:25:14,942 # Length: 448
2021-10-31 21:25:14,943 # Length: 512
2021-10-31 21:25:14,945 # Length: 576
2021-10-31 21:25:14,946 # Length: 640
2021-10-31 21:25:14,947 # Length: 704
2021-10-31 21:25:14,948 # Length: 768
2021-10-31 21:25:14,949 # Length: 832
2021-10-31 21:25:14,950 # Length: 896
2021-10-31 21:25:14,951 # Length: 960
2021-10-31 21:25:14,952 # Length: 1024
2021-10-31 21:25:14,954 # Endpoint 1 halted
2021-10-31 21:25:14,955 # Length: 64
2021-10-31 21:25:14,956 # Endpoint 1 unhalted
2021-10-31 21:25:15,961 # Length: 128
2021-10-31 21:25:15,962 # Length: 192
2021-10-31 21:25:15,963 # Length: 256
2021-10-31 21:25:15,964 # Length: 320
2021-10-31 21:25:15,966 # Length: 384
2021-10-31 21:25:15,967 # Length: 448
2021-10-31 21:25:15,968 # Length: 512
2021-10-31 21:25:15,969 # Length: 576
2021-10-31 21:25:15,970 # Length: 640
2021-10-31 21:25:15,971 # Length: 704
2021-10-31 21:25:15,972 # Length: 768
2021-10-31 21:25:15,973 # Length: 832
2021-10-31 21:25:15,974 # Length: 896
2021-10-31 21:25:15,975 # Length: 960
2021-10-31 21:25:15,977 # Length: 1024
2021-10-31 21:25:15,978 # Endpoint 1 halted
2021-10-31 21:25:15,979 # Length: 64
2021-10-31 21:25:15,981 # Endpoint 1 unhalted

Of interest in that excerpt are the two "Endpoint 1 halted" and "Endpoint 1 unhalted".

In Wireshark, dumping the USB traffic, this looks like:

With the CLEAR FEATURE request visible to clear the stall condition on the endpoint

After the stall condition traffic to the device continues again normally without requiring a restart of the device or the USB connection.

Issues/PRs references

Needs #17086

[dylad]

@bergzand I tried on SAME54-XPRO with your patch applied on top on this branch. I can see:

2021-11-04 21:08:37,153 # Endpoint 1 halted
2021-11-04 21:08:37,154 # Endpoint 1 unhalted

But after that, I cannot ping anymore the board. Looks like the board didn't recover.

[bergzand]

@bergzand I tried on SAME54-XPRO with your patch applied on top on this branch.

The overall problem with this PR is that the handler (CDC-ECM in this case) is not aware that the halt condition got resolved and that it can restart the pipe to the host. I think this needs some callback (not sure if we can reuse one) triggered at the end of the clear halt function. On the CDC-ECM end this would flush the RX status.

[OlegHahm]

What's the state here?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[gschorcht]

Please rebase.

[bergzand]

Please rebase.

rebased!

[bergzand]

Main issue with this PR so far is that there is no mechanism to communicate the halt clear back to the interface. Meaning that in the test example above, the CDC ECM interface is not aware that communication can resume. This needs an extension of the USBUS interface api

[riot-ci]

Murdock results

:heavy_check_mark: PASSED

7b2db7bf2b7d5c4dc1ebb4d9528baecb6189d0e8 usbus: Implement GET STATUS and SET/CLEAR FEATURE requests

Success	Failures	Total	Runtime
6882	0	6882	10m:14s

Artifacts

• Documentation preview

[gschorcht]

bors merge

[gschorcht]

bors cancel

[bors[bot]]

Canceled.

[gschorcht]

bors merge

[bors[bot]]

Build succeeded:

• Murdock
• static-tests
• tools-build-success