[Spice-devel] [PATCH] usbredirserver : enable TCP keepalive
In some bad cases, for example, host OS crashes without sending any FIN to usbredirserver, and usbredirserver will keep idle connection for a long time. We can also set the kernel arguments, it means that other processes may be affected. Setting a sensible timeout(like 10 minutes) seems good. But QEMU is restarted after host OS crashes, it usually needs to reconnect usbredirserver within 2 minutes. So, add cmdline argument '-k' and "--keepalive" for usbredirserver to set tcp keepalive idle time, interval time(10s), and maximum number of keepalive probes count(3). Detecting disconnection costs time is : idle time + 10s * 3 If setting TCP keepalive fails with errno ENOTSUP, ignore the specific error. Signed-off-by: zhenwei.pi <zhenwei...@youruncloud.com> Tested-by: Uri Lublin <u...@redhat.com> Acked-by: Frediano Ziglio <fzig...@redhat.com> --- usbredirserver/usbredirserver.c | 45 - 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/usbredirserver/usbredirserver.c b/usbredirserver/usbredirserver.c index 5575181..849aa05 100644 --- a/usbredirserver/usbredirserver.c +++ b/usbredirserver/usbredirserver.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "usbredirhost.h" @@ -52,6 +53,7 @@ static const struct option longopts[] = { { "verbose", required_argument, NULL, 'v' }, { "ipv4", required_argument, NULL, '4' }, { "ipv6", required_argument, NULL, '6' }, +{ "keepalive", required_argument, NULL, 'k' }, { "help", no_argument, NULL, 'h' }, { NULL, 0, NULL, 0 } }; @@ -98,6 +100,7 @@ static void usage(int exit_code, char *argv0) fprintf(exit_code? stderr:stdout, "Usage: %s [-p|--port ] [-v|--verbose <0-5>] " "[[-4|--ipv4 ipaddr]|[-6|--ipv6 ipaddr]] " +"[-k|--keepalive seconds] " "<busnum-devnum|vendorid:prodid>\n", argv0); exit(exit_code); @@ -203,6 +206,7 @@ int main(int argc, char *argv[]) int usbvendor = -1; int usbproduct = -1; int on = 1; +int keepalive = -1; char *ipv4_addr = NULL, *ipv6_addr = NULL; union { struct sockaddr_in v4; @@ -211,7 +215,7 @@ int main(int argc, char *argv[]) struct sigaction act; libusb_device_handle *handle = NULL; -while ((o = getopt_long(argc, argv, "hp:v:4:6:", longopts, NULL)) != -1) { +while ((o = getopt_long(argc, argv, "hp:v:4:6:k:", longopts, NULL)) != -1) { switch (o) { case 'p': port = strtol(optarg, , 10); @@ -233,6 +237,13 @@ int main(int argc, char *argv[]) case '6': ipv6_addr = optarg; break; +case 'k': +keepalive = strtol(optarg, , 10); +if (*endptr != '\0') { +fprintf(stderr, "Invalid value for -k: '%s'\n", optarg); +usage(1, argv[0]); +} +break; case '?': case 'h': usage(o == '?', argv[0]); @@ -348,6 +359,38 @@ int main(int argc, char *argv[]) break; } +if (keepalive > 0) { +int optval = 1; +socklen_t optlen = sizeof(optval); +if (setsockopt(client_fd, SOL_SOCKET, SO_KEEPALIVE, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt SO_KEEPALIVE error."); +break; +} +} +optval = keepalive;/* set default TCP_KEEPIDLE time from cmdline */ +if (setsockopt(client_fd, SOL_TCP, TCP_KEEPIDLE, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt TCP_KEEPIDLE error."); +break; +} +} +optval = 10; /* set default TCP_KEEPINTVL time as 10s */ +if (setsockopt(client_fd, SOL_TCP, TCP_KEEPINTVL, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt TCP_KEEPINTVL error."); +break; +} +} +optval = 3;/* set default TCP_KEEPCNT as 3 */ +if (setsockopt(client_fd, SOL_TCP, TCP_KEEPCNT, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt TCP_KEEPCNT error."); +break; +} +} +} + flags = fcntl(client_fd, F_GETFL); if (flags == -1) { perror("fcntl F_GETFL"); -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] usbredirserver : enable TCP keepalive
In some bad cases, for example, host OS crashes without sending any FIN to usbredirserver, and usbredirserver will keep idle connection for a long time. We can also set the kernel arguments, it means that other processes may be affected. Setting a sensible timeout(like 10 minutes) seems good. But QEMU is restarted after host OS crashes, it usually needs to reconnect usbredirserver within 2 minutes. So, add cmdline argument '-k' and "--keepalive" for usbredirserver to set tcp keepalive idle time(30s), interval time(10s), and maximum number of keepalive probes count(3). Detecting disconnection costs time is : 30s + 10s * 3 = 60s If setting TCP keepalive fails with errno ENOTSUP, ignore the specific error. Signed-off-by: zhenwei.pi <zhenwei...@youruncloud.com> --- usbredirserver/usbredirserver.c | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/usbredirserver/usbredirserver.c b/usbredirserver/usbredirserver.c index 5575181..246171c 100644 --- a/usbredirserver/usbredirserver.c +++ b/usbredirserver/usbredirserver.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "usbredirhost.h" @@ -52,6 +53,7 @@ static const struct option longopts[] = { { "verbose", required_argument, NULL, 'v' }, { "ipv4", required_argument, NULL, '4' }, { "ipv6", required_argument, NULL, '6' }, +{ "keepalive", no_argument, NULL, 'k' }, { "help", no_argument, NULL, 'h' }, { NULL, 0, NULL, 0 } }; @@ -98,6 +100,7 @@ static void usage(int exit_code, char *argv0) fprintf(exit_code? stderr:stdout, "Usage: %s [-p|--port ] [-v|--verbose <0-5>] " "[[-4|--ipv4 ipaddr]|[-6|--ipv6 ipaddr]] " +"[-k|--keepalive] " "<busnum-devnum|vendorid:prodid>\n", argv0); exit(exit_code); @@ -203,6 +206,7 @@ int main(int argc, char *argv[]) int usbvendor = -1; int usbproduct = -1; int on = 1; +int keepalive = 0; char *ipv4_addr = NULL, *ipv6_addr = NULL; union { struct sockaddr_in v4; @@ -211,7 +215,7 @@ int main(int argc, char *argv[]) struct sigaction act; libusb_device_handle *handle = NULL; -while ((o = getopt_long(argc, argv, "hp:v:4:6:", longopts, NULL)) != -1) { +while ((o = getopt_long(argc, argv, "hp:v:4:6:k", longopts, NULL)) != -1) { switch (o) { case 'p': port = strtol(optarg, , 10); @@ -233,6 +237,9 @@ int main(int argc, char *argv[]) case '6': ipv6_addr = optarg; break; +case 'k': +keepalive = 1; +break; case '?': case 'h': usage(o == '?', argv[0]); @@ -348,6 +355,38 @@ int main(int argc, char *argv[]) break; } +if (keepalive) { +int optval = 1; +socklen_t optlen = sizeof(optval); +if (setsockopt(client_fd, SOL_SOCKET, SO_KEEPALIVE, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt SO_KEEPALIVE error."); +break; +} +} +optval = 30; /* set default TCP_KEEPIDLE time as 30s */ +if (setsockopt(client_fd, SOL_TCP, TCP_KEEPIDLE, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt TCP_KEEPIDLE error."); +break; +} +} +optval = 10; /* set default TCP_KEEPINTVL time as 10s */ +if (setsockopt(client_fd, SOL_TCP, TCP_KEEPINTVL, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt TCP_KEEPINTVL error."); +break; +} +} +optval = 3;/* set default TCP_KEEPCNT as 3 */ +if (setsockopt(client_fd, SOL_TCP, TCP_KEEPCNT, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt TCP_KEEPCNT error."); +break; +} +} +} + flags = fcntl(client_fd, F_GETFL); if (flags == -1) { perror("fcntl F_GETFL"); -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH usbredirserver] usbredirserver : enable TCP keepalive
On 03/08/2018 03:30 AM, Uri Lublin wrote: On 03/01/2018 11:08 AM, zhenwei.pi wrote: In some bad cases, for example, host OS crashes without sending any FIN to usbredirserver, and usbredirserver will keep idle connection for a long time. We can also set the kernel arguments, it means that other processes may be affected. Setting a sensible timeout(like 10 minutes) seems good. But QEMU is restarted after host OS crashes, it usually needs to reconnect usbredirserver within 2 minutes. Hi, Note that since you only setup TCP_KEEPIDLE, it will take longer to detect disconnection (interval * probes more, aka TCP_KEEPINTVL * TCP_KEEPCNT). How much longer depends on the system configuration. The patch looks good. I tested it with usbredirtestclient and iptables. Uri. We can also add more cmdline arguments for TCP_KEEPINTVL and TCP_KEEPCNT, but usbredirserver seems a little difficult to setup. Setting default value (like TCP_KEEPINTVL as 10s, TCP_KEEPCNT as 3) or using the system configuration, which is better? So, add cmdline argument '-k' and "--keepalive" for usbredirserver to set tcp keepalive idle time. If setting TCP keepalive fails with errno ENOTSUP, ignore the specific error. Signed-off-by: zhenwei.pi <zhenwei...@youruncloud.com> --- usbredirserver/usbredirserver.c | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/usbredirserver/usbredirserver.c b/usbredirserver/usbredirserver.c index 5575181..3a7620a 100644 --- a/usbredirserver/usbredirserver.c +++ b/usbredirserver/usbredirserver.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "usbredirhost.h" @@ -52,6 +53,7 @@ static const struct option longopts[] = { { "verbose", required_argument, NULL, 'v' }, { "ipv4", required_argument, NULL, '4' }, { "ipv6", required_argument, NULL, '6' }, + { "keepalive", required_argument, NULL, 'k' }, { "help", no_argument, NULL, 'h' }, { NULL, 0, NULL, 0 } }; @@ -98,6 +100,7 @@ static void usage(int exit_code, char *argv0) fprintf(exit_code? stderr:stdout, "Usage: %s [-p|--port ] [-v|--verbose <0-5>] " "[[-4|--ipv4 ipaddr]|[-6|--ipv6 ipaddr]] " + "[-k|--keepalive time] " "<busnum-devnum|vendorid:prodid>\n", argv0); exit(exit_code); @@ -203,6 +206,7 @@ int main(int argc, char *argv[]) int usbvendor = -1; int usbproduct = -1; int on = 1; + int keepalive = -1; char *ipv4_addr = NULL, *ipv6_addr = NULL; union { struct sockaddr_in v4; @@ -211,7 +215,7 @@ int main(int argc, char *argv[]) struct sigaction act; libusb_device_handle *handle = NULL; - while ((o = getopt_long(argc, argv, "hp:v:4:6:", longopts, NULL)) != -1) { + while ((o = getopt_long(argc, argv, "hp:v:4:6:k:", longopts, NULL)) != -1) { switch (o) { case 'p': port = strtol(optarg, , 10); @@ -233,6 +237,13 @@ int main(int argc, char *argv[]) case '6': ipv6_addr = optarg; break; + case 'k': + keepalive = strtol(optarg, , 10); + if (*endptr != '\0') { + fprintf(stderr, "Invalid value for --keepalive: '%s'\n", optarg); + usage(1, argv[0]); + } + break; case '?': case 'h': usage(o == '?', argv[0]); @@ -348,6 +359,24 @@ int main(int argc, char *argv[]) break; } + if (keepalive > 0) { + int optval = 1; + socklen_t optlen = sizeof(optval); + if (setsockopt(client_fd, SOL_SOCKET, SO_KEEPALIVE, , optlen) == -1) { + if (errno != ENOTSUP) { + perror("setsockopt SO_KEEPALIVE error."); + break; + } + } + optval = keepalive; + if (setsockopt(client_fd, SOL_TCP, TCP_KEEPIDLE, , optlen) == -1) { + if (errno != ENOTSUP) { + perror("setsockopt TCP_KEEPIDLE error."); + break; + } + } + } + flags = fcntl(client_fd, F_GETFL); if (flags == -1) { perror("fcntl F_GETFL"); ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH usbredirserver] usbredirserver : enable TCP keepalive
In some bad cases, for example, host OS crashes without sending any FIN to usbredirserver, and usbredirserver will keep idle connection for a long time. We can also set the kernel arguments, it means that other processes may be affected. Setting a sensible timeout(like 10 minutes) seems good. But QEMU is restarted after host OS crashes, it usually needs to reconnect usbredirserver within 2 minutes. So, add cmdline argument '-k' and "--keepalive" for usbredirserver to set tcp keepalive idle time. If setting TCP keepalive fails with errno ENOTSUP, ignore the specific error. Signed-off-by: zhenwei.pi <zhenwei...@youruncloud.com> I think I got completely wrong the problem. So this patch is trying to use tcp keepalives to detect disconnection making possible new connections. The problem is that the server stops accepting connections when it handle one client and return to accepting new connections when it finishes servicing the client. This as the usb device usage is exclusive. I can see that in the current protocol there's no authentication so all are filtered at tcp/firewall levels. I suppose being usb redirection that the expected latency is relatively low. The disconnection is detected when writing, as the other side is not replying with acks (or if the machine became available replies with some ICMP notifications) so the usb server must be stuck reading from the client. Would not better instead of having to tweak the time by hand to detect new connection attempts and then try to detect disconnections or either stop serving the current client? Patch looks good otherwise. This patch is a method of "try to detect disconnections"! "client_fd" means the connection of the server and client, and the server sets TCP keepalive for the connection. So the server will send TCP KEEPALIVE message to client. If client does not response or send RESET, the server will release the idle connection. A new connection (or reconnection) could be accepted by the server. --- usbredirserver/usbredirserver.c | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/usbredirserver/usbredirserver.c b/usbredirserver/usbredirserver.c index 5575181..3a7620a 100644 --- a/usbredirserver/usbredirserver.c +++ b/usbredirserver/usbredirserver.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "usbredirhost.h" @@ -52,6 +53,7 @@ static const struct option longopts[] = { { "verbose", required_argument, NULL, 'v' }, { "ipv4", required_argument, NULL, '4' }, { "ipv6", required_argument, NULL, '6' }, +{ "keepalive", required_argument, NULL, 'k' }, { "help", no_argument, NULL, 'h' }, { NULL, 0, NULL, 0 } }; @@ -98,6 +100,7 @@ static void usage(int exit_code, char *argv0) fprintf(exit_code? stderr:stdout, "Usage: %s [-p|--port ] [-v|--verbose <0-5>] " "[[-4|--ipv4 ipaddr]|[-6|--ipv6 ipaddr]] " +"[-k|--keepalive time] " "<busnum-devnum|vendorid:prodid>\n", argv0); exit(exit_code); @@ -203,6 +206,7 @@ int main(int argc, char *argv[]) int usbvendor = -1; int usbproduct = -1; int on = 1; +int keepalive = -1; char *ipv4_addr = NULL, *ipv6_addr = NULL; union { struct sockaddr_in v4; @@ -211,7 +215,7 @@ int main(int argc, char *argv[]) struct sigaction act; libusb_device_handle *handle = NULL; -while ((o = getopt_long(argc, argv, "hp:v:4:6:", longopts, NULL)) != -1) { +while ((o = getopt_long(argc, argv, "hp:v:4:6:k:", longopts, NULL)) != -1) { switch (o) { case 'p': port = strtol(optarg, , 10); @@ -233,6 +237,13 @@ int main(int argc, char *argv[]) case '6': ipv6_addr = optarg; break; +case 'k': +keepalive = strtol(optarg, , 10); +if (*endptr != '\0') { +fprintf(stderr, "Invalid value for --keepalive: '%s'\n", optarg); +usage(1, argv[0]); +} +break; case '?': case 'h': usage(o == '?', argv[0]); @@ -348,6 +359,24 @@ int main(int argc, char *argv[]) break; } +if (keepalive > 0) { +int optval = 1; +socklen_t optlen = sizeof(optval); +if (setsockopt(client_fd, SOL_SOCKET, SO_KEEPALIVE, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt SO_KEEPALIVE error."); +break; +} +} +optval = keepalive; +if (setsockopt(client_fd, SOL_TCP, TCP_KEEPIDLE, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsock
[Spice-devel] [PATCH usbredirserver] usbredirserver : enable TCP keepalive
In some bad cases, for example, host OS crashes without sending any FIN to usbredirserver, and usbredirserver will keep idle connection for a long time. We can also set the kernel arguments, it means that other processes may be affected. Setting a sensible timeout(like 10 minutes) seems good. But QEMU is restarted after host OS crashes, it usually needs to reconnect usbredirserver within 2 minutes. So, add cmdline argument '-k' and "--keepalive" for usbredirserver to set tcp keepalive idle time. If setting TCP keepalive fails with errno ENOTSUP, ignore the specific error. Signed-off-by: zhenwei.pi <zhenwei...@youruncloud.com> --- usbredirserver/usbredirserver.c | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/usbredirserver/usbredirserver.c b/usbredirserver/usbredirserver.c index 5575181..3a7620a 100644 --- a/usbredirserver/usbredirserver.c +++ b/usbredirserver/usbredirserver.c @@ -37,6 +37,7 @@ #include #include #include +#include #include "usbredirhost.h" @@ -52,6 +53,7 @@ static const struct option longopts[] = { { "verbose", required_argument, NULL, 'v' }, { "ipv4", required_argument, NULL, '4' }, { "ipv6", required_argument, NULL, '6' }, +{ "keepalive", required_argument, NULL, 'k' }, { "help", no_argument, NULL, 'h' }, { NULL, 0, NULL, 0 } }; @@ -98,6 +100,7 @@ static void usage(int exit_code, char *argv0) fprintf(exit_code? stderr:stdout, "Usage: %s [-p|--port ] [-v|--verbose <0-5>] " "[[-4|--ipv4 ipaddr]|[-6|--ipv6 ipaddr]] " +"[-k|--keepalive time] " "<busnum-devnum|vendorid:prodid>\n", argv0); exit(exit_code); @@ -203,6 +206,7 @@ int main(int argc, char *argv[]) int usbvendor = -1; int usbproduct = -1; int on = 1; +int keepalive = -1; char *ipv4_addr = NULL, *ipv6_addr = NULL; union { struct sockaddr_in v4; @@ -211,7 +215,7 @@ int main(int argc, char *argv[]) struct sigaction act; libusb_device_handle *handle = NULL; -while ((o = getopt_long(argc, argv, "hp:v:4:6:", longopts, NULL)) != -1) { +while ((o = getopt_long(argc, argv, "hp:v:4:6:k:", longopts, NULL)) != -1) { switch (o) { case 'p': port = strtol(optarg, , 10); @@ -233,6 +237,13 @@ int main(int argc, char *argv[]) case '6': ipv6_addr = optarg; break; +case 'k': +keepalive = strtol(optarg, , 10); +if (*endptr != '\0') { +fprintf(stderr, "Invalid value for --keepalive: '%s'\n", optarg); +usage(1, argv[0]); +} +break; case '?': case 'h': usage(o == '?', argv[0]); @@ -348,6 +359,24 @@ int main(int argc, char *argv[]) break; } +if (keepalive > 0) { +int optval = 1; +socklen_t optlen = sizeof(optval); +if (setsockopt(client_fd, SOL_SOCKET, SO_KEEPALIVE, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt SO_KEEPALIVE error."); +break; +} +} +optval = keepalive; +if (setsockopt(client_fd, SOL_TCP, TCP_KEEPIDLE, , optlen) == -1) { +if (errno != ENOTSUP) { +perror("setsockopt TCP_KEEPIDLE error."); +break; +} +} +} + flags = fcntl(client_fd, F_GETFL); if (flags == -1) { perror("fcntl F_GETFL"); -- 2.7.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel