From c67afb07a1fe48468bf526ee9397d95280b605b8 Mon Sep 17 00:00:00 2001 From: windowsair Date: Sun, 3 Oct 2021 23:04:27 +0800 Subject: [PATCH] fix(usbip): Try to handle UNLINK behavior --- components/USBIP/usbip_defs.h | 1 + main/DAP_handle.c | 37 ++++++++++++++++++++++++++++++++++- main/DAP_handle.h | 1 + main/usbip_server.c | 8 +++++++- sdkconfig | 6 +++--- 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/components/USBIP/usbip_defs.h b/components/USBIP/usbip_defs.h index 84112a7..fe3550b 100644 --- a/components/USBIP/usbip_defs.h +++ b/components/USBIP/usbip_defs.h @@ -189,6 +189,7 @@ typedef struct typedef struct { int32_t status; + uint8_t padding[24]; // Linux only. For usbip-win, it ignores this field. } __attribute__((packed)) usbip_stage2_header_ret_unlink; /** diff --git a/main/DAP_handle.c b/main/DAP_handle.c index 9fe2ccb..4687b42 100644 --- a/main/DAP_handle.c +++ b/main/DAP_handle.c @@ -1,10 +1,11 @@ /** * @file DAP_handle.c * @brief Handle DAP packets and transaction push - * @version 0.4 + * @version 0.5 * @change: 2020.02.04 first version * 2020.11.11 support WinUSB mode * 2021.02.17 support SWO + * 2021.10.03 try to handle unlink behavior * * @copyright Copyright (c) 2021 * @@ -266,3 +267,37 @@ int fast_reply(uint8_t *buf, uint32_t length) } return 0; } + +void handle_dap_unlink() +{ + // `USBIP_CMD_UNLINK` means calling `usb_unlink_urb()` or `usb_kill_urb()`. + // Note that execution of an URB is inherently an asynchronous operation, and there may be + // synchronization problems in the following solutions. + + // One of the reasons this happens is that the host wants to abort the URB transfer operation + // as soon as possible. USBIP network fluctuations will also cause this error, but I don't know + // whether this is the main reason. + + // Unlink may be applied to zero length URB of "DIR_IN", or a URB containing data. + // In the case of unlink, for the new "DIR_IN" request, it may always return an older response, + // which will lead to panic. This code is a compromise for eliminating the lagging response + // caused by UNLINK. It will clean up the buffers that may have data for return to the host. + // In general, we assume that there is at most one piece of data that has not yet been returned. + if (dap_respond > 0) + { + DapPacket_t *item; + size_t packetSize = 0; + item = (DapPacket_t *)xRingbufferReceiveUpTo(dap_dataOUT_handle, &packetSize, + pdMS_TO_TICKS(10), DAP_HANDLE_SIZE); + if (packetSize == DAP_HANDLE_SIZE) + { + if (xSemaphoreTake(data_response_mux, portMAX_DELAY) == pdTRUE) + { + --dap_respond; + xSemaphoreGive(data_response_mux); + } + + vRingbufferReturnItem(dap_dataOUT_handle, (void *)item); + } + } +} \ No newline at end of file diff --git a/main/DAP_handle.h b/main/DAP_handle.h index bce40ca..4d75e6e 100644 --- a/main/DAP_handle.h +++ b/main/DAP_handle.h @@ -6,6 +6,7 @@ void handle_dap_data_request(usbip_stage2_header *header, uint32_t length); void handle_dap_data_response(usbip_stage2_header *header); void handle_swo_trace_response(usbip_stage2_header *header); +void handle_dap_unlink(); int fast_reply(uint8_t *buf, uint32_t length); diff --git a/main/usbip_server.c b/main/usbip_server.c index d0a8ee5..29c2692 100644 --- a/main/usbip_server.c +++ b/main/usbip_server.c @@ -375,8 +375,10 @@ void send_stage2_submit_data_fast(usbip_stage2_header *req_header, const void *c static void handle_unlink(usbip_stage2_header *header) { os_printf("s2 handling cmd unlink...\r\n"); + handle_dap_unlink(); send_stage2_unlink(header); } + static void send_stage2_unlink(usbip_stage2_header *req_header) { @@ -385,7 +387,11 @@ static void send_stage2_unlink(usbip_stage2_header *req_header) memset(&(req_header->u.ret_unlink), 0, sizeof(usbip_stage2_header_ret_unlink)); - // req_header.u.ret_unlink.status = 0; + // To be more precise, the value is `-ECONNRESET`, but usbip-win only cares if it is a + // non zero value. A non-zero value indicates that our UNLINK operation was "successful", + // but the host driver's may behave differently, or may even ignore this state. For consistent + // behavior, we use non-zero value here. See also comments regarding `handle_dap_unlink()`. + req_header->u.ret_unlink.status = -1; pack(req_header, sizeof(usbip_stage2_header)); diff --git a/sdkconfig b/sdkconfig index e9edbd4..22d1a06 100644 --- a/sdkconfig +++ b/sdkconfig @@ -278,18 +278,18 @@ CONFIG_LWIP_MAX_LISTENING_TCP=8 CONFIG_LWIP_TCP_MAXRTX=12 CONFIG_LWIP_TCP_SYNMAXRTX=6 CONFIG_LWIP_TCP_MSS=1460 -CONFIG_LWIP_TCP_TMR_INTERVAL=250 +CONFIG_LWIP_TCP_TMR_INTERVAL=100 CONFIG_LWIP_TCP_MSL=60000 CONFIG_LWIP_TCP_SND_BUF_DEFAULT=11680 CONFIG_LWIP_TCP_WND_DEFAULT=10240 CONFIG_LWIP_TCP_RECVMBOX_SIZE=16 CONFIG_LWIP_TCP_QUEUE_OOSEQ=y -# CONFIG_LWIP_TCP_SACK_OUT is not set +CONFIG_LWIP_TCP_SACK_OUT=y # CONFIG_LWIP_TCP_KEEP_CONNECTION_WHEN_IP_CHANGES is not set CONFIG_LWIP_TCP_OVERSIZE_MSS=y # CONFIG_LWIP_TCP_OVERSIZE_QUARTER_MSS is not set # CONFIG_LWIP_TCP_OVERSIZE_DISABLE is not set -CONFIG_LWIP_TCP_RTO_TIME=3000 +CONFIG_LWIP_TCP_RTO_TIME=1500 CONFIG_LWIP_MAX_UDP_PCBS=4 CONFIG_LWIP_UDP_RECVMBOX_SIZE=6 CONFIG_LWIP_TCPIP_TASK_STACK_SIZE=4096