qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/5] Add support for ipv6 host forwarding
@ 2021-02-20  0:13 Doug Evans via
  2021-02-20  0:13 ` [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support Doug Evans via
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Doug Evans via @ 2021-02-20  0:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Samuel Thibault, Daniel P . Berrangé, Doug Evans

This patchset takes the original patch from Maxim,
https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html
and updates it.

Option hostfwd is extended to support ipv6 addresses.
Commands hostfwd_add, hostfwd_remove are extended as well.

The libslirp part of the patch has been committed upstream,
and is now in qemu. See patch 1/5.

Changes from v4:

1/5 slirp: Advance libslirp submodule to add ipv6 host-forward support
NOTE TO REVIEWERS: I need some hand-holding to know what The Right
way to submit this particular patch is.

- no change

2/5 util/qemu-sockets.c: Split host:port parsing out of inet_parse

- move recognition of "[]:port" to separate patch
- allow passing NULL for ip_v6
- fix some formatting issues

3/5 inet_parse_host_and_addr: Recognize []:port (empty ipv6 address)

- new in this patchset revision

4/5 net/slirp.c: Refactor address parsing

- was 3/4 in v4
- fix some formatting issues

5/5 net: Extend host forwarding to support IPv6

- was 4/4 in v4
- fix some formatting issues

Changes from v3:

1/4 slirp: Advance libslirp submodule to add ipv6 host-forward support

- pick up latest libslirp patch to reject ipv6 addr-any for guest address
  - libslirp currently only provides a stateless DHCPv6 server, which means
    it can't know in advance what the guest's IP address is, and thus
    cannot do the "addr-any -> guest ip address" translation that is done
    for ipv4

2/4 util/qemu-sockets.c: Split host:port parsing out of inet_parse

- this patch is new in v4
  - provides new utility: inet_parse_host_and_port, updates inet_parse
    to use it

3/4 net/slirp.c: Refactor address parsing

- this patch renamed from 2/3 to 3/4
- call inet_parse_host_and_port from util/qemu-sockets.c
- added tests/acceptance/hostfwd.py

4/4 net: Extend host forwarding to support IPv6

- this patch renamed from 3/3 to 4/4
- ipv6 support added to existing hostfwd option, commands
  - instead of creating new ipv6 option, commands
- added tests to tests/acceptance/hostfwd.py

Changes from v2:
- split out libslirp commit
- clarify spelling of ipv6 addresses in docs
- tighten parsing of ipv6 addresses

Change from v1:
- libslirp part is now upstream
- net/slirp.c changes split into two pieces (refactor, add ipv6)
- added docs

Doug Evans (5):
  slirp: Advance libslirp submodule to add ipv6 host-forward support
  util/qemu-sockets.c: Split host:port parsing out of inet_parse
  inet_parse_host_and_addr: Recognize []:port (empty ipv6 address)
  net/slirp.c: Refactor address parsing
  net: Extend host forwarding to support IPv6

 hmp-commands.hx             |  15 +++
 include/qemu/sockets.h      |   3 +
 net/slirp.c                 | 196 ++++++++++++++++++++++++------------
 slirp                       |   2 +-
 tests/acceptance/hostfwd.py | 174 ++++++++++++++++++++++++++++++++
 util/qemu-sockets.c         |  84 ++++++++++++----
 6 files changed, 390 insertions(+), 84 deletions(-)
 create mode 100644 tests/acceptance/hostfwd.py

-- 
2.30.0.617.g56c4b15f3c-goog



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support
  2021-02-20  0:13 [PATCH v5 0/5] Add support for ipv6 host forwarding Doug Evans via
@ 2021-02-20  0:13 ` Doug Evans via
  2021-02-20  0:20   ` Philippe Mathieu-Daudé
  2021-02-20  0:13 ` [PATCH v5 2/5] util/qemu-sockets.c: Split host:port parsing out of inet_parse Doug Evans via
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Doug Evans via @ 2021-02-20  0:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Samuel Thibault, Daniel P . Berrangé, Doug Evans

Signed-off-by: Doug Evans <dje@google.com>
---

Changes from v4:
NOTE TO REVIEWERS: I need some hand-holding to know what The Right
way to submit this particular patch is.

- no change

Changes from v3:
- pick up latest libslirp patch to reject ipv6 addr-any for guest address
  - libslirp currently only provides a stateless DHCPv6 server, which means
    it can't know in advance what the guest's IP address is, and thus
    cannot do the "addr-any -> guest ip address" translation that is done
    for ipv4

Changes from v2:
- this patch is new in v3, split out from v2

 slirp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp b/slirp
index 8f43a99191..26ae658a83 160000
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
+Subproject commit 26ae658a83eeca16780cf5615c8247cbb151c3fa
-- 
2.30.0.617.g56c4b15f3c-goog



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 2/5] util/qemu-sockets.c: Split host:port parsing out of inet_parse
  2021-02-20  0:13 [PATCH v5 0/5] Add support for ipv6 host forwarding Doug Evans via
  2021-02-20  0:13 ` [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support Doug Evans via
@ 2021-02-20  0:13 ` Doug Evans via
  2021-02-20  0:13 ` [PATCH v5 3/5] inet_parse_host_and_addr: Recognize []:port (empty ipv6 address) Doug Evans via
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Doug Evans via @ 2021-02-20  0:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Samuel Thibault, Daniel P . Berrangé, Doug Evans

The parsing is moved into new function inet_parse_host_and_port.
This is done in preparation for using the function in net/slirp.c.

Signed-off-by: Doug Evans <dje@google.com>
---

Changes from v4:
- move recognition of "[]:port" to separate patch
- allow passing NULL for ip_v6
- fix some formatting issues

Changes from v3:
- this patch is new in v4
  - provides new utility: inet_parse_host_and_port, updates inet_parse
    to use it

 include/qemu/sockets.h |  3 ++
 util/qemu-sockets.c    | 80 +++++++++++++++++++++++++++++++-----------
 2 files changed, 62 insertions(+), 21 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 7d1f813576..b1448cfa24 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_and_port(const char *str, int terminator,
+                                     char **hostp, char **portp, bool *is_v6,
+                                     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..3ca6a6fb3d 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -615,44 +615,82 @@ 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)
+/*
+ * Parse an inet host and port as "host:port<terminator>".
+ * Terminator may be '\0'.
+ * The syntax for IPv4 addresses is: address:port. "address" is optional,
+ * and may be empty (i.e., str is ":port").
+ * The syntax for IPv6 addresses is: [address]:port. Upon return the wrapping
+ * [] brackets are removed.
+ * Host names are also supported as hostname:port. It is up to the caller to
+ * distinguish host names from numeric IPv4 addresses.
+ * On success, returns a pointer to the terminator. Space for the address and
+ * port is malloced and stored in *host, *port, the caller must free.
+ * If is_v6 is non-NULL, then it is set to true if the address is an IPv6
+ * address (i.e., [address]), otherwise it is set to false.
+ * On failure NULL is returned with the error stored in *errp.
+ */
+const char *inet_parse_host_and_port(const char *str, int terminator,
+                                     char **hostp, char **portp, bool *is_v6,
+                                     Error **errp)
 {
-    const char *optstr, *h;
+    const char *terminator_ptr = strchr(str, terminator);
+    g_autofree char *buf = NULL;
     char host[65];
     char port[33];
-    int to;
-    int pos;
-    char *begin;
 
-    memset(addr, 0, sizeof(*addr));
+    if (terminator_ptr == NULL) {
+        /* If the terminator isn't found then use the entire string. */
+        terminator_ptr = str + strlen(str);
+    }
+    buf = g_strndup(str, terminator_ptr - str);
 
-    /* parse address */
-    if (str[0] == ':') {
+    if (buf[0] == ':') {
         /* no host given */
         host[0] = '\0';
-        if (sscanf(str, ":%32[^,]%n", port, &pos) != 1) {
-            error_setg(errp, "error parsing port in address '%s'", str);
-            return -1;
+        if (sscanf(buf, ":%32s", port) != 1) {
+            error_setg(errp, "error parsing port in address '%s'", buf);
+            return NULL;
         }
-    } else if (str[0] == '[') {
+    } else if (buf[0] == '[') {
         /* IPv6 addr */
-        if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos) != 2) {
-            error_setg(errp, "error parsing IPv6 address '%s'", str);
-            return -1;
+        if (sscanf(buf, "[%64[^]]]:%32s", host, port) != 2) {
+            error_setg(errp, "error parsing IPv6 address '%s'", buf);
+            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;
+        if (sscanf(buf, "%64[^:]:%32s", host, port) != 2) {
+            error_setg(errp, "error parsing address '%s'", buf);
+            return NULL;
         }
     }
 
-    addr->host = g_strdup(host);
-    addr->port = g_strdup(port);
+    *hostp = g_strdup(host);
+    *portp = g_strdup(port);
+    if (is_v6 != NULL) {
+        *is_v6 = buf[0] == '[';
+    }
+
+    return terminator_ptr;
+}
+
+int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
+{
+    const char *optstr, *h;
+    int to;
+    int pos;
+    char *begin;
+
+    memset(addr, 0, sizeof(*addr));
+
+    optstr = inet_parse_host_and_port(str, ',', &addr->host, &addr->port,
+                                      NULL, errp);
+    if (optstr == NULL) {
+        return -1;
+    }
 
     /* parse options */
-    optstr = str + pos;
     h = strstr(optstr, ",to=");
     if (h) {
         h += 4;
-- 
2.30.0.617.g56c4b15f3c-goog



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 3/5] inet_parse_host_and_addr: Recognize []:port (empty ipv6 address)
  2021-02-20  0:13 [PATCH v5 0/5] Add support for ipv6 host forwarding Doug Evans via
  2021-02-20  0:13 ` [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support Doug Evans via
  2021-02-20  0:13 ` [PATCH v5 2/5] util/qemu-sockets.c: Split host:port parsing out of inet_parse Doug Evans via
@ 2021-02-20  0:13 ` Doug Evans via
  2021-02-20  0:13 ` [PATCH v5 4/5] net/slirp.c: Refactor address parsing Doug Evans via
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Doug Evans via @ 2021-02-20  0:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Samuel Thibault, Daniel P . Berrangé, Doug Evans

Some callers need to distinguish empty ipv4 addresses from ipv6.

Signed-off-by: Doug Evans <dje@google.com>
---

Changes from v4:
- new in this patchset revision

 util/qemu-sockets.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 3ca6a6fb3d..062f0eb074 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -620,7 +620,8 @@ static int inet_parse_flag(const char *flagname, const char *optstr, bool *val,
  * Terminator may be '\0'.
  * The syntax for IPv4 addresses is: address:port. "address" is optional,
  * and may be empty (i.e., str is ":port").
- * The syntax for IPv6 addresses is: [address]:port. Upon return the wrapping
+ * The syntax for IPv6 addresses is: [address]:port. "address" is optional,
+ * and may be empty (i.e., str is "[]:port"). Upon return the wrapping
  * [] brackets are removed.
  * Host names are also supported as hostname:port. It is up to the caller to
  * distinguish host names from numeric IPv4 addresses.
@@ -654,7 +655,10 @@ const char *inet_parse_host_and_port(const char *str, int terminator,
         }
     } else if (buf[0] == '[') {
         /* IPv6 addr */
-        if (sscanf(buf, "[%64[^]]]:%32s", host, port) != 2) {
+        /* Note: sscanf %[ doesn't recognize empty contents. */
+        if (sscanf(buf, "[]:%32s", port) == 1) {
+            host[0] = '\0';
+        } else if (sscanf(buf, "[%64[^]]]:%32s", host, port) != 2) {
             error_setg(errp, "error parsing IPv6 address '%s'", buf);
             return NULL;
         }
-- 
2.30.0.617.g56c4b15f3c-goog



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 4/5] net/slirp.c: Refactor address parsing
  2021-02-20  0:13 [PATCH v5 0/5] Add support for ipv6 host forwarding Doug Evans via
                   ` (2 preceding siblings ...)
  2021-02-20  0:13 ` [PATCH v5 3/5] inet_parse_host_and_addr: Recognize []:port (empty ipv6 address) Doug Evans via
@ 2021-02-20  0:13 ` Doug Evans via
  2021-02-20  0:13 ` [PATCH v5 5/5] net: Extend host forwarding to support IPv6 Doug Evans via
  2021-02-20  0:27 ` [PATCH v5 0/5] Add support for ipv6 host forwarding no-reply
  5 siblings, 0 replies; 10+ messages in thread
From: Doug Evans via @ 2021-02-20  0:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Samuel Thibault, Daniel P . Berrangé, Doug Evans

... in preparation for adding ipv6 host forwarding support.

New test: avocado run tests/acceptance/hostfwd.py

Signed-off-by: Doug Evans <dje@google.com>
---

Changes from v4:
- was 3/4 in v4
- fix some formatting issues

Changes from v3:
- this patch renamed from 2/3 to 3/4
- call inet_parse_host_and_port from util/qemu-sockets.c
- added tests/acceptance/hostfwd.py

Changes from v2:
- nothing of consequence

Changes from v1:
- this patch is new in v2
  - address parsing refactor split out, ipv4 changes here
- libslirp part is now upstream in libslirp repo

 net/slirp.c                 | 165 ++++++++++++++++++++++--------------
 tests/acceptance/hostfwd.py |  94 ++++++++++++++++++++
 2 files changed, 196 insertions(+), 63 deletions(-)
 create mode 100644 tests/acceptance/hostfwd.py

diff --git a/net/slirp.c b/net/slirp.c
index be914c0be0..e0284492b9 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -631,15 +631,91 @@ static SlirpState *slirp_lookup(Monitor *mon, const char *id)
     }
 }
 
+/*
+ * Parse a protocol name of the form "name<sep>".
+ * Valid protocols are "tcp" and "udp". An empty string means "tcp".
+ * Returns a pointer to the end of the parsed string on success, and stores
+ * the result in *is_udp.
+ * Otherwise returns NULL and stores the error in *errp.
+ */
+static const char *parse_protocol(const char *str, int sep, bool *is_udp,
+                                  Error **errp)
+{
+    char buf[10];
+    const char *p = str;
+
+    if (get_str_sep(buf, sizeof(buf), &p, sep) < 0) {
+        error_setg(errp, "Missing protocol name separator");
+        return NULL;
+    }
+
+    if (!strcmp(buf, "tcp") || buf[0] == '\0') {
+        *is_udp = false;
+    } else if (!strcmp(buf, "udp")) {
+        *is_udp = true;
+    } else {
+        error_setg(errp, "Bad protocol name");
+        return NULL;
+    }
+
+    return p;
+}
+
+/*
+ * Parse an ip address/port of the form "address:port<terminator>".
+ * An empty address means INADDR_ANY.
+ * Returns a pointer to after the terminator, unless it was '\0' in which case
+ * the result points to the '\0'.
+ * The parsed results are stored in *addr, *port.
+ * On error NULL is returned and stores the error in *errp.
+ */
+static const char *parse_ip_addr_and_port(const char *str, int terminator,
+                                          struct in_addr *addr, int *port,
+                                          Error **errp)
+{
+    g_autofree char *addr_str = NULL;
+    g_autofree char *port_str = NULL;
+    bool is_v6;
+    const char *p = inet_parse_host_and_port(str, terminator, &addr_str,
+                                             &port_str, &is_v6, errp);
+
+    if (p == NULL) {
+        return NULL;
+    }
+
+    /* Ignore is_v6 for the moment, if inet_aton fails let it. */
+    if (addr_str[0] == '\0') {
+        addr->s_addr = INADDR_ANY;
+    } else if (!inet_aton(addr_str, addr)) {
+        error_setg(errp, "Bad address");
+        return NULL;
+    }
+
+    if (qemu_strtoi(port_str, NULL, 10, port) < 0 ||
+        *port < 0 || *port > 65535) {
+        error_setg(errp, "Bad port");
+        return NULL;
+    }
+
+    /*
+     * At this point "p" points to the terminator or trailing NUL if the
+     * terminator is not present.
+     */
+    if (*p) {
+        ++p;
+    }
+    return p;
+}
+
 void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 {
-    struct in_addr host_addr = { .s_addr = INADDR_ANY };
+    struct in_addr host_addr;
     int host_port;
-    char buf[256];
     const char *src_str, *p;
     SlirpState *s;
-    int is_udp = 0;
+    bool is_udp;
     int err;
+    Error *error = NULL;
     const char *arg1 = qdict_get_str(qdict, "arg1");
     const char *arg2 = qdict_get_try_str(qdict, "arg2");
 
@@ -654,30 +730,18 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
         return;
     }
 
+    g_assert(src_str != NULL);
     p = src_str;
-    if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
-        goto fail_syntax;
-    }
-
-    if (!strcmp(buf, "tcp") || buf[0] == '\0') {
-        is_udp = 0;
-    } else if (!strcmp(buf, "udp")) {
-        is_udp = 1;
-    } else {
-        goto fail_syntax;
-    }
 
-    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
-        goto fail_syntax;
-    }
-    if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
+    p = parse_protocol(p, ':', &is_udp, &error);
+    if (p == NULL) {
         goto fail_syntax;
     }
 
-    if (qemu_strtoi(p, NULL, 10, &host_port)) {
+    if (parse_ip_addr_and_port(p, '\0', &host_addr, &host_port,
+                               &error) == NULL) {
         goto fail_syntax;
     }
-
     err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port);
 
     monitor_printf(mon, "host forwarding rule for %s %s\n", src_str,
@@ -685,65 +749,39 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
     return;
 
  fail_syntax:
-    monitor_printf(mon, "invalid format\n");
+    monitor_printf(mon, "Invalid format: %s\n", error_get_pretty(error));
+    error_free(error);
 }
 
 static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
 {
-    struct in_addr host_addr = { .s_addr = INADDR_ANY };
-    struct in_addr guest_addr = { .s_addr = 0 };
+    struct in_addr host_addr, guest_addr;
     int host_port, guest_port;
     const char *p;
-    char buf[256];
-    int is_udp;
-    char *end;
-    const char *fail_reason = "Unknown reason";
+    bool is_udp;
+    Error *error = NULL;
 
+    g_assert(redir_str != NULL);
     p = redir_str;
-    if (!p || get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
-        fail_reason = "No : separators";
-        goto fail_syntax;
-    }
-    if (!strcmp(buf, "tcp") || buf[0] == '\0') {
-        is_udp = 0;
-    } else if (!strcmp(buf, "udp")) {
-        is_udp = 1;
-    } else {
-        fail_reason = "Bad protocol name";
-        goto fail_syntax;
-    }
 
-    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
-        fail_reason = "Missing : separator";
-        goto fail_syntax;
-    }
-    if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
-        fail_reason = "Bad host address";
+    p = parse_protocol(p, ':', &is_udp, &error);
+    if (p == NULL) {
         goto fail_syntax;
     }
 
-    if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
-        fail_reason = "Bad host port separator";
-        goto fail_syntax;
-    }
-    host_port = strtol(buf, &end, 0);
-    if (*end != '\0' || host_port < 0 || host_port > 65535) {
-        fail_reason = "Bad host port";
+    p = parse_ip_addr_and_port(p, '-', &host_addr, &host_port, &error);
+    if (p == NULL) {
+        error_prepend(&error, "For host address: ");
         goto fail_syntax;
     }
 
-    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
-        fail_reason = "Missing guest address";
+    if (parse_ip_addr_and_port(p, '\0', &guest_addr, &guest_port,
+                               &error) == NULL) {
+        error_prepend(&error, "For guest address: ");
         goto fail_syntax;
     }
-    if (buf[0] != '\0' && !inet_aton(buf, &guest_addr)) {
-        fail_reason = "Bad guest address";
-        goto fail_syntax;
-    }
-
-    guest_port = strtol(p, &end, 0);
-    if (*end != '\0' || guest_port < 1 || guest_port > 65535) {
-        fail_reason = "Bad guest port";
+    if (guest_port == 0) {
+        error_setg(&error, "For guest address: Bad port");
         goto fail_syntax;
     }
 
@@ -757,7 +795,8 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
 
  fail_syntax:
     error_setg(errp, "Invalid host forwarding rule '%s' (%s)", redir_str,
-               fail_reason);
+               error_get_pretty(error));
+    error_free(error);
     return -1;
 }
 
diff --git a/tests/acceptance/hostfwd.py b/tests/acceptance/hostfwd.py
new file mode 100644
index 0000000000..e5602a7708
--- /dev/null
+++ b/tests/acceptance/hostfwd.py
@@ -0,0 +1,94 @@
+# Hostfwd command tests
+#
+# Copyright 2021 Google LLC
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by the
+# Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+# for more details.
+
+
+from avocado_qemu import Test
+
+
+class Hostfwd(Test):
+    """
+    :avocado: tags=hostfwd
+    """
+    def hmc(self, cmd):
+        return self.vm.command('human-monitor-command', command_line=cmd)
+
+    def test_qmp_hostfwd_ipv4(self):
+        self.vm.add_args('-nodefaults',
+                         '-netdev', 'user,id=vnet',
+                         '-device', 'virtio-net,netdev=vnet')
+        self.vm.launch()
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022-:22'), '')
+        self.assertEquals(self.hmc('hostfwd_remove vnet tcp::65022'),
+                          'host forwarding rule for tcp::65022 removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_add tcp::65022-:22'), '')
+        self.assertEquals(self.hmc('hostfwd_remove tcp::65022'),
+                          'host forwarding rule for tcp::65022 removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'), '')
+        self.assertEquals(self.hmc('hostfwd_remove udp::65042'),
+                          'host forwarding rule for udp::65042 removed\r\n')
+
+    def test_qmp_hostfwd_ipv4_functional_errors(self):
+        """Verify handling of various kinds of errors given valid addresses."""
+        self.vm.add_args('-nodefaults',
+                         '-netdev', 'user,id=vnet',
+                         '-device', 'virtio-net,netdev=vnet')
+        self.vm.launch()
+        self.assertEquals(self.hmc('hostfwd_remove ::65022'),
+                          'host forwarding rule for ::65022 not found\r\n')
+        self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'), '')
+        self.assertEquals(self.hmc('hostfwd_add udp::65042-:42'),
+                          "Could not set up host forwarding rule "
+                          "'udp::65042-:42'\r\n")
+        self.assertEquals(self.hmc('hostfwd_remove ::65042'),
+                          'host forwarding rule for ::65042 not found\r\n')
+        self.assertEquals(self.hmc('hostfwd_remove udp::65042'),
+                          'host forwarding rule for udp::65042 removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_remove udp::65042'),
+                          'host forwarding rule for udp::65042 not found\r\n')
+
+    def test_qmp_hostfwd_ipv4_parsing_errors(self):
+        """Verify handling of various kinds of parsing errors."""
+        self.vm.add_args('-nodefaults',
+                         '-netdev', 'user,id=vnet',
+                         '-device', 'virtio-net,netdev=vnet')
+        self.vm.launch()
+        self.assertEquals(self.hmc('hostfwd_remove abc::42'),
+                          'Invalid format: Bad protocol name\r\n')
+        self.assertEquals(self.hmc('hostfwd_add abc::65022-:22'),
+                          "Invalid host forwarding rule 'abc::65022-:22' "
+                          "(Bad protocol name)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :a.b.c.d:66-:66'),
+                          "Invalid host forwarding rule ':a.b.c.d:66-:66' "
+                          "(For host address: Bad address)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add ::66-a.b.c.d:66'),
+                          "Invalid host forwarding rule '::66-a.b.c.d:66' "
+                          "(For guest address: Bad address)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add ::66666-:66666'),
+                          "Invalid host forwarding rule '::66666-:66666' "
+                          "(For host address: Bad port)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add ::-1-foo'),
+                          "Invalid host forwarding rule '::-1-foo' (For "
+                          "host address: error parsing port in address ':')\r\n")
+        self.assertEquals(self.hmc('hostfwd_add ::66-foo'),
+                          "Invalid host forwarding rule '::66-foo' (For "
+                          "guest address: error parsing address 'foo')\r\n")
+        self.assertEquals(self.hmc('hostfwd_add ::66-:66666'),
+                          "Invalid host forwarding rule '::66-:66666' "
+                          "(For guest address: Bad port)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add ::66-:-1'),
+                          "Invalid host forwarding rule '::66-:-1' "
+                          "(For guest address: Bad port)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add ::66-:0'),
+                          "Invalid host forwarding rule '::66-:0' "
+                          "(For guest address: Bad port)\r\n")
-- 
2.30.0.617.g56c4b15f3c-goog



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 5/5] net: Extend host forwarding to support IPv6
  2021-02-20  0:13 [PATCH v5 0/5] Add support for ipv6 host forwarding Doug Evans via
                   ` (3 preceding siblings ...)
  2021-02-20  0:13 ` [PATCH v5 4/5] net/slirp.c: Refactor address parsing Doug Evans via
@ 2021-02-20  0:13 ` Doug Evans via
  2021-02-20  0:27 ` [PATCH v5 0/5] Add support for ipv6 host forwarding no-reply
  5 siblings, 0 replies; 10+ messages in thread
From: Doug Evans via @ 2021-02-20  0:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Samuel Thibault, Daniel P . Berrangé, Doug Evans

Net option "-hostfwd" now supports IPv6 addresses.
Commands hostfwd_add, hostfwd_remove now support IPv6 addresses.

Signed-off-by: Doug Evans <dje@google.com>
---

Changes from v4:
- was 4/4 in v4
- fix some formatting issues

Differences from v3:
- this patch renamed from 3/3 to 4/4
- ipv6 support added to existing hostfwd option, commands
  - instead of creating new ipv6 option, commands
- added tests to tests/acceptance/hostfwd.py

Differences from v2:
- clarify spelling of ipv6 addresses in docs
- tighten parsing of ipv6 addresses

Differences from v1:
- parsing refactor split out into separate patch (2/3)

 hmp-commands.hx             | 15 +++++++
 net/slirp.c                 | 77 +++++++++++++++++++++++++----------
 tests/acceptance/hostfwd.py | 80 +++++++++++++++++++++++++++++++++++++
 3 files changed, 150 insertions(+), 22 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d4001f9c5d..4de4e4979d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1375,6 +1375,16 @@ ERST
 SRST
 ``hostfwd_add``
   Redirect TCP or UDP connections from host to guest (requires -net user).
+  IPV6 addresses are wrapped in square brackes, IPV4 addresses are not.
+
+  Examples:
+  hostfwd_add net0 tcp:127.0.0.1:10022-:22
+  hostfwd_add net0 tcp:[::1]:10022-[fe80::1:2:3:4]:22
+
+  Note that Libslirp currently only provides a "stateless" DHCPv6 server, a
+  consequence of which is that it cannot do the "addr-any" translation to the
+  guest address that is done for IPv4. In other words, the following is
+  currently not supported: hostfwd_add net0 tcp:[::1]:10022-:22
 ERST
 
 #ifdef CONFIG_SLIRP
@@ -1390,6 +1400,11 @@ ERST
 SRST
 ``hostfwd_remove``
   Remove host-to-guest TCP or UDP redirection.
+  IPV6 addresses are wrapped in square brackes, IPV4 addresses are not.
+
+  Examples:
+  hostfwd_remove net0 tcp:127.0.0.1:10022
+  hostfwd_remove net0 tcp:[::1]:10022
 ERST
 
     {
diff --git a/net/slirp.c b/net/slirp.c
index e0284492b9..32df65c1f0 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -96,6 +96,11 @@ typedef struct SlirpState {
     GSList *fwd;
 } SlirpState;
 
+union in4or6_addr {
+    struct in_addr addr4;
+    struct in6_addr addr6;
+};
+
 static struct slirp_config_str *slirp_configs;
 static QTAILQ_HEAD(, SlirpState) slirp_stacks =
     QTAILQ_HEAD_INITIALIZER(slirp_stacks);
@@ -663,32 +668,40 @@ static const char *parse_protocol(const char *str, int sep, bool *is_udp,
 
 /*
  * Parse an ip address/port of the form "address:port<terminator>".
- * An empty address means INADDR_ANY.
+ * IPv6 addresses are wrapped in [] brackets.
+ * An empty address means INADDR_ANY/in6addr_any.
  * Returns a pointer to after the terminator, unless it was '\0' in which case
  * the result points to the '\0'.
- * The parsed results are stored in *addr, *port.
+ * The parsed results are stored in *addr, *port, *is_v6.
  * On error NULL is returned and stores the error in *errp.
  */
 static const char *parse_ip_addr_and_port(const char *str, int terminator,
-                                          struct in_addr *addr, int *port,
-                                          Error **errp)
+                                          union in4or6_addr *addr, int *port,
+                                          bool *is_v6, Error **errp)
 {
     g_autofree char *addr_str = NULL;
     g_autofree char *port_str = NULL;
-    bool is_v6;
     const char *p = inet_parse_host_and_port(str, terminator, &addr_str,
-                                             &port_str, &is_v6, errp);
+                                             &port_str, is_v6, errp);
 
     if (p == NULL) {
         return NULL;
     }
 
-    /* Ignore is_v6 for the moment, if inet_aton fails let it. */
-    if (addr_str[0] == '\0') {
-        addr->s_addr = INADDR_ANY;
-    } else if (!inet_aton(addr_str, addr)) {
-        error_setg(errp, "Bad address");
-        return NULL;
+    if (*is_v6) {
+        if (addr_str[0] == '\0') {
+            addr->addr6 = in6addr_any;
+        } else if (!inet_pton(AF_INET6, addr_str, &addr->addr6)) {
+            error_setg(errp, "Bad address");
+            return NULL;
+        }
+    } else {
+        if (addr_str[0] == '\0') {
+            addr->addr4.s_addr = INADDR_ANY;
+        } else if (!inet_pton(AF_INET, addr_str, &addr->addr4)) {
+            error_setg(errp, "Bad address");
+            return NULL;
+        }
     }
 
     if (qemu_strtoi(port_str, NULL, 10, port) < 0 ||
@@ -709,11 +722,11 @@ static const char *parse_ip_addr_and_port(const char *str, int terminator,
 
 void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 {
-    struct in_addr host_addr;
+    union in4or6_addr host_addr;
     int host_port;
     const char *src_str, *p;
     SlirpState *s;
-    bool is_udp;
+    bool is_udp, is_v6;
     int err;
     Error *error = NULL;
     const char *arg1 = qdict_get_str(qdict, "arg1");
@@ -738,11 +751,18 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
         goto fail_syntax;
     }
 
-    if (parse_ip_addr_and_port(p, '\0', &host_addr, &host_port,
+    if (parse_ip_addr_and_port(p, '\0', &host_addr, &host_port, &is_v6,
                                &error) == NULL) {
         goto fail_syntax;
     }
-    err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port);
+
+    if (is_v6) {
+        err = slirp_remove_ipv6_hostfwd(s->slirp, is_udp, host_addr.addr6,
+                                        host_port);
+    } else {
+        err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr.addr4,
+                                   host_port);
+    }
 
     monitor_printf(mon, "host forwarding rule for %s %s\n", src_str,
                    err ? "not found" : "removed");
@@ -755,11 +775,12 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 
 static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
 {
-    struct in_addr host_addr, guest_addr;
+    union in4or6_addr host_addr, guest_addr;
     int host_port, guest_port;
     const char *p;
-    bool is_udp;
+    bool is_udp, host_is_v6, guest_is_v6;
     Error *error = NULL;
+    int err;
 
     g_assert(redir_str != NULL);
     p = redir_str;
@@ -769,24 +790,36 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
         goto fail_syntax;
     }
 
-    p = parse_ip_addr_and_port(p, '-', &host_addr, &host_port, &error);
+    p = parse_ip_addr_and_port(p, '-', &host_addr, &host_port, &host_is_v6,
+                               &error);
     if (p == NULL) {
         error_prepend(&error, "For host address: ");
         goto fail_syntax;
     }
 
-    if (parse_ip_addr_and_port(p, '\0', &guest_addr, &guest_port,
+    if (parse_ip_addr_and_port(p, '\0', &guest_addr, &guest_port, &guest_is_v6,
                                &error) == NULL) {
         error_prepend(&error, "For guest address: ");
         goto fail_syntax;
     }
+    if (host_is_v6 != guest_is_v6) {
+        /* TODO: Can libslirp support this? */
+        error_setg(&error, "Both host,guest must be one of ipv4 or ipv6");
+        goto fail_syntax;
+    }
     if (guest_port == 0) {
         error_setg(&error, "For guest address: Bad port");
         goto fail_syntax;
     }
 
-    if (slirp_add_hostfwd(s->slirp, is_udp, host_addr, host_port, guest_addr,
-                          guest_port) < 0) {
+    if (host_is_v6) {
+        err = slirp_add_ipv6_hostfwd(s->slirp, is_udp, host_addr.addr6,
+                                     host_port, guest_addr.addr6, guest_port);
+    } else {
+        err = slirp_add_hostfwd(s->slirp, is_udp, host_addr.addr4, host_port,
+                                guest_addr.addr4, guest_port);
+    }
+    if (err < 0) {
         error_setg(errp, "Could not set up host forwarding rule '%s'",
                    redir_str);
         return -1;
diff --git a/tests/acceptance/hostfwd.py b/tests/acceptance/hostfwd.py
index e5602a7708..d4c4b7b86d 100644
--- a/tests/acceptance/hostfwd.py
+++ b/tests/acceptance/hostfwd.py
@@ -92,3 +92,83 @@ def test_qmp_hostfwd_ipv4_parsing_errors(self):
         self.assertEquals(self.hmc('hostfwd_add ::66-:0'),
                           "Invalid host forwarding rule '::66-:0' "
                           "(For guest address: Bad port)\r\n")
+
+    def test_qmp_hostfwd_ipv6(self):
+        self.vm.add_args('-nodefaults',
+                         '-netdev', 'user,id=vnet',
+                         '-device', 'virtio-net,netdev=vnet')
+        self.vm.launch()
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp:[::1]:65022-[fe80::1]:22'),
+                          '')
+        self.assertEquals(self.hmc('hostfwd_remove vnet tcp:[::1]:65022'),
+                          'host forwarding rule for tcp:[::1]:65022 removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_add tcp:[]:65042-[fe80::1]:42'),
+                          '')
+        self.assertEquals(self.hmc('hostfwd_remove tcp:[]:65042'),
+                          'host forwarding rule for tcp:[]:65042 removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_add udp:[::1]:65042-[fe80::1]:42'),
+                          '')
+        self.assertEquals(self.hmc('hostfwd_remove udp:[::1]:65042'),
+                          'host forwarding rule for udp:[::1]:65042 removed\r\n')
+
+    def test_qmp_hostfwd_ipv6_functional_errors(self):
+        """Verify handling of various kinds of errors given valid addresses."""
+        self.vm.add_args('-nodefaults',
+                         '-netdev', 'user,id=vnet',
+                         '-device', 'virtio-net,netdev=vnet')
+        self.vm.launch()
+        self.assertEquals(self.hmc('hostfwd_remove :[::1]:65022'),
+                          'host forwarding rule for :[::1]:65022 not found\r\n')
+        self.assertEquals(self.hmc('hostfwd_add udp:[::1]:65042-[fe80::1]:42'),
+                          '')
+        self.assertEquals(self.hmc('hostfwd_add udp:[::1]:65042-[fe80::1]:42'),
+                          "Could not set up host forwarding rule "
+                          "'udp:[::1]:65042-[fe80::1]:42'\r\n")
+        self.assertEquals(self.hmc('hostfwd_remove :[::1]:65042'),
+                          'host forwarding rule for :[::1]:65042 not found\r\n')
+        self.assertEquals(self.hmc('hostfwd_remove udp:[::1]:65042'),
+                          'host forwarding rule for udp:[::1]:65042 removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_remove udp:[::1]:65042'),
+                          'host forwarding rule for udp:[::1]:65042 not found\r\n')
+
+    def test_qmp_hostfwd_ipv6_errors(self):
+        """Verify handling of various kinds of errors."""
+        self.vm.add_args('-nodefaults',
+                         '-netdev', 'user,id=vnet',
+                         '-device', 'virtio-net,netdev=vnet')
+        self.vm.launch()
+        self.assertEquals(self.hmc('hostfwd_add :[::1-'),
+                          "Invalid host forwarding rule ':[::1-' "
+                          "(For host address: error parsing IPv6 address '[::1')\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[::1]:66-[fe80::1'),
+                          "Invalid host forwarding rule ':[::1]:66-[fe80::1' "
+                          "(For guest address: error parsing IPv6 address "
+                          "'[fe80::1')\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[:::]:66-foo'),
+                          "Invalid host forwarding rule ':[:::]:66-foo' "
+                          "(For host address: Bad address)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[::1]-foo'),
+                          "Invalid host forwarding rule ':[::1]-foo' "
+                          "(For host address: error parsing IPv6 address '[::1]')\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[::1]:66-[foo]'),
+                          "Invalid host forwarding rule ':[::1]:66-[foo]' "
+                          "(For guest address: error parsing IPv6 address '[foo]')\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[::1]:66666-foo'),
+                          "Invalid host forwarding rule ':[::1]:66666-foo' "
+                          "(For host address: Bad port)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[::1]:66-[fe80::1]:-1'),
+                          "Invalid host forwarding rule "
+                          "':[::1]:66-[fe80::1]:-1' (For guest address: Bad port)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[::1]:66-[fe80::1]:66666'),
+                          "Invalid host forwarding rule "
+                          "':[::1]:66-[fe80::1]:66666' (For guest address: Bad port)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[::1]:66-[fe80::1]:0'),
+                          "Invalid host forwarding rule "
+                          "':[::1]:66-[fe80::1]:0' (For guest address: Bad port)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :[::1]:66-1.2.3.4:66'),
+                          "Invalid host forwarding rule ':[::1]:66-1.2.3.4:66' "
+                          "(Both host,guest must be one of ipv4 or ipv6)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :1.2.3.4:66-[fe80::1]:66'),
+                          "Invalid host forwarding rule "
+                          "':1.2.3.4:66-[fe80::1]:66' (Both host,guest must be "
+                          "one of ipv4 or ipv6)\r\n")
-- 
2.30.0.617.g56c4b15f3c-goog



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support
  2021-02-20  0:13 ` [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support Doug Evans via
@ 2021-02-20  0:20   ` Philippe Mathieu-Daudé
  2021-02-23 20:18     ` Doug Evans
  0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-20  0:20 UTC (permalink / raw)
  To: Doug Evans, qemu-devel; +Cc: Samuel Thibault, Daniel P . Berrangé

Hi Doug,

On 2/20/21 1:13 AM, Doug Evans via wrote:

When updating submodules, the commit description is a good
good place to include the output of:

  $ git shortlog 8f43a99..26ae658

See for example QEMU commit f350d78f102 ("Update SLOF").

Anyhow up to the maintainer merging your patch.

> Signed-off-by: Doug Evans <dje@google.com>
> ---
> 
> Changes from v4:
> NOTE TO REVIEWERS: I need some hand-holding to know what The Right
> way to submit this particular patch is.
> 
> - no change
> 
> Changes from v3:
> - pick up latest libslirp patch to reject ipv6 addr-any for guest address
>   - libslirp currently only provides a stateless DHCPv6 server, which means
>     it can't know in advance what the guest's IP address is, and thus
>     cannot do the "addr-any -> guest ip address" translation that is done
>     for ipv4
> 
> Changes from v2:
> - this patch is new in v3, split out from v2
> 
>  slirp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/slirp b/slirp
> index 8f43a99191..26ae658a83 160000
> --- a/slirp
> +++ b/slirp
> @@ -1 +1 @@
> -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
> +Subproject commit 26ae658a83eeca16780cf5615c8247cbb151c3fa
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/5] Add support for ipv6 host forwarding
  2021-02-20  0:13 [PATCH v5 0/5] Add support for ipv6 host forwarding Doug Evans via
                   ` (4 preceding siblings ...)
  2021-02-20  0:13 ` [PATCH v5 5/5] net: Extend host forwarding to support IPv6 Doug Evans via
@ 2021-02-20  0:27 ` no-reply
  5 siblings, 0 replies; 10+ messages in thread
From: no-reply @ 2021-02-20  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: samuel.thibault, berrange, qemu-devel, dje

Patchew URL: https://patchew.org/QEMU/20210220001322.1311139-1-dje@google.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210220001322.1311139-1-dje@google.com
Subject: [PATCH v5 0/5] Add support for ipv6 host forwarding

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210220001322.1311139-1-dje@google.com -> patchew/20210220001322.1311139-1-dje@google.com
Switched to a new branch 'test'
9d33831 net: Extend host forwarding to support IPv6
2b79933 net/slirp.c: Refactor address parsing
5090008 inet_parse_host_and_addr: Recognize []:port (empty ipv6 address)
5c2dcad util/qemu-sockets.c: Split host:port parsing out of inet_parse
79b77c4 slirp: Advance libslirp submodule to add ipv6 host-forward support

=== OUTPUT BEGIN ===
1/5 Checking commit 79b77c431b30 (slirp: Advance libslirp submodule to add ipv6 host-forward support)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Doug Evans via <qemu-devel@nongnu.org>

total: 1 errors, 0 warnings, 2 lines checked

Patch 1/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/5 Checking commit 5c2dcad2990c (util/qemu-sockets.c: Split host:port parsing out of inet_parse)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Doug Evans via <qemu-devel@nongnu.org>

total: 1 errors, 0 warnings, 117 lines checked

Patch 2/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/5 Checking commit 509000883fbd (inet_parse_host_and_addr: Recognize []:port (empty ipv6 address))
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Doug Evans via <qemu-devel@nongnu.org>

total: 1 errors, 0 warnings, 20 lines checked

Patch 3/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/5 Checking commit 2b7993354518 (net/slirp.c: Refactor address parsing)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Doug Evans via <qemu-devel@nongnu.org>

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#248: 
new file mode 100644

WARNING: line over 80 characters
#334: FILE: tests/acceptance/hostfwd.py:82:
+                          "host address: error parsing port in address ':')\r\n")

total: 1 errors, 2 warnings, 315 lines checked

Patch 4/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/5 Checking commit 9d3383170e32 (net: Extend host forwarding to support IPv6)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Doug Evans via <qemu-devel@nongnu.org>

WARNING: line over 80 characters
#225: FILE: tests/acceptance/hostfwd.py:101:
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp:[::1]:65022-[fe80::1]:22'),

WARNING: line over 80 characters
#228: FILE: tests/acceptance/hostfwd.py:104:
+                          'host forwarding rule for tcp:[::1]:65022 removed\r\n')

WARNING: line over 80 characters
#236: FILE: tests/acceptance/hostfwd.py:112:
+                          'host forwarding rule for udp:[::1]:65042 removed\r\n')

WARNING: line over 80 characters
#254: FILE: tests/acceptance/hostfwd.py:130:
+                          'host forwarding rule for udp:[::1]:65042 removed\r\n')

WARNING: line over 80 characters
#256: FILE: tests/acceptance/hostfwd.py:132:
+                          'host forwarding rule for udp:[::1]:65042 not found\r\n')

WARNING: line over 80 characters
#266: FILE: tests/acceptance/hostfwd.py:142:
+                          "(For host address: error parsing IPv6 address '[::1')\r\n")

WARNING: line over 80 characters
#276: FILE: tests/acceptance/hostfwd.py:152:
+                          "(For host address: error parsing IPv6 address '[::1]')\r\n")

WARNING: line over 80 characters
#279: FILE: tests/acceptance/hostfwd.py:155:
+                          "(For guest address: error parsing IPv6 address '[foo]')\r\n")

WARNING: line over 80 characters
#285: FILE: tests/acceptance/hostfwd.py:161:
+                          "':[::1]:66-[fe80::1]:-1' (For guest address: Bad port)\r\n")

WARNING: line over 80 characters
#288: FILE: tests/acceptance/hostfwd.py:164:
+                          "':[::1]:66-[fe80::1]:66666' (For guest address: Bad port)\r\n")

WARNING: line over 80 characters
#291: FILE: tests/acceptance/hostfwd.py:167:
+                          "':[::1]:66-[fe80::1]:0' (For guest address: Bad port)\r\n")

total: 1 errors, 11 warnings, 260 lines checked

Patch 5/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210220001322.1311139-1-dje@google.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support
  2021-02-20  0:20   ` Philippe Mathieu-Daudé
@ 2021-02-23 20:18     ` Doug Evans
  2021-02-26 12:18       ` Marc-André Lureau
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Evans @ 2021-02-23 20:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: QEMU Developers, Samuel Thibault, Daniel P . Berrangé

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

On Fri, Feb 19, 2021 at 4:20 PM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Hi Doug,
>
> On 2/20/21 1:13 AM, Doug Evans via wrote:
>
> When updating submodules, the commit description is a good
> good place to include the output of:
>
>   $ git shortlog 8f43a99..26ae658
>
> See for example QEMU commit f350d78f102 ("Update SLOF").
>
> Anyhow up to the maintainer merging your patch.
>


Thanks, that helps a little.
The issue here is that qemu is using branch 4.2 of libslirp, whereas the
patch is currently just on libslirp's master branch.
Part of the problem is the commit description, which you've helped with.
But what about the functional part of the patch?
I can only get git format-patch to include a commit id, and the only commit
id available is on the libslirp master branch.

Is there an additional step I need to do like submit libslirp changes in
three parts?
Step 1: Submit patch to libslirp's repo (not qemu's copy, but the
libslirp master repo)
Step 2: Submit a patch to libslirp's repo to add the patch to its 4.2 branch
Step 2b: Wait for qemu's copy of libslirp's 4.2 branch to appear in
qemu's libslirp repo
Step 3: Submit patch to advance qemu's libslirp submodule

I've done steps 1,3, it's just effecting the equivalent of step2 that I'm
fuzzy on.


> > Signed-off-by: Doug Evans <dje@google.com>
> > ---
> >
> > Changes from v4:
> > NOTE TO REVIEWERS: I need some hand-holding to know what The Right
> > way to submit this particular patch is.
> >
> > - no change
> >
> > Changes from v3:
> > - pick up latest libslirp patch to reject ipv6 addr-any for guest address
> >   - libslirp currently only provides a stateless DHCPv6 server, which
> means
> >     it can't know in advance what the guest's IP address is, and thus
> >     cannot do the "addr-any -> guest ip address" translation that is done
> >     for ipv4
> >
> > Changes from v2:
> > - this patch is new in v3, split out from v2
> >
> >  slirp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/slirp b/slirp
> > index 8f43a99191..26ae658a83 160000
> > --- a/slirp
> > +++ b/slirp
> > @@ -1 +1 @@
> > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
> > +Subproject commit 26ae658a83eeca16780cf5615c8247cbb151c3fa
> >
>
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support
  2021-02-23 20:18     ` Doug Evans
@ 2021-02-26 12:18       ` Marc-André Lureau
  0 siblings, 0 replies; 10+ messages in thread
From: Marc-André Lureau @ 2021-02-26 12:18 UTC (permalink / raw)
  To: Doug Evans
  Cc: Samuel Thibault, Daniel P . Berrangé,
	Philippe Mathieu-Daudé,
	QEMU Developers

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

Hi

On Wed, Feb 24, 2021 at 12:20 AM Doug Evans <dje@google.com> wrote:

> On Fri, Feb 19, 2021 at 4:20 PM Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
>
>> Hi Doug,
>>
>> On 2/20/21 1:13 AM, Doug Evans via wrote:
>>
>> When updating submodules, the commit description is a good
>> good place to include the output of:
>>
>>   $ git shortlog 8f43a99..26ae658
>>
>> See for example QEMU commit f350d78f102 ("Update SLOF").
>>
>> Anyhow up to the maintainer merging your patch.
>>
>
>
> Thanks, that helps a little.
> The issue here is that qemu is using branch 4.2 of libslirp, whereas the
> patch is currently just on libslirp's master branch.
> Part of the problem is the commit description, which you've helped with.
> But what about the functional part of the patch?
> I can only get git format-patch to include a commit id, and the only
> commit id available is on the libslirp master branch.
>
> Is there an additional step I need to do like submit libslirp changes in
> three parts?
> Step 1: Submit patch to libslirp's repo (not qemu's copy, but the
> libslirp master repo)
> Step 2: Submit a patch to libslirp's repo to add the patch to its 4.2
> branch
> Step 2b: Wait for qemu's copy of libslirp's 4.2 branch to appear in
> qemu's libslirp repo
> Step 3: Submit patch to advance qemu's libslirp submodule
>
> I've done steps 1,3, it's just effecting the equivalent of step2 that I'm
> fuzzy on.
>

Only bug fixes should go in libslirp 4.2 branch. You can submit MR against
the stable-4.2 branch in gitlab.

We may update qemu to libslirp master while not in freeze (as long as
feature versioning is handled correctly, which may be error prone).

I tried to update QEMU to libslirp master some time ago, unfortunately it
failed to pass the merge because of the next commit changing the build
system (
https://patchew.org/QEMU/20210125073427.3970606-1-marcandre.lureau@redhat.com/20210125073427.3970606-2-marcandre.lureau@redhat.com/).
But just updating libslirp to master should be fine. You can see in the
commit message that I use a git tool "cherry-diff" to highlight the
differences between the branches.




>
>> > Signed-off-by: Doug Evans <dje@google.com>
>> > ---
>> >
>> > Changes from v4:
>> > NOTE TO REVIEWERS: I need some hand-holding to know what The Right
>> > way to submit this particular patch is.
>> >
>> > - no change
>> >
>> > Changes from v3:
>> > - pick up latest libslirp patch to reject ipv6 addr-any for guest
>> address
>> >   - libslirp currently only provides a stateless DHCPv6 server, which
>> means
>> >     it can't know in advance what the guest's IP address is, and thus
>> >     cannot do the "addr-any -> guest ip address" translation that is
>> done
>> >     for ipv4
>> >
>> > Changes from v2:
>> > - this patch is new in v3, split out from v2
>> >
>> >  slirp | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/slirp b/slirp
>> > index 8f43a99191..26ae658a83 160000
>> > --- a/slirp
>> > +++ b/slirp
>> > @@ -1 +1 @@
>> > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
>> > +Subproject commit 26ae658a83eeca16780cf5615c8247cbb151c3fa
>> >
>>
>>

-- 
Marc-André Lureau

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-02-26 12:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-20  0:13 [PATCH v5 0/5] Add support for ipv6 host forwarding Doug Evans via
2021-02-20  0:13 ` [PATCH v5 1/5] slirp: Advance libslirp submodule to add ipv6 host-forward support Doug Evans via
2021-02-20  0:20   ` Philippe Mathieu-Daudé
2021-02-23 20:18     ` Doug Evans
2021-02-26 12:18       ` Marc-André Lureau
2021-02-20  0:13 ` [PATCH v5 2/5] util/qemu-sockets.c: Split host:port parsing out of inet_parse Doug Evans via
2021-02-20  0:13 ` [PATCH v5 3/5] inet_parse_host_and_addr: Recognize []:port (empty ipv6 address) Doug Evans via
2021-02-20  0:13 ` [PATCH v5 4/5] net/slirp.c: Refactor address parsing Doug Evans via
2021-02-20  0:13 ` [PATCH v5 5/5] net: Extend host forwarding to support IPv6 Doug Evans via
2021-02-20  0:27 ` [PATCH v5 0/5] Add support for ipv6 host forwarding no-reply

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).