Re: [PATCH usbredirserver] usbredirserver : enable TCP keepalive

["Zhenwei.Pi" <zhenwei.pi@xxxxxxxxxxxxxxx>]

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.pi@xxxxxxxxxxxxxxx>
> > ---
> >    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");
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel