RFC: IPv6 hostfwd command line syntax [was Re: [PATCH v6 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse]

From: Doug Evans <dje@google.com>
To: "Marc-André Lureau" <marcandre.lureau@gmail.com>,
	"Samuel Thibault" <samuel.thibault@ens-lyon.org>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Cc: QEMU <qemu-devel@nongnu.org>
Subject: RFC: IPv6 hostfwd command line syntax [was Re: [PATCH v6 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse]
Date: Tue, 25 May 2021 12:37:21 -0700	[thread overview]
Message-ID: <CADPb22TW5GkwcF_08R1KBr+NKBGLOHBwFqHPOJKVgiDa6w2VKA@mail.gmail.com> (raw)
In-Reply-To: <CAJ+F1CJUzAbZmfY59x6YYLWba-TMyZYwW7Pqu75zs93qbscr3Q@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5898 bytes --]

Hi.

I want to confirm the command line syntax y'all want for ipv6 host
forwarding.

IIUC, the command line syntax is required to be consistent with the use of
"ipv6=on|off" elsewhere.
Can you confirm that's correct?

If so, how does one apply "ipv6=on" to the "::60022-:22" hostfwd spec in
the following example:

$ qemu-system-x86_64 [...] --nic user,id=n1,model=e1000,hostfwd=::60022-:22

?

Square brackets are for parsing purposes only and are not allowed to be the
deciding factor in determining whether an address is ipv4 or ipv6. Thus
while one might think to write that as ":[]:60022-[]:22" to mean "this is
for ipv6" one cannot do so.

On Fri, May 7, 2021 at 8:23 AM Marc-André Lureau <marcandre.lureau@gmail.com>
wrote:

>
>
> On Thu, Apr 15, 2021 at 7:40 AM Doug Evans <dje@google.com> wrote:
>
>> The parsing is moved into new function inet_parse_host_port.
>> Also split out is ipv4=flag, ipv6=flag processing into inet_parse_ipv46.
>> This is done in preparation for using these functions in net/slirp.c.
>>
>> Signed-off-by: Doug Evans <dje@google.com>
>>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> ---
>>
>> Changes from v5:
>>
>> Also split out parsing of ipv4=on|off, ipv6=on|off
>>
>>  include/qemu/sockets.h |  3 ++
>>  util/qemu-sockets.c    | 65 +++++++++++++++++++++++++++++-------------
>>  2 files changed, 48 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
>> index 7d1f813576..94f4e8de83 100644
>> --- a/include/qemu/sockets.h
>> +++ b/include/qemu/sockets.h
>> @@ -31,6 +31,9 @@ int socket_set_fast_reuse(int fd);
>>
>>  int inet_ai_family_from_address(InetSocketAddress *addr,
>>                                  Error **errp);
>> +const char *inet_parse_host_port(InetSocketAddress *addr,
>> +                                 const char *str, Error **errp);
>> +int inet_parse_ipv46(InetSocketAddress *addr, const char *optstr, Error
>> **errp);
>>  int inet_parse(InetSocketAddress *addr, const char *str, Error **errp);
>>  int inet_connect(const char *str, Error **errp);
>>  int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);
>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
>> index 8af0278f15..c0069f2565 100644
>> --- a/util/qemu-sockets.c
>> +++ b/util/qemu-sockets.c
>> @@ -615,14 +615,12 @@ static int inet_parse_flag(const char *flagname,
>> const char *optstr, bool *val,
>>      return 0;
>>  }
>>
>> -int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
>> +const char *inet_parse_host_port(InetSocketAddress *addr, const char
>> *str,
>> +                                 Error **errp)
>>  {
>> -    const char *optstr, *h;
>>      char host[65];
>>      char port[33];
>> -    int to;
>>      int pos;
>> -    char *begin;
>>
>>      memset(addr, 0, sizeof(*addr));
>>
>> @@ -632,38 +630,32 @@ int inet_parse(InetSocketAddress *addr, const char
>> *str, Error **errp)
>>          host[0] = '\0';
>>          if (sscanf(str, ":%32[^,]%n", port, &pos) != 1) {
>>              error_setg(errp, "error parsing port in address '%s'", str);
>> -            return -1;
>> +            return NULL;
>>          }
>>      } else if (str[0] == '[') {
>>          /* IPv6 addr */
>>          if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos) != 2) {
>>              error_setg(errp, "error parsing IPv6 address '%s'", str);
>> -            return -1;
>> +            return NULL;
>>          }
>>      } else {
>>          /* hostname or IPv4 addr */
>>          if (sscanf(str, "%64[^:]:%32[^,]%n", host, port, &pos) != 2) {
>>              error_setg(errp, "error parsing address '%s'", str);
>> -            return -1;
>> +            return NULL;
>>          }
>>      }
>>
>>      addr->host = g_strdup(host);
>>      addr->port = g_strdup(port);
>>
>> -    /* parse options */
>> -    optstr = str + pos;
>> -    h = strstr(optstr, ",to=");
>> -    if (h) {
>> -        h += 4;
>> -        if (sscanf(h, "%d%n", &to, &pos) != 1 ||
>> -            (h[pos] != '\0' && h[pos] != ',')) {
>> -            error_setg(errp, "error parsing to= argument");
>> -            return -1;
>> -        }
>> -        addr->has_to = true;
>> -        addr->to = to;
>> -    }
>> +    return str + pos;
>> +}
>> +
>> +int inet_parse_ipv46(InetSocketAddress *addr, const char *optstr, Error
>> **errp)
>> +{
>> +    char *begin;
>> +
>>      begin = strstr(optstr, ",ipv4");
>>      if (begin) {
>>          if (inet_parse_flag("ipv4", begin + 5, &addr->ipv4, errp) < 0) {
>> @@ -678,6 +670,39 @@ int inet_parse(InetSocketAddress *addr, const char
>> *str, Error **errp)
>>          }
>>          addr->has_ipv6 = true;
>>      }
>> +
>> +    return 0;
>> +}
>> +
>> +int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
>> +{
>> +    const char *optstr, *h;
>> +    int to;
>> +    int pos;
>> +    char *begin;
>> +
>> +    optstr = inet_parse_host_port(addr, str, errp);
>> +    if (optstr == NULL) {
>> +        return -1;
>> +    }
>> +
>> +    /* parse options */
>> +
>> +    if (inet_parse_ipv46(addr, optstr, errp) < 0) {
>> +        return -1;
>> +    }
>> +
>> +    h = strstr(optstr, ",to=");
>> +    if (h) {
>> +        h += 4;
>> +        if (sscanf(h, "%d%n", &to, &pos) != 1 ||
>> +            (h[pos] != '\0' && h[pos] != ',')) {
>> +            error_setg(errp, "error parsing to= argument");
>> +            return -1;
>> +        }
>> +        addr->has_to = true;
>> +        addr->to = to;
>> +    }
>>      begin = strstr(optstr, ",keep-alive");
>>      if (begin) {
>>          if (inet_parse_flag("keep-alive", begin + strlen(",keep-alive"),
>> --
>> 2.31.1.295.g9ea45b61b8-goog
>>
>>
>>
>
> --
> Marc-André Lureau
>

[-- Attachment #2: Type: text/html, Size: 8613 bytes --]