[Spice-devel] [PATCH 1/1] channel: add optional tcp keepalive timeout to channels

[Frediano Ziglio]

> Hi,

> With firewall running between spice server and client, if idle time is larger
> than firewall session timeout, spice sessions freeze and users lose their
> keyboard and mouse control.

> To workaround this issue, I made a patch to add tcp keepalive timeout to
> spice server. The timeout can be added to qemu config like below.
> <domain type='kvm' xmlns:qemu=' http://libvirt.org/schemas/domain/qemu/1.0 '>
> <qemu:commandline>
> <qemu:env name='SPICE_KEEPALIVE_TIMEOUT' value='1800'/>
> </qemu:commandline>

> I wanted to add this option to spice client, but there was no setsockopt()
> option of TCP_KEEPIDLE for windows platform, So, I ended up adding it to
> spice server. Please review the patch and let me know what you think. Thank
> you.

For Windows see https://msdn.microsoft.com/en-us/library/windows/desktop/ee470551%28v=vs.85%29.aspx, are used SO_KEEPALIVE and SIO_KEEPALIVE_VALS. 

> ---------------------------------------------------------------------------------------------------
> [PATCH 1/1] channel: add optional tcp keepalive timeout to channels

> Signed-off-by: Sunny Shin < sunny4s.git at gmail.com >
> ---
> server/reds.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)

> diff --git a/server/reds.c b/server/reds.c
> index 8b3c3cb..05d0b1d 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2254,6 +2254,9 @@ static RedLinkInfo *reds_init_client_connection(int
> socket)
> RedLinkInfo *link;
> int delay_val = 1;
> int flags;
> + char *keepalive_timeout_str;
> + int keepalive_timeout;
> + int keepalive = 1;

> if ((flags = fcntl(socket, F_GETFL)) == -1) {
> spice_warning("accept failed, %s", strerror(errno));
> @@ -2271,6 +2274,31 @@ static RedLinkInfo *reds_init_client_connection(int
> socket)
> }
> }

> + keepalive_timeout_str = getenv("SPICE_KEEPALIVE_TIMEOUT");

See Djasa comment, environment are not good here. 

> + if (keepalive_timeout_str != NULL) {
> + errno = 0;
> + keepalive_timeout = strtol(keepalive_timeout_str, NULL, 10);
> + if (errno != 0) {
> + spice_warning("error parsing SPICE_KEEPALIVE_TIMEOUT: %s",
> strerror(errno));
> + goto error;
> + }
> +
> + spice_debug("keepalive timeout %ds", keepalive_timeout);
> + if (setsockopt(socket, SOL_SOCKET, SO_KEEPALIVE, &keepalive,
> sizeof(keepalive)) == -1) {
> + if (errno != ENOTSUP) {
> + spice_printerr("setsockopt for keepalive failed, %s", strerror(errno));
> + goto error;
> + }
> + }

I would just issue a warning on error and keep going (not jump to error), we can live without keepalive 

> + if (setsockopt(socket, SOL_TCP, TCP_KEEPIDLE,
> + &keepalive_timeout, sizeof(keepalive_timeout)) == -1) {
> + if (errno != ENOTSUP) {
> + spice_printerr("setsockopt for keepalive timeout failed, %s",
> strerror(errno));
> + goto error;
> + }
> + }

same here. 

> + }
> +
> link = spice_new0(RedLinkInfo, 1);
> link->stream = reds_stream_new(socket);

Frediano 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151127/1ceca858/attachment.html>