Re: [PATCH 4/5] util/qemu-sockets: add TCP_USER_TIMEOUT support

[Markus Armbruster <armbru@redhat.com>]

Mitsuru Kariya <Mitsuru.Kariya@oss.nttdata.com> writes:

> The TCP_USER_TIMEOUT socket option (Linux-only) specifies the maximum
> amount of time in milliseconds that transmitted data may remain
> unacknowledged, or buffered data may remain untransmitted, before TCP
> forcibly closes the connection and returns ETIMEDOUT to the
> application.  Without it, a peer that becomes silently unreachable can
> take well over an hour to be noticed (the kernel default), which is
> particularly painful for use cases like QEMU live migration.
>
> Expose user-timeout as a new optional InetSocketAddress member, gated
> on a new HAVE_TCP_USER_TIMEOUT meson check, and apply it in
> inet_set_sockopts() alongside the existing keep-alive options.  Plumb
> the same option through the legacy QemuOpts-based inet_parse() path so
> that URI-style users (-incoming tcp:, migrate tcp:, HMP
> nbd_server_start, ...) can set it too.
>
> Update the documentation for -netdev stream (addr.user-timeout=N) and
> -incoming tcp: (user-timeout=N, flat key as for the other inet_opts
> entries), and extend test_inet_parse_all_options_good accordingly.
>
> Signed-off-by: Mitsuru Kariya <Mitsuru.Kariya@oss.nttdata.com>
> ---
>  meson.build                    |  2 ++
>  qapi/sockets.json              |  7 +++++++
>  qemu-options.hx                | 13 +++++++++----
>  tests/unit/test-util-sockets.c | 13 +++++++++++++
>  util/qemu-sockets.c            | 24 ++++++++++++++++++++++++
>  5 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index eb07491819..c5afe8754d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2726,6 +2726,8 @@ config_host_data.set('HAVE_TCP_KEEPINTVL',
>                       #endif
>                       int main(void) { return 0; }''',
>                       name: 'Win32 TCP_KEEPINTVL'))
> +config_host_data.set('HAVE_TCP_USER_TIMEOUT',
> +                     cc.has_header_symbol('netinet/tcp.h', 'TCP_USER_TIMEOUT'))
>  
>  # has_member
>  config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',
> diff --git a/qapi/sockets.json b/qapi/sockets.json
> index 473be2ac58..543689a2c7 100644
> --- a/qapi/sockets.json
> +++ b/qapi/sockets.json
> @@ -79,6 +79,12 @@
>  #     defined (this includes Linux, Windows, macOS, FreeBSD, but not
>  #     OpenBSD).  When set to 0, system setting is used.  (Since 10.1)
>  #
> +# @user-timeout: time in milliseconds that transmitted data may remain
> +#     unacknowledged before the connection is closed.  Only supported
> +#     for TCP sockets on systems where TCP_USER_TIMEOUT socket option
> +#     is defined (Linux only).  When set to 0, system setting is used.
> +#     (Since 11.1)
> +#

Actually, it's "transmitted data may remain unacknowledged, or buffered
data may remain untransmitted".  Moreover, the interaction with
@keep-alive is not mentioned.  We want to keep the description
reasonably short, though.  Have it point to tcp(7) for details?

Perhaps something like

  # @user-timeout: time in milliseconds that transmitted data may remain
  #     unacknowledged, or buffered data may remain untransmitted before
  #     the connection is closed.  See tcp(7) for details.  Only
  #     supported when the host provides socket option TCP_USER_TIMEOUT
  #     (Linux).  0 means system default.  (default 0) (since 11.1)

What do you think?

>  # @mptcp: enable multi-path TCP.  (Since 6.1)
>  #
>  # Since: 1.3
> @@ -94,6 +100,7 @@
>      '*keep-alive-count': { 'type': 'uint32', 'if': 'HAVE_TCP_KEEPCNT' },
>      '*keep-alive-idle': { 'type': 'uint32', 'if': 'HAVE_TCP_KEEPIDLE' },
>      '*keep-alive-interval': { 'type': 'uint32', 'if': 'HAVE_TCP_KEEPINTVL' },
> +    '*user-timeout': { 'type': 'uint32', 'if': 'HAVE_TCP_USER_TIMEOUT' },
>      '*mptcp': { 'type': 'bool', 'if': 'HAVE_IPPROTO_MPTCP' } } }
>  
>  ##
> diff --git a/qemu-options.hx b/qemu-options.hx
> index d2b816e16f..63587cccd3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3034,7 +3034,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>      "-netdev socket,id=str[,fd=h][,udp=host:port][,localaddr=host:port]\n"
>      "                configure a network backend to connect to another network\n"
>      "                using an UDP tunnel\n"
> -    "-netdev stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port[,addr.to=maxport][,addr.numeric=on|off][,addr.keep-alive=on|off][,addr.keep-alive-count=count][,addr.keep-alive-idle=idle][,addr.keep-alive-interval=interval][,addr.mptcp=on|off][,addr.ipv4=on|off][,addr.ipv6=on|off][,reconnect-ms=milliseconds]\n"
> +    "-netdev stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port[,addr.to=maxport][,addr.numeric=on|off][,addr.keep-alive=on|off][,addr.keep-alive-count=count][,addr.keep-alive-idle=idle][,addr.keep-alive-interval=interval][,addr.user-timeout=timeout][,addr.mptcp=on|off][,addr.ipv4=on|off][,addr.ipv6=on|off][,reconnect-ms=milliseconds]\n"
>      "-netdev stream,id=str[,server=on|off],addr.type=unix,addr.path=path[,addr.abstract=on|off][,addr.tight=on|off][,reconnect-ms=milliseconds]\n"
>      "-netdev stream,id=str[,server=on|off],addr.type=fd,addr.str=file-descriptor[,reconnect-ms=milliseconds]\n"
>      "                configure a network backend to connect to another network\n"
> @@ -3640,7 +3640,7 @@ SRST
>                           -device e1000,netdev=n1,mac=52:54:00:12:34:56 \\
>                           -netdev socket,id=n1,mcast=239.192.168.1:1102,localaddr=1.2.3.4
>  
> -``-netdev stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port[,addr.to=maxport][,addr.numeric=on|off][,addr.keep-alive=on|off][,addr.keep-alive-count=count][,addr.keep-alive-idle=idle][,addr.keep-alive-interval=interval][,addr.mptcp=on|off][,addr.ipv4=on|off][,addr.ipv6=on|off][,reconnect-ms=milliseconds]``
> +``-netdev stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port[,addr.to=maxport][,addr.numeric=on|off][,addr.keep-alive=on|off][,addr.keep-alive-count=count][,addr.keep-alive-idle=idle][,addr.keep-alive-interval=interval][,addr.user-timeout=timeout][,addr.mptcp=on|off][,addr.ipv4=on|off][,addr.ipv6=on|off][,reconnect-ms=milliseconds]``
>      Configure a network backend to connect to another QEMU virtual machine or a proxy using a TCP/IP socket.
>  
>      ``server=on|off``
> @@ -3670,6 +3670,11 @@ SRST
>          time in seconds between individual keep-alive packets.
>          Set to 0 to use the system default.  (default: 0)
>  
> +    ``addr.user-timeout=timeout``
> +        time in milliseconds that transmitted data may remain unacknowledged
> +        before the connection is forcibly closed.
> +        Set to 0 to use the system default.  (default: 0)
> +

If you improve the description in net.json, update this one to match it.

>      ``addr.mptcp=on|off``
>          enable multipath TCP
>  
> @@ -5365,7 +5370,7 @@ SRST
>  ERST
>  
>  DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
> -    "-incoming tcp:[host]:port[,to=maxport][,numeric=on|off][,keep-alive=on|off][,keep-alive-count=count][,keep-alive-idle=idle][,keep-alive-interval=interval][,mptcp=on|off][,ipv4=on|off][,ipv6=on|off]\n" \
> +    "-incoming tcp:[host]:port[,to=maxport][,numeric=on|off][,keep-alive=on|off][,keep-alive-count=count][,keep-alive-idle=idle][,keep-alive-interval=interval][,user-timeout=timeout][,mptcp=on|off][,ipv4=on|off][,ipv6=on|off]\n" \
>      "-incoming rdma:host:port[,ipv4=on|off][,ipv6=on|off]\n" \
>      "-incoming unix:socketpath\n" \
>      "                prepare for incoming migration, listen on\n" \
> @@ -5387,7 +5392,7 @@ migration channel types.  The channel type is specified in <channel>,
>  or is 'main' for all other forms of -incoming.  If multiple -incoming
>  options are specified for a channel type, the last one takes precedence.
>  
> -``-incoming tcp:[host]:port[,to=maxport][,numeric=on|off][,keep-alive=on|off][,keep-alive-count=count][,keep-alive-idle=idle][,keep-alive-interval=interval][,mptcp=on|off][,ipv4=on|off][,ipv6=on|off]``
> +``-incoming tcp:[host]:port[,to=maxport][,numeric=on|off][,keep-alive=on|off][,keep-alive-count=count][,keep-alive-idle=idle][,keep-alive-interval=interval][,user-timeout=timeout][,mptcp=on|off][,ipv4=on|off][,ipv6=on|off]``
>    \ 
>  ``-incoming rdma:host:port[,ipv4=on|off][,ipv6=on|off]``
>      Prepare for incoming migration, listen on a given tcp port.

[...]