| Message ID | 20210415033925.1290401-3-dje@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for ipv6 host forwarding | expand |
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 > > >
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 >
On Tue, May 25, 2021 at 12:37:21PM -0700, Doug Evans wrote: > 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 > > ? Probably easier if we start from the HMP hostfwd_add command which takes hostfwd_add ::60022-:22 With that, adding the flags is obvious hostfwd_add ::60022-:22,ipv6=on|off,ipv4=on|off IIUC, that can be handled in the slirp_hostfwd() method impl. The question is then how this works on the CLI. IIUC ,the "hostfwd=XXX" ARG value is passed to slirp_hostfwd() eventually, so the change for the HMP parsing will "just work". The complication is that the comma is ambiguous between the --net arg parsing, and the hostfwd parsing. So you would end up having to escape the commas (ie replace , with ,,): --nic user,id=n1,model=e1000,hostfwd=::60022-:22,,ipv6=on,,ipv4=on If you forget to escape the commas, then the flag ends up applying to the --nic instead, where ipv4/ipv6 are indeed value for other reasons. This kind of sucks, but that's where we are with the old fashioned design of --nic parsing Not sure if someone else has better ideas here ? Regards, Daniel
On Wed, May 26, 2021 at 6:57 AM Daniel P. Berrangé <berrange@redhat.com> wrote: > On Tue, May 25, 2021 at 12:37:21PM -0700, Doug Evans wrote: > > 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 > > > > ? > > Probably easier if we start from the HMP hostfwd_add command which takes > > hostfwd_add ::60022-:22 > > With that, adding the flags is obvious > > hostfwd_add ::60022-:22,ipv6=on|off,ipv4=on|off > Data point: There's been discussion of supporting ipv6->ipv4 and ipv4->ipv6. If we want to provide for this then the ipv4/6 flags need to apply to the host/guess address. E.g., hostfwd_add ::60022,ipv6=on-:22,ipv4=on [for ipv6->ipv4] > IIUC, that can be handled in the slirp_hostfwd() method impl. > > The question is then how this works on the CLI. IIUC ,the "hostfwd=XXX" > ARG value is passed to slirp_hostfwd() eventually, so the change for > the HMP parsing will "just work". > > The complication is that the comma is ambiguous between the --net arg > parsing, and the hostfwd parsing. So you would end up having to escape > the commas (ie replace , with ,,): > > --nic user,id=n1,model=e1000,hostfwd=::60022-:22,,ipv6=on,,ipv4=on > > If you forget to escape the commas, then the flag ends up applying > to the --nic instead, where ipv4/ipv6 are indeed value for other > reasons. > > This kind of sucks, but that's where we are with the old fashioned > design of --nic parsing > > > Not sure if someone else has better ideas here ? > >
On Wed, May 26, 2021 at 08:26:33AM -0700, Doug Evans wrote: > On Wed, May 26, 2021 at 6:57 AM Daniel P. Berrangé <berrange@redhat.com> > wrote: > > > On Tue, May 25, 2021 at 12:37:21PM -0700, Doug Evans wrote: > > > 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 > > > > > > ? > > > > Probably easier if we start from the HMP hostfwd_add command which takes > > > > hostfwd_add ::60022-:22 > > > > With that, adding the flags is obvious > > > > hostfwd_add ::60022-:22,ipv6=on|off,ipv4=on|off > > > > > Data point: > > There's been discussion of supporting ipv6->ipv4 and ipv4->ipv6. > If we want to provide for this then the ipv4/6 flags need to apply to the > host/guess address. > > E.g., > > hostfwd_add ::60022,ipv6=on-:22,ipv4=on > [for ipv6->ipv4] Yes, good point, that would make sense. Regards, Daniel
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"),
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> --- 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(-)