>
> 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?
>
Setting fixed (compiled in) values if -k is specified seems a good idea
to me.
>
> >> 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 <netdb.h>
> >> #include <netinet/in.h>
> >> #include <arpa/inet.h>
> >> +#include <netinet/tcp.h>
> >> #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 <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, &endptr, 10);
> >> @@ -233,6 +237,13 @@ int main(int argc, char *argv[])
> >> case '6':
> >> ipv6_addr = optarg;
> >> break;
> >> + case 'k':
> >> + keepalive = strtol(optarg, &endptr, 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, &optval,
> >> optlen) == -1) {
> >> + if (errno != ENOTSUP) {
> >> + perror("setsockopt SO_KEEPALIVE error.");
> >> + break;
> >> + }
> >> + }
> >> + optval = keepalive;
> >> + if (setsockopt(client_fd, SOL_TCP, TCP_KEEPIDLE, &optval,
> >> optlen) == -1) {
> >> + if (errno != ENOTSUP) {
> >> + perror("setsockopt TCP_KEEPIDLE error.");
> >> + break;
> >> + }
> >> + }
> >> + }
> >> +
> >> flags = fcntl(client_fd, F_GETFL);
> >> if (flags == -1) {
> >> perror("fcntl F_GETFL");
> >
>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
