search

cezanne / usbip-win

USB/IP for Windows
GNU General Public License v3.0
1.96k stars 351 forks source link

TCP KeepAlive: Handle USBIP Client interrupts #33

Closed Oxalin closed 3 years ago

Oxalin commented 5 years ago

According to the following bug report, an attached device will not be freed if the connection between a client and a server is abruptly interrupted.

From here: https://sourceforge.net/p/usbip/discussion/418508/thread/34da2931c0/?limit=25#13b9

Under usbip_network.c > usbip_net_tcp_connect(), there is a TODO note stating "/ TODO: write code for heartbeat /".

yelfarri commented 5 years ago

@Oxalin the keepalive (heartbeat) was completed I see the keepalive message when using Wireshark. Isn't keepAlive a part of the TCPIP or from the application? Since we are getting the keepAlive messages does that mean that the configuration of time and interval in application side is wrong?

image

Oxalin commented 5 years ago

It can be set system wide and per application / socket. From my quick overlook at usbipd, the application is not setting any customized Keep-Alive options, thus relying on the system's default values.

yelfarri commented 5 years ago

I understand but I m talking about usbip client side. I Found in code that usbip has a keepalive function call in "usbip_network.c" line 206. If I remember correctly this call does not set time and intervals. It might be using system default values. Is "usbip_network.c" shared between usbipd server and usbip client?

Oxalin commented 5 years ago

Yes, indeed. setsockopt() could be called to set the IPPROTO_TCP socket options (as described here https://docs.microsoft.com/en-us/windows/desktop/WinSock/ipproto-tcp-socket-options).However, many options are only available under Windows 10 (1709 and up), even though there are commonly available under Linux...

Thus, I would be tempted to add a heartbeat (keepalive) at the application level. This way, we will be able to support a decent heartbeat in any Windows version. That's why the usbip_net_set_keepalive() doesn't set any IPPROTO_TCP socket options: it was supposed to be added under usbip_net_tcp_connect(). See line 259.

cezuni commented 5 years ago

@Oxalin : I agree that application-level heartbeat can help to detect abortive disconnection more quickly. But it's impossible without usbip protocol modification. I think that it's a too big task to resolve at usbip-win project level.

cezanne commented 3 years ago

Socket-specific KeepAlive is implemented by be88d56c4d740d453436777622defe0583098910. Application-level heartbeat might be not useful unless a linux implementation follows.