Re: [RFC PATCH 5/5] sockets: Support multipath TCP

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Apr 08, 2021 at 08:11:59PM +0100, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> 
>> Multipath TCP allows combining multiple interfaces/routes into a single
>> socket, with very little work for the user/admin.
>> 
>> It's enabled by 'mptcp' on most socket addresses:
>> 
>>    ./qemu-system-x86_64 -nographic -incoming tcp:0:4444,mptcp
>> 
>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>  io/dns-resolver.c   |  2 ++
>>  qapi/sockets.json   |  5 ++++-
>>  util/qemu-sockets.c | 34 ++++++++++++++++++++++++++++++++++
>>  3 files changed, 40 insertions(+), 1 deletion(-)
>> 
>> diff --git a/io/dns-resolver.c b/io/dns-resolver.c
>> index 743a0efc87..b081e098bb 100644
>> --- a/io/dns-resolver.c
>> +++ b/io/dns-resolver.c
>> @@ -122,6 +122,8 @@ static int qio_dns_resolver_lookup_sync_inet(QIODNSResolver *resolver,
>>              .ipv4 = iaddr->ipv4,
>>              .has_ipv6 = iaddr->has_ipv6,
>>              .ipv6 = iaddr->ipv6,
>> +            .has_mptcp = iaddr->has_mptcp,
>> +            .mptcp = iaddr->mptcp,
>>          };
>>  
>>          (*addrs)[i] = newaddr;
>> diff --git a/qapi/sockets.json b/qapi/sockets.json
>> index 2e83452797..43122a38bf 100644
>> --- a/qapi/sockets.json
>> +++ b/qapi/sockets.json
>> @@ -57,6 +57,8 @@
>>  # @keep-alive: enable keep-alive when connecting to this socket. Not supported
>>  #              for passive sockets. (Since 4.2)
>>  #
>> +# @mptcp: enable multi-path TCP. (Since 6.0)
>> +#
>>  # Since: 1.3
>>  ##
>>  { 'struct': 'InetSocketAddress',
>> @@ -66,7 +68,8 @@
>>      '*to': 'uint16',
>>      '*ipv4': 'bool',
>>      '*ipv6': 'bool',
>> -    '*keep-alive': 'bool' } }
>> +    '*keep-alive': 'bool',
>> +    '*mptcp': 'bool' } }
>
> I think this would need to be
>
>    '*mptcp': { 'type': 'bool', 'if': 'IPPROTO_MPTCP' }
>
> so that mgmt apps can probe when it truely is supported or not for
> this build

Yes.  Instance of a somewhat common anti-pattern "declare
unconditionally (this hunk), write unconditionally (previous hunk), read
conditionally (next hunk).  Besides defeating introspection, it also
exposes configuration knobs that don't do anything.

>>  
>>  ##
>>  # @UnixSocketAddress:
>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
>> index 8af0278f15..72527972d5 100644
>> --- a/util/qemu-sockets.c
>> +++ b/util/qemu-sockets.c
>> @@ -206,6 +206,21 @@ static int try_bind(int socket, InetSocketAddress *saddr, struct addrinfo *e)
>>  #endif
>>  }
>>  
>> +static int check_mptcp(const InetSocketAddress *saddr, struct addrinfo *ai,
>> +                       Error **errp)
>> +{
>> +    if (saddr->has_mptcp && saddr->mptcp) {
>> +#ifdef IPPROTO_MPTCP
>> +        ai->ai_protocol = IPPROTO_MPTCP;
>> +#else
>> +        error_setg(errp, "MPTCP unavailable in this build");
>> +        return -1;
>> +#endif
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>  static int inet_listen_saddr(InetSocketAddress *saddr,
>>                               int port_offset,
>>                               int num,
[...]