[PATCH v6 0/4] Add support for ipv6 host forwarding

[PATCH v6 0/4] Add support for ipv6 host forwarding

[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/4.

Changes from v5:

1/4 slirp: Advance libslirp submodule to current master
NOTE TO REVIEWERS: It may be a better use of everyone's time if a
maintainer takes on advancing QEMU's libslirp to libslirp's master.
Beyond that, I really don't know what to do except submit this patch as
is currently provided.

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

Also split out parsing of ipv4=on|off, ipv6=on|off

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

Use InetSocketAddress and getaddrinfo().
Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.

4/4: net: Extend host forwarding to support IPv6

Recognize ipv4=,ipv6= options.

Note: v5's 3/5 "Recognize []:port (empty ipv6 address)" has been deleted:
the churn on this patch series needs to be reduced.
This change is not required, and can easily be done in a later patch.

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 (4):
  slirp: Advance libslirp submodule to add ipv6 host-forward support
  util/qemu-sockets.c: Split host:port parsing out of inet_parse
  net/slirp.c: Refactor address parsing
  net: Extend host forwarding to support IPv6

 hmp-commands.hx             |  18 ++-
 include/qemu/sockets.h      |   5 +
 net/slirp.c                 | 236 ++++++++++++++++++++++++++----------
 slirp                       |   2 +-
 tests/acceptance/hostfwd.py | 185 ++++++++++++++++++++++++++++
 util/qemu-sockets.c         |  82 +++++++++----
 6 files changed, 436 insertions(+), 92 deletions(-)
 create mode 100644 tests/acceptance/hostfwd.py

-- 
2.31.1.295.g9ea45b61b8-goog

[PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support

[Doug Evans]

5eraph (2):
      disable_dns option
      limit vnameserver_addr to port 53

Akihiro Suda (1):
      libslirp.h: fix SlirpConfig v3 documentation

Doug Evans (11):
      Add ipv6 host forward support
      tcpx_listen: Pass sizeof(addr) to memset
      Reject host forwarding to ipv6 "addr-any"
      Add /build/ to .gitignore
      New utility slirp_ether_ntoa
      m_cleanup_list: make static
      New API routine slirp_neighbor_info
      Move DEBUG_CALL("if_start") to DEBUG_VERBOSE_CALL
      tcpx_listen: tcp_newtcpcb doesn't fail
      slirp_add_host*fwd: Ensure all error paths set errno
      Perform lazy guest address resolution for IPv6

Dr. David Alan Gilbert (1):
      ip_stripoptions use memmove

Giuseppe Scrivano (1):
      socket: consume empty packets

Hafiz Abid Qadeer (1):
      Fix a typo that can cause slow socket response on Windows.

Jindrich Novy (4):
      Fix possible infinite loops and use-after-free
      Use secure string copy to avoid overflow
      Be sure to initialize sockaddr structure
      Check lseek() for failure

Marc-André Lureau (26):
      Merge branch 'master' into 'master'
      Merge branch 'fix-slirpconfig-3-doc' into 'master'
      Fix use-afte-free in ip_reass() (CVE-2020-1983)
      Update CHANGELOG
      Merge branch 'cve-2020-1983' into 'master'
      Release v4.3.0
      Merge branch 'release-v4.3.0' into 'master'
      changelog: post-release
      util: do not silently truncate
      Merge branch 'slirp-fmt-truncate' into 'master'
      Release v4.3.1
      Merge branch 'release-v4.3.1' into 'master'
      changelog: post-release
      .gitlab-ci: add a Coverity stage
      Merge branch 'coverity' into 'master'
      Merge branch 'ios-support' into 'master'
      Merge branch 'master' into 'master'
      Remove the QEMU-special make build-system
      Merge branch 'qemu' into 'master'
      Release v4.4.0
      Merge branch '4.4.0-release' into 'master'
      changelog: post-release
      Remove some needless (void)casts
      Fix unused variables
      Merge branch 'gitignore-build' into 'master'
      Merge branch 'macos-deployment-target' into 'master'

Nathaniel Wesley Filardo (1):
      fork_exec_child_setup: improve signal handling

Paolo Bonzini (2):
      meson: remove meson-dist script
      meson: support compiling as subproject

Philippe Mathieu-Daudé (3):
      Fix win32 builds by using the SLIRP_PACKED definition
      Fix constness warnings
      Remove unnecessary break

Prasad J Pandit (1):
      slirp: check pkt_len before reading protocol header

Ralf Haferkamp (2):
      Drop bogus IPv6 messages
      Fix MTU check

Samuel Thibault (45):
      Merge branch 'ip6_payload_len' into 'master'
      Merge branch 'lp1878043' into 'master'
      udp, udp6, icmp: handle TTL value
      icmp, icmp6: Add icmp_forward_error and icmp6_forward_error
      udp, udp6, icmp, icmp6: Enable forwarding errors on Linux
      TCPIPHDR_DELTA: Fix potential negative value
      sosendoob: better document what urgc is used for
      Merge branch 'G_GNUC_PRINTF' into 'master'
      Merge branch 'CVE-2020-29129' into 'master'
      Merge branch 'ttl' into 'master'
      Merge branch 'errors' into 'master'
      Merge branch 'consume-empty-packet' into 'master'
      Merge branch 'void' into 'master'
      Merge branch 'master' into 'master'
      Merge branch 'unused' into 'master'
      Merge branch 'socket_delay' into 'master'
      tcp_subr: simplify code
      Merge branch 'ipv6-host-fwd-9-patch' into 'master'
      Document the slirp API
      Complete timeout documentation
      Merge branch 'memset-sizeof' into 'master'
      Merge branch 'reject-ipv6-addr-any' into 'master'
      ip6_output: fix memory leak on fast-send
      Merge branch 'ndp-leak' into 'master'
      Merge branch 'memory_leaks' into 'master'
      TODO for generalizing the hostfwd calls
      socket.h: add missing sbuf.h inclusion
      Expose udpx_listen and tcpx_listen as taking sockaddr
      Disable polling for PRI on MacOS
      Merge branch 'macos-pri' into 'master'
      Merge branch 'x_listen' into 'master'
      udpx/tcpx_listen: Add missing const qualifier
      sockaddr_*: add missing const qualifiers
      Merge branch 'm-cleanup-list-prototype' into 'master'
      Merge branch 'neighbor-info' into 'master'
      udpx/tcpx_listen: Use struct sockaddr * types
      Add ipv4/ipv6-agnostic host forwarding functions
      hostfwd: Add SLIRP_HOSTFWD_V6ONLY flag
      Merge branch 'hostxfwd' into 'master'
      Merge branch 'verbose-if-start' into 'master'
      Remove slirp_add/remove_ipv6_hostfwd
      Merge branch 'listen-errno' into 'master'
      Merge branch 'newtcpcb-no-fail' into 'master'
      Merge branch 'listen_v6only' into 'master'
      Merge branch 'lazy-ipv6-resolution' into 'master'

Stefan Weil (1):
      Add G_GNUC_PRINTF to local function slirp_vsnprintf

WaluigiWare64 (1):
      Set macOS deployment target to macOS 10.4 Without a macOS deployment target, the resulting library does not work on macOS versions lower than it was currently built on. For example, if libslirp was built on macOS 10.15, it would not work on macOS 10.14.

jeremy marchand (4):
      m_free: remove the M_EXT flag after freeing the mbuf extended buffer
      refactor m_cleanup as requested in slirp/libslirp!68
      m_cleanup: fix memory leaks
      m_cleanup: set qh_link and qh_rlink to the list head

osy (1):
      Add DNS resolving for iOS

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

Changes from v5:

1/4 slirp: Advance libslirp submodule to current master
NOTE TO REVIEWERS: It may be a better use of everyone's time if a
maintainer takes on advancing QEMU's libslirp to libslirp's master.
Beyond that, I really don't know what to do except submit this patch as
is currently provided.

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

diff --git a/slirp b/slirp
index 8f43a99191..4e6444e842 160000
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
+Subproject commit 4e6444e842695a6bfb00e15a8d0edfceb5c4628d
-- 
2.31.1.295.g9ea45b61b8-goog

Re: [PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support

[Marc-André Lureau]

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

Hi

On Thu, Apr 15, 2021 at 7:41 AM Doug Evans <dje@google.com> wrote:

> 5eraph (2):
>       disable_dns option
>       limit vnameserver_addr to port 53
>
> Akihiro Suda (1):
>       libslirp.h: fix SlirpConfig v3 documentation
>
> Doug Evans (11):
>       Add ipv6 host forward support
>       tcpx_listen: Pass sizeof(addr) to memset
>       Reject host forwarding to ipv6 "addr-any"
>       Add /build/ to .gitignore
>       New utility slirp_ether_ntoa
>       m_cleanup_list: make static
>       New API routine slirp_neighbor_info
>       Move DEBUG_CALL("if_start") to DEBUG_VERBOSE_CALL
>       tcpx_listen: tcp_newtcpcb doesn't fail
>       slirp_add_host*fwd: Ensure all error paths set errno
>       Perform lazy guest address resolution for IPv6
>
> Dr. David Alan Gilbert (1):
>       ip_stripoptions use memmove
>
> Giuseppe Scrivano (1):
>       socket: consume empty packets
>
> Hafiz Abid Qadeer (1):
>       Fix a typo that can cause slow socket response on Windows.
>
> Jindrich Novy (4):
>       Fix possible infinite loops and use-after-free
>       Use secure string copy to avoid overflow
>       Be sure to initialize sockaddr structure
>       Check lseek() for failure
>
> Marc-André Lureau (26):
>       Merge branch 'master' into 'master'
>       Merge branch 'fix-slirpconfig-3-doc' into 'master'
>       Fix use-afte-free in ip_reass() (CVE-2020-1983)
>       Update CHANGELOG
>       Merge branch 'cve-2020-1983' into 'master'
>       Release v4.3.0
>       Merge branch 'release-v4.3.0' into 'master'
>       changelog: post-release
>       util: do not silently truncate
>       Merge branch 'slirp-fmt-truncate' into 'master'
>       Release v4.3.1
>       Merge branch 'release-v4.3.1' into 'master'
>       changelog: post-release
>       .gitlab-ci: add a Coverity stage
>       Merge branch 'coverity' into 'master'
>       Merge branch 'ios-support' into 'master'
>       Merge branch 'master' into 'master'
>       Remove the QEMU-special make build-system
>       Merge branch 'qemu' into 'master'
>       Release v4.4.0
>       Merge branch '4.4.0-release' into 'master'
>       changelog: post-release
>       Remove some needless (void)casts
>       Fix unused variables
>       Merge branch 'gitignore-build' into 'master'
>       Merge branch 'macos-deployment-target' into 'master'
>
> Nathaniel Wesley Filardo (1):
>       fork_exec_child_setup: improve signal handling
>
> Paolo Bonzini (2):
>       meson: remove meson-dist script
>       meson: support compiling as subproject
>
> Philippe Mathieu-Daudé (3):
>       Fix win32 builds by using the SLIRP_PACKED definition
>       Fix constness warnings
>       Remove unnecessary break
>
> Prasad J Pandit (1):
>       slirp: check pkt_len before reading protocol header
>
> Ralf Haferkamp (2):
>       Drop bogus IPv6 messages
>       Fix MTU check
>
> Samuel Thibault (45):
>       Merge branch 'ip6_payload_len' into 'master'
>       Merge branch 'lp1878043' into 'master'
>       udp, udp6, icmp: handle TTL value
>       icmp, icmp6: Add icmp_forward_error and icmp6_forward_error
>       udp, udp6, icmp, icmp6: Enable forwarding errors on Linux
>       TCPIPHDR_DELTA: Fix potential negative value
>       sosendoob: better document what urgc is used for
>       Merge branch 'G_GNUC_PRINTF' into 'master'
>       Merge branch 'CVE-2020-29129' into 'master'
>       Merge branch 'ttl' into 'master'
>       Merge branch 'errors' into 'master'
>       Merge branch 'consume-empty-packet' into 'master'
>       Merge branch 'void' into 'master'
>       Merge branch 'master' into 'master'
>       Merge branch 'unused' into 'master'
>       Merge branch 'socket_delay' into 'master'
>       tcp_subr: simplify code
>       Merge branch 'ipv6-host-fwd-9-patch' into 'master'
>       Document the slirp API
>       Complete timeout documentation
>       Merge branch 'memset-sizeof' into 'master'
>       Merge branch 'reject-ipv6-addr-any' into 'master'
>       ip6_output: fix memory leak on fast-send
>       Merge branch 'ndp-leak' into 'master'
>       Merge branch 'memory_leaks' into 'master'
>       TODO for generalizing the hostfwd calls
>       socket.h: add missing sbuf.h inclusion
>       Expose udpx_listen and tcpx_listen as taking sockaddr
>       Disable polling for PRI on MacOS
>       Merge branch 'macos-pri' into 'master'
>       Merge branch 'x_listen' into 'master'
>       udpx/tcpx_listen: Add missing const qualifier
>       sockaddr_*: add missing const qualifiers
>       Merge branch 'm-cleanup-list-prototype' into 'master'
>       Merge branch 'neighbor-info' into 'master'
>       udpx/tcpx_listen: Use struct sockaddr * types
>       Add ipv4/ipv6-agnostic host forwarding functions
>       hostfwd: Add SLIRP_HOSTFWD_V6ONLY flag
>       Merge branch 'hostxfwd' into 'master'
>       Merge branch 'verbose-if-start' into 'master'
>       Remove slirp_add/remove_ipv6_hostfwd
>       Merge branch 'listen-errno' into 'master'
>       Merge branch 'newtcpcb-no-fail' into 'master'
>       Merge branch 'listen_v6only' into 'master'
>       Merge branch 'lazy-ipv6-resolution' into 'master'
>
> Stefan Weil (1):
>       Add G_GNUC_PRINTF to local function slirp_vsnprintf
>
> WaluigiWare64 (1):
>       Set macOS deployment target to macOS 10.4 Without a macOS deployment
> target, the resulting library does not work on macOS versions lower than it
> was currently built on. For example, if libslirp was built on macOS 10.15,
> it would not work on macOS 10.14.
>
> jeremy marchand (4):
>       m_free: remove the M_EXT flag after freeing the mbuf extended buffer
>       refactor m_cleanup as requested in slirp/libslirp!68
>       m_cleanup: fix memory leaks
>       m_cleanup: set qh_link and qh_rlink to the list head
>
> osy (1):
>       Add DNS resolving for iOS
>
> Signed-off-by: Doug Evans <dje@google.com>
> ---
>
> Changes from v5:
>
> 1/4 slirp: Advance libslirp submodule to current master
> NOTE TO REVIEWERS: It may be a better use of everyone's time if a
> maintainer takes on advancing QEMU's libslirp to libslirp's master.
> Beyond that, I really don't know what to do except submit this patch as
> is currently provided.
>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

It can do, but it should rather be a diff of the commits that are new,
those that were not in the stable branch.



>  slirp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/slirp b/slirp
> index 8f43a99191..4e6444e842 160000
> --- a/slirp
> +++ b/slirp
> @@ -1 +1 @@
> -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
> +Subproject commit 4e6444e842695a6bfb00e15a8d0edfceb5c4628d
> --
> 2.31.1.295.g9ea45b61b8-goog
>
>
>

-- 
Marc-André Lureau

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

Re: [PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support

[Doug Evans]

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

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

> Hi
>
> [...]
>>
>>
>> Changes from v5:
>>
>> 1/4 slirp: Advance libslirp submodule to current master
>> NOTE TO REVIEWERS: It may be a better use of everyone's time if a
>> maintainer takes on advancing QEMU's libslirp to libslirp's master.
>> Beyond that, I really don't know what to do except submit this patch as
>> is currently provided.
>>
>>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> It can do, but it should rather be a diff of the commits that are new,
> those that were not in the stable branch.
>


Can you elaborate?
E.g., Should a new libslirp release be made first, and then update
qemu-master to that new release?

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

Re: [PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support

[Doug Evans]

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

Ping.

On Fri, May 7, 2021 at 8:46 AM Doug Evans <dje@google.com> wrote:

> On Fri, May 7, 2021 at 8:23 AM Marc-André Lureau <
> marcandre.lureau@gmail.com> wrote:
>
>> Hi
>>
>> [...]
>>>
>>>
>>> Changes from v5:
>>>
>>> 1/4 slirp: Advance libslirp submodule to current master
>>> NOTE TO REVIEWERS: It may be a better use of everyone's time if a
>>> maintainer takes on advancing QEMU's libslirp to libslirp's master.
>>> Beyond that, I really don't know what to do except submit this patch as
>>> is currently provided.
>>>
>>>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> It can do, but it should rather be a diff of the commits that are new,
>> those that were not in the stable branch.
>>
>
>
> Can you elaborate?
> E.g., Should a new libslirp release be made first, and then update
> qemu-master to that new release?
>


Hey Marc-André and Samuel,
What's the next step here (if any)?
Would it be preferable to make a new libslirp release instead?
[I also don't understand the comment "it should rather be a diff of the
commits that are new, ...".
I haven't seen this request before (apologies if I missed it), but more
importantly
isn't it trivial to generate such diffs, without adding them to the email?
I could be missing something of course.]

At some point a new libslirp release needs to be made, and at some point
QEMU needs to be updated to use it.
Seems like now is a good time, but maybe there are reasons to prefer not
doing so now.
I can imagine QEMU wanting to always use a stable branch of libslirp,
so I just want to be absolutely clear that switching QEMU to use libslirp's
master branch is desired,
and if anything more is needed from me.
I'm happy with whatever you decide, but I don't want to waste your time
guessing and then having to iterate when I guess wrong.

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

Re: [PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support

[Marc-André Lureau]

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

Hi

On Wed, May 12, 2021 at 8:43 PM Doug Evans <dje@google.com> wrote:

> Ping.
>
> On Fri, May 7, 2021 at 8:46 AM Doug Evans <dje@google.com> wrote:
>
>> On Fri, May 7, 2021 at 8:23 AM Marc-André Lureau <
>> marcandre.lureau@gmail.com> wrote:
>>
>>> Hi
>>>
>>> [...]
>>>>
>>>>
>>>> Changes from v5:
>>>>
>>>> 1/4 slirp: Advance libslirp submodule to current master
>>>> NOTE TO REVIEWERS: It may be a better use of everyone's time if a
>>>> maintainer takes on advancing QEMU's libslirp to libslirp's master.
>>>> Beyond that, I really don't know what to do except submit this patch as
>>>> is currently provided.
>>>>
>>>>
>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> It can do, but it should rather be a diff of the commits that are new,
>>> those that were not in the stable branch.
>>>
>>
>>
>> Can you elaborate?
>> E.g., Should a new libslirp release be made first, and then update
>> qemu-master to that new release?
>>
>
>
> Hey Marc-André and Samuel,
> What's the next step here (if any)?
>

There isn't much point in bumping qemu dependency if it doesn't make use of
the new API. Thus first step is to get the rest of the series
reviewed/approved imho.

Would it be preferable to make a new libslirp release instead?
>

yes, as I said in some other path review, we would need a libslirp release
AND compatiblity with older slirp releases.

[I also don't understand the comment "it should rather be a diff of the
> commits that are new, ...".
> I haven't seen this request before (apologies if I missed it), but more
> importantly
> isn't it trivial to generate such diffs, without adding them to the email?
> I could be missing something of course.]
>
> At some point a new libslirp release needs to be made, and at some point
> QEMU needs to be updated to use it.
> Seems like now is a good time, but maybe there are reasons to prefer not
> doing so now.
> I can imagine QEMU wanting to always use a stable branch of libslirp,
> so I just want to be absolutely clear that switching QEMU to use
> libslirp's master branch is desired,
> and if anything more is needed from me.
> I'm happy with whatever you decide, but I don't want to waste your time
> guessing and then having to iterate when I guess wrong.
>

You need to rework the series to be compatible with current libslirp. That
may be one of the reason why you don't get more reviews, Jason, the
maintainer, may expect you to do that based on feedback received earlier.

thanks

-- 
Marc-André Lureau

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

Re: [PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support

[Doug Evans]

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

On Wed, May 12, 2021 at 10:18 AM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:

> Hi
>
> On Wed, May 12, 2021 at 8:43 PM Doug Evans <dje@google.com> wrote:
>
>> Ping.
>>
>> On Fri, May 7, 2021 at 8:46 AM Doug Evans <dje@google.com> wrote:
>>
>>> On Fri, May 7, 2021 at 8:23 AM Marc-André Lureau <
>>> marcandre.lureau@gmail.com> wrote:
>>>
>>>> Hi
>>>>
>>>> [...]
>>>>>
>>>>>
>>>>> Changes from v5:
>>>>>
>>>>> 1/4 slirp: Advance libslirp submodule to current master
>>>>> NOTE TO REVIEWERS: It may be a better use of everyone's time if a
>>>>> maintainer takes on advancing QEMU's libslirp to libslirp's master.
>>>>> Beyond that, I really don't know what to do except submit this patch as
>>>>> is currently provided.
>>>>>
>>>>>
>>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>
>>>> It can do, but it should rather be a diff of the commits that are new,
>>>> those that were not in the stable branch.
>>>>
>>>
>>>
>>> Can you elaborate?
>>> E.g., Should a new libslirp release be made first, and then update
>>> qemu-master to that new release?
>>>
>>
>>
>> Hey Marc-André and Samuel,
>> What's the next step here (if any)?
>>
>
> There isn't much point in bumping qemu dependency if it doesn't make use
> of the new API. Thus first step is to get the rest of the series
> reviewed/approved imho.
>


I'm not suggesting the dependency be bumped prior to the entire series
being approved.
By "next step" I meant whether this patch (1/4) in the series is done.
The question I asked: "Should a new libslirp release be made and then have
qemu-master use that (*1)?" was outstanding as it wasn't(isn't) clear
whether the plan was indeed to first make a new libslirp release (even
taking into account the comments on patch 3/4).

(*1): When using QEMU-provided libslirp. When not using QEMU-provided
slirp, of course, be compatible with the libslirp that is being used.
Thanks for bringing that to my attention btw, it's easy to forget given
that QEMU provides its own copy of libslirp and I have completely left that
out of v1 to v5.



> Would it be preferable to make a new libslirp release instead?
>>
>
> yes, as I said in some other path review, we would need a libslirp release
> AND compatiblity with older slirp releases.
>


Indeed! I was, perhaps errantly, treating them as orthogonal discussions.
[On the theory that:
- if we resolve all issues for each piece of the current iteration then
that will reduce the number of iterations,
and I'm not sure patch 1/4 is complete and what happens next for it (given
that a separate repo is involved)
- discussions of making a new libslirp release can proceed independent of
updating patch 3/4
That was the theory anyway.]



> [I also don't understand the comment "it should rather be a diff of the
>> commits that are new, ...".
>> I haven't seen this request before (apologies if I missed it), but more
>> importantly
>> isn't it trivial to generate such diffs, without adding them to the email?
>> I could be missing something of course.]
>>
>> At some point a new libslirp release needs to be made, and at some point
>> QEMU needs to be updated to use it.
>> Seems like now is a good time, but maybe there are reasons to prefer not
>> doing so now.
>> I can imagine QEMU wanting to always use a stable branch of libslirp,
>> so I just want to be absolutely clear that switching QEMU to use
>> libslirp's master branch is desired,
>> and if anything more is needed from me.
>> I'm happy with whatever you decide, but I don't want to waste your time
>> guessing and then having to iterate when I guess wrong.
>>
>
> You need to rework the series to be compatible with current libslirp. That
> may be one of the reason why you don't get more reviews, Jason, the
> maintainer, may expect you to do that based on feedback received earlier.
>


I'm indeed updating v7 in this series to be compatible with current
libslirp, but let's get the issue of a new libslirp release resolved too.

Btw, can you elaborate on "should rather be a diff of the commits that are
new"?
Up until now I've been told to provide "git shortlog old..new" output.
The patch itself is just a one-liner to update the subproject sha1.

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

Re: [PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support

[Marc-André Lureau]

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

Hi

On Wed, May 12, 2021 at 11:51 PM Doug Evans <dje@google.com> wrote:

>
> Btw, can you elaborate on "should rather be a diff of the commits that are
> new"?
> Up until now I've been told to provide "git shortlog old..new" output.
> The patch itself is just a one-liner to update the subproject sha1.
>

git modules used by qemu are usually tracking the master branch, so a
shortlog works fine and shows the additional comments. But when it's
switching branches, the shortlog doesn't provide the diff between the
branches, that is the commits that were not already in the branch being
tracked. In my last update (
https://patchew.org/QEMU/20210125073427.3970606-1-marcandre.lureau@redhat.com/20210125073427.3970606-2-marcandre.lureau@redhat.com/),
I used a tool called git-cherry-diff, but there might be git log arguments
to do that I ignore.


-- 
Marc-André Lureau

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

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

[Doug Evans]

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(-)

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

Re: [PATCH v6 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

[Marc-André Lureau]

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

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: 6576 bytes --]

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

[Doug Evans]

[-- 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 --]

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

[Daniel P. Berrangé]

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Doug Evans]

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

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 ?
>
>

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

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

[Daniel P. Berrangé]

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v6 3/4] net/slirp.c: Refactor address parsing

[Doug Evans]

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

Tested:
avocado run tests/acceptance/hostfwd.py

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

Changes from v5:

Use InetSocketAddress and getaddrinfo().
Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.

 include/qemu/sockets.h      |   2 +
 net/slirp.c                 | 200 ++++++++++++++++++++++++------------
 tests/acceptance/hostfwd.py |  91 ++++++++++++++++
 util/qemu-sockets.c         |  17 +--
 4 files changed, 241 insertions(+), 69 deletions(-)
 create mode 100644 tests/acceptance/hostfwd.py

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 94f4e8de83..6fd71775ce 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -29,6 +29,8 @@ int socket_set_fast_reuse(int fd);
 #define SHUT_RDWR 2
 #endif
 
+int sockaddr_getport(const struct sockaddr *addr);
+
 int inet_ai_family_from_address(InetSocketAddress *addr,
                                 Error **errp);
 const char *inet_parse_host_port(InetSocketAddress *addr,
diff --git a/net/slirp.c b/net/slirp.c
index a01a0fccd3..4be065c30b 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -641,15 +641,108 @@ static SlirpState *slirp_lookup(Monitor *mon, const char *id)
     }
 }
 
+static const char *parse_protocol(const char *str, bool *is_udp,
+                                  Error **errp)
+{
+    char buf[10];
+    const char *p = str;
+
+    if (get_str_sep(buf, sizeof(buf), &p, ':') < 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 '%s'", buf);
+        return NULL;
+    }
+
+    return p;
+}
+
+static int parse_hostfwd_sockaddr(const char *str, int socktype,
+                                  struct sockaddr_storage *saddr,
+                                  Error **errp)
+{
+    struct addrinfo hints, *res = NULL, *e;
+    InetSocketAddress *addr = g_new(InetSocketAddress, 1);
+    int gai_rc;
+    int rc = -1;
+
+    const char *optstr = inet_parse_host_port(addr, str, errp);
+    if (optstr == NULL) {
+        goto fail_return;
+    }
+
+    memset(&hints, 0, sizeof(hints));
+    hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */
+    hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
+    hints.ai_socktype = socktype;
+    hints.ai_family = PF_INET;
+
+    /*
+     * Calling getaddrinfo for guest addresses is dubious, but addresses are
+     * restricted to numeric only. Convert "" to NULL for getaddrinfo's
+     * benefit.
+     */
+    gai_rc = getaddrinfo(*addr->host ? addr->host : NULL,
+                         *addr->port ? addr->port : NULL, &hints, &res);
+    if (gai_rc != 0) {
+        error_setg(errp, "address resolution failed for '%s': %s",
+                   str, gai_strerror(gai_rc));
+        goto fail_return;
+    }
+    if (res->ai_next != NULL) {
+        /*
+         * The caller only wants one address, and except for "any" for both
+         * ipv4 and ipv6 (which we've already precluded above), we shouldn't
+         * get more than one. To assist debugging print all we find.
+         */
+        GString *s = g_string_new(NULL);
+        for (e = res; e != NULL; e = e->ai_next) {
+            char host[NI_MAXHOST];
+            char serv[NI_MAXSERV];
+            int ret = getnameinfo((struct sockaddr *)e->ai_addr, e->ai_addrlen,
+                                  host, sizeof(host),
+                                  serv, sizeof(serv),
+                                  NI_NUMERICHOST | NI_NUMERICSERV);
+            if (ret == 0) {
+                g_string_append_printf(s, "\n  %s:%s", host, serv);
+            } else {
+                g_string_append_printf(s, "\n  unknown, got: %s",
+                                       gai_strerror(ret));
+            }
+        }
+        error_setg(errp, "multiple addresses resolved for '%s':%s",
+                   str, s->str);
+        g_string_free(s, TRUE);
+        goto fail_return;
+    }
+
+    memcpy(saddr, res->ai_addr, res->ai_addrlen);
+    rc = 0;
+
+ fail_return:
+    qapi_free_InetSocketAddress(addr);
+    if (res) {
+        freeaddrinfo(res);
+    }
+    return rc;
+}
+
 void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 {
-    struct in_addr host_addr = { .s_addr = INADDR_ANY };
-    int host_port;
-    char buf[256];
+    struct sockaddr_storage host_addr;
     const char *src_str, *p;
     SlirpState *s;
-    int is_udp = 0;
+    bool is_udp;
+    Error *error = NULL;
     int err;
+    int flags = 0;
     const char *arg1 = qdict_get_str(qdict, "arg1");
     const char *arg2 = qdict_get_try_str(qdict, "arg2");
 
@@ -664,110 +757,91 @@ 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 {
+    p = parse_protocol(p, &is_udp, &error);
+    if (p == NULL) {
         goto fail_syntax;
     }
-
-    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
-        goto fail_syntax;
-    }
-    if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
-        goto fail_syntax;
+    if (is_udp) {
+        flags |= SLIRP_HOSTFWD_UDP;
     }
 
-    if (qemu_strtoi(p, NULL, 10, &host_port)) {
+    if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM,
+                               &host_addr, &error) < 0) {
         goto fail_syntax;
     }
 
-    err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port);
+    err = slirp_remove_hostxfwd(s->slirp, (struct sockaddr *) &host_addr,
+                                sizeof(host_addr), flags);
 
     monitor_printf(mon, "host forwarding rule for %s %s\n", src_str,
                    err ? "not found" : "removed");
     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 };
-    int host_port, guest_port;
+    struct sockaddr_storage host_addr, guest_addr;
     const char *p;
     char buf[256];
-    int is_udp;
-    char *end;
-    const char *fail_reason = "Unknown reason";
+    bool is_udp;
+    Error *error = NULL;
+    int flags = 0;
+    int port;
 
+    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";
+    p = parse_protocol(p, &is_udp, &error);
+    if (p == NULL) {
         goto fail_syntax;
     }
-    if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
-        fail_reason = "Bad host address";
-        goto fail_syntax;
+    if (is_udp) {
+        flags |= SLIRP_HOSTFWD_UDP;
     }
 
     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";
+        error_setg(&error, "missing host-guest separator");
         goto fail_syntax;
     }
 
-    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
-        fail_reason = "Missing guest address";
+    if (parse_hostfwd_sockaddr(buf, is_udp ? SOCK_DGRAM : SOCK_STREAM,
+                               &host_addr, &error) < 0) {
+        error_prepend(&error, "For host address: ");
         goto fail_syntax;
     }
-    if (buf[0] != '\0' && !inet_aton(buf, &guest_addr)) {
-        fail_reason = "Bad guest address";
+
+    if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM,
+                               &guest_addr, &error) < 0) {
+        error_prepend(&error, "For 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";
+    port = sockaddr_getport((struct sockaddr *) &guest_addr);
+    if (port == 0) {
+        error_setg(&error, "For guest address: invalid port '0'");
         goto fail_syntax;
     }
 
-    if (slirp_add_hostfwd(s->slirp, is_udp, host_addr, host_port, guest_addr,
-                          guest_port) < 0) {
-        error_setg(errp, "Could not set up host forwarding rule '%s'",
-                   redir_str);
+    if (slirp_add_hostxfwd(s->slirp,
+                           (struct sockaddr *) &host_addr, sizeof(host_addr),
+                           (struct sockaddr *) &guest_addr, sizeof(guest_addr),
+                           flags) < 0) {
+        error_setg(errp, "Could not set up host forwarding rule '%s': %s",
+                   redir_str, strerror(errno));
         return -1;
     }
     return 0;
 
  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..9b9db142c3
--- /dev/null
+++ b/tests/acceptance/hostfwd.py
@@ -0,0 +1,91 @@
+# 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': Address already in use\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 'abc'\r\n")
+        self.assertEquals(self.hmc('hostfwd_add abc::65022-:22'),
+                          "Invalid host forwarding rule 'abc::65022-:22'" + \
+                          " (bad protocol name 'abc')\r\n")
+        self.assertEquals(self.hmc('hostfwd_add :foo'),
+                          "Invalid host forwarding rule ':foo'" + \
+                          " (missing host-guest separator)\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: address resolution failed for" \
+                          " 'a.b.c.d:66': Name or service not known)\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: address resolution failed" + \
+                          " for 'a.b.c.d:66': Name or service not known)\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-:0'),
+                          "Invalid host forwarding rule '::66-:0'" + \
+                          " (For guest address: invalid port '0')\r\n")
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index c0069f2565..983efeee2f 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -46,23 +46,28 @@
 #endif
 
 
-static int inet_getport(struct addrinfo *e)
+int sockaddr_getport(const struct sockaddr *addr)
 {
-    struct sockaddr_in *i4;
-    struct sockaddr_in6 *i6;
+    const struct sockaddr_in *i4;
+    const struct sockaddr_in6 *i6;
 
-    switch (e->ai_family) {
+    switch (addr->sa_family) {
     case PF_INET6:
-        i6 = (void*)e->ai_addr;
+        i6 = (void *)addr;
         return ntohs(i6->sin6_port);
     case PF_INET:
-        i4 = (void*)e->ai_addr;
+        i4 = (void *)addr;
         return ntohs(i4->sin_port);
     default:
         return 0;
     }
 }
 
+static int inet_getport(struct addrinfo *e)
+{
+    return sockaddr_getport(e->ai_addr);
+}
+
 static void inet_setport(struct addrinfo *e, int port)
 {
     struct sockaddr_in *i4;
-- 
2.31.1.295.g9ea45b61b8-goog

Re: [PATCH v6 3/4] net/slirp.c: Refactor address parsing

[Doug Evans]

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

On Wed, Apr 14, 2021 at 8:40 PM Doug Evans <dje@google.com> wrote:

> ... in preparation for adding ipv6 host forwarding support.
>
> Tested:
> avocado run tests/acceptance/hostfwd.py
>
> Signed-off-by: Doug Evans <dje@google.com>
> ---
>
> [...]
>
> diff --git a/tests/acceptance/hostfwd.py b/tests/acceptance/hostfwd.py
> new file mode 100644
> index 0000000000..9b9db142c3
> --- /dev/null
> +++ b/tests/acceptance/hostfwd.py
> @@ -0,0 +1,91 @@
> [...]
> +
> +    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 'abc'\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add abc::65022-:22'),
> +                          "Invalid host forwarding rule 'abc::65022-:22'"
> + \
> +                          " (bad protocol name 'abc')\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add :foo'),
> +                          "Invalid host forwarding rule ':foo'" + \
> +                          " (missing host-guest separator)\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: address resolution failed
> for" \
> +                          " 'a.b.c.d:66': Name or service not known)\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: address resolution
> failed" + \
> +                          " for 'a.b.c.d:66': Name or service not
> known)\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-:0'),
> +                          "Invalid host forwarding rule '::66-:0'" + \
> +                          " (For guest address: invalid port '0')\r\n")
>


One improvement I think I'd like to make here is that I'm not sure how
portable the text of the result of, e.g., gai_strerror() is,
and relax the expected text of the error messages in the potentially
host-specific part.
But I'll wait until everything else is reviewed.

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

Re: [PATCH v6 3/4] net/slirp.c: Refactor address parsing

[Marc-André Lureau]

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

Hi

On Thu, Apr 15, 2021 at 7:44 AM Doug Evans <dje@google.com> wrote:

> ... in preparation for adding ipv6 host forwarding support.
>
> Tested:
> avocado run tests/acceptance/hostfwd.py
>
> Signed-off-by: Doug Evans <dje@google.com>
> ---
>
> Changes from v5:
>
> Use InetSocketAddress and getaddrinfo().
> Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.
>
>  include/qemu/sockets.h      |   2 +
>  net/slirp.c                 | 200 ++++++++++++++++++++++++------------
>  tests/acceptance/hostfwd.py |  91 ++++++++++++++++
>  util/qemu-sockets.c         |  17 +--
>  4 files changed, 241 insertions(+), 69 deletions(-)
>  create mode 100644 tests/acceptance/hostfwd.py
>
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 94f4e8de83..6fd71775ce 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -29,6 +29,8 @@ int socket_set_fast_reuse(int fd);
>  #define SHUT_RDWR 2
>  #endif
>
> +int sockaddr_getport(const struct sockaddr *addr);
> +
>  int inet_ai_family_from_address(InetSocketAddress *addr,
>                                  Error **errp);
>  const char *inet_parse_host_port(InetSocketAddress *addr,
> diff --git a/net/slirp.c b/net/slirp.c
> index a01a0fccd3..4be065c30b 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -641,15 +641,108 @@ static SlirpState *slirp_lookup(Monitor *mon, const
> char *id)
>      }
>  }
>
> +static const char *parse_protocol(const char *str, bool *is_udp,
> +                                  Error **errp)
> +{
> +    char buf[10];
> +    const char *p = str;
> +
> +    if (get_str_sep(buf, sizeof(buf), &p, ':') < 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 '%s'", buf);
> +        return NULL;
> +    }
> +
> +    return p;
> +}
> +
> +static int parse_hostfwd_sockaddr(const char *str, int socktype,
> +                                  struct sockaddr_storage *saddr,
> +                                  Error **errp)
> +{
> +    struct addrinfo hints, *res = NULL, *e;
> +    InetSocketAddress *addr = g_new(InetSocketAddress, 1);
> +    int gai_rc;
> +    int rc = -1;
> +
> +    const char *optstr = inet_parse_host_port(addr, str, errp);
> +    if (optstr == NULL) {
> +        goto fail_return;
> +    }
> +
> +    memset(&hints, 0, sizeof(hints));
> +    hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */
> +    hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
> +    hints.ai_socktype = socktype;
> +    hints.ai_family = PF_INET;
> +
> +    /*
> +     * Calling getaddrinfo for guest addresses is dubious, but addresses
> are
> +     * restricted to numeric only. Convert "" to NULL for getaddrinfo's
> +     * benefit.
> +     */
> +    gai_rc = getaddrinfo(*addr->host ? addr->host : NULL,
> +                         *addr->port ? addr->port : NULL, &hints, &res);
> +    if (gai_rc != 0) {
> +        error_setg(errp, "address resolution failed for '%s': %s",
> +                   str, gai_strerror(gai_rc));
> +        goto fail_return;
> +    }
> +    if (res->ai_next != NULL) {
> +        /*
> +         * The caller only wants one address, and except for "any" for
> both
> +         * ipv4 and ipv6 (which we've already precluded above), we
> shouldn't
> +         * get more than one. To assist debugging print all we find.
> +         */
> +        GString *s = g_string_new(NULL);
> +        for (e = res; e != NULL; e = e->ai_next) {
> +            char host[NI_MAXHOST];
> +            char serv[NI_MAXSERV];
> +            int ret = getnameinfo((struct sockaddr *)e->ai_addr,
> e->ai_addrlen,
> +                                  host, sizeof(host),
> +                                  serv, sizeof(serv),
> +                                  NI_NUMERICHOST | NI_NUMERICSERV);
> +            if (ret == 0) {
> +                g_string_append_printf(s, "\n  %s:%s", host, serv);
> +            } else {
> +                g_string_append_printf(s, "\n  unknown, got: %s",
> +                                       gai_strerror(ret));
> +            }
> +        }
> +        error_setg(errp, "multiple addresses resolved for '%s':%s",
> +                   str, s->str);
> +        g_string_free(s, TRUE);
> +        goto fail_return;
> +    }
> +
> +    memcpy(saddr, res->ai_addr, res->ai_addrlen);
> +    rc = 0;
> +
> + fail_return:
> +    qapi_free_InetSocketAddress(addr);
> +    if (res) {
> +        freeaddrinfo(res);
> +    }
> +    return rc;
> +}
> +
>  void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
>  {
> -    struct in_addr host_addr = { .s_addr = INADDR_ANY };
> -    int host_port;
> -    char buf[256];
> +    struct sockaddr_storage host_addr;
>      const char *src_str, *p;
>      SlirpState *s;
> -    int is_udp = 0;
> +    bool is_udp;
> +    Error *error = NULL;
>      int err;
> +    int flags = 0;
>      const char *arg1 = qdict_get_str(qdict, "arg1");
>      const char *arg2 = qdict_get_try_str(qdict, "arg2");
>
> @@ -664,110 +757,91 @@ 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 {
> +    p = parse_protocol(p, &is_udp, &error);
> +    if (p == NULL) {
>          goto fail_syntax;
>      }
> -
> -    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
> -        goto fail_syntax;
> -    }
> -    if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
> -        goto fail_syntax;
> +    if (is_udp) {
> +        flags |= SLIRP_HOSTFWD_UDP;
>

This fails to build with the system version of libslirp, as there has not
been releases with this new symbol yet. I am not sure what's the status for
upstream release, we should create an issue there and discuss it.

Anyway, you'll probably need to make the new code conditional with
SLIRP_CHECK_VERSION(), as we don't want qemu to depend on too recent
releases.

thanks


     }
>
> -    if (qemu_strtoi(p, NULL, 10, &host_port)) {
> +    if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM,
> +                               &host_addr, &error) < 0) {
>          goto fail_syntax;
>      }
>
> -    err = slirp_remove_hostfwd(s->slirp, is_udp, host_addr, host_port);
> +    err = slirp_remove_hostxfwd(s->slirp, (struct sockaddr *) &host_addr,
> +                                sizeof(host_addr), flags);
>
>      monitor_printf(mon, "host forwarding rule for %s %s\n", src_str,
>                     err ? "not found" : "removed");
>      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 };
> -    int host_port, guest_port;
> +    struct sockaddr_storage host_addr, guest_addr;
>      const char *p;
>      char buf[256];
> -    int is_udp;
> -    char *end;
> -    const char *fail_reason = "Unknown reason";
> +    bool is_udp;
> +    Error *error = NULL;
> +    int flags = 0;
> +    int port;
>
> +    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";
> +    p = parse_protocol(p, &is_udp, &error);
> +    if (p == NULL) {
>          goto fail_syntax;
>      }
> -    if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
> -        fail_reason = "Bad host address";
> -        goto fail_syntax;
> +    if (is_udp) {
> +        flags |= SLIRP_HOSTFWD_UDP;
>      }
>
>      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";
> +        error_setg(&error, "missing host-guest separator");
>          goto fail_syntax;
>      }
>
> -    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
> -        fail_reason = "Missing guest address";
> +    if (parse_hostfwd_sockaddr(buf, is_udp ? SOCK_DGRAM : SOCK_STREAM,
> +                               &host_addr, &error) < 0) {
> +        error_prepend(&error, "For host address: ");
>          goto fail_syntax;
>      }
> -    if (buf[0] != '\0' && !inet_aton(buf, &guest_addr)) {
> -        fail_reason = "Bad guest address";
> +
> +    if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM,
> +                               &guest_addr, &error) < 0) {
> +        error_prepend(&error, "For 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";
> +    port = sockaddr_getport((struct sockaddr *) &guest_addr);
> +    if (port == 0) {
> +        error_setg(&error, "For guest address: invalid port '0'");
>          goto fail_syntax;
>      }
>
> -    if (slirp_add_hostfwd(s->slirp, is_udp, host_addr, host_port,
> guest_addr,
> -                          guest_port) < 0) {
> -        error_setg(errp, "Could not set up host forwarding rule '%s'",
> -                   redir_str);
> +    if (slirp_add_hostxfwd(s->slirp,
> +                           (struct sockaddr *) &host_addr,
> sizeof(host_addr),
> +                           (struct sockaddr *) &guest_addr,
> sizeof(guest_addr),
> +                           flags) < 0) {
> +        error_setg(errp, "Could not set up host forwarding rule '%s': %s",
> +                   redir_str, strerror(errno));
>          return -1;
>      }
>      return 0;
>
>   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..9b9db142c3
> --- /dev/null
> +++ b/tests/acceptance/hostfwd.py
> @@ -0,0 +1,91 @@
> +# 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': Address already in use\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 'abc'\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add abc::65022-:22'),
> +                          "Invalid host forwarding rule 'abc::65022-:22'"
> + \
> +                          " (bad protocol name 'abc')\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add :foo'),
> +                          "Invalid host forwarding rule ':foo'" + \
> +                          " (missing host-guest separator)\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: address resolution failed
> for" \
> +                          " 'a.b.c.d:66': Name or service not known)\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: address resolution
> failed" + \
> +                          " for 'a.b.c.d:66': Name or service not
> known)\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-:0'),
> +                          "Invalid host forwarding rule '::66-:0'" + \
> +                          " (For guest address: invalid port '0')\r\n")
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index c0069f2565..983efeee2f 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -46,23 +46,28 @@
>  #endif
>
>
> -static int inet_getport(struct addrinfo *e)
> +int sockaddr_getport(const struct sockaddr *addr)
>  {
> -    struct sockaddr_in *i4;
> -    struct sockaddr_in6 *i6;
> +    const struct sockaddr_in *i4;
> +    const struct sockaddr_in6 *i6;
>
> -    switch (e->ai_family) {
> +    switch (addr->sa_family) {
>      case PF_INET6:
> -        i6 = (void*)e->ai_addr;
> +        i6 = (void *)addr;
>          return ntohs(i6->sin6_port);
>      case PF_INET:
> -        i4 = (void*)e->ai_addr;
> +        i4 = (void *)addr;
>          return ntohs(i4->sin_port);
>      default:
>          return 0;
>      }
>  }
>
> +static int inet_getport(struct addrinfo *e)
> +{
> +    return sockaddr_getport(e->ai_addr);
> +}
> +
>  static void inet_setport(struct addrinfo *e, int port)
>  {
>      struct sockaddr_in *i4;
> --
> 2.31.1.295.g9ea45b61b8-goog
>
>
>

-- 
Marc-André Lureau

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

[PATCH v6 4/4] net: Extend host forwarding to support IPv6

[Doug Evans]

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

Tested:
avocado run tests/acceptance/hostfwd.py

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

Changes from v5:

Recognize ipv4=,ipv6= options.

 hmp-commands.hx             | 18 ++++++-
 net/slirp.c                 | 54 +++++++++++++++++----
 tests/acceptance/hostfwd.py | 94 +++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+), 11 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 435c591a1c..05f88e893b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1322,7 +1322,8 @@ ERST
     {
         .name       = "hostfwd_add",
         .args_type  = "arg1:s,arg2:s?",
-        .params     = "[netdev_id] [tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport",
+        .params     = "[netdev_id] [tcp|udp]:[hostaddr]:hostport\n"
+                      "[,ipv4=on|off][,ipv6=on|off]-[guestaddr]:guestport",
         .help       = "redirect TCP or UDP connections from host to guest (requires -net user)",
         .cmd        = hmp_hostfwd_add,
     },
@@ -1330,13 +1331,20 @@ ERST
 SRST
 ``hostfwd_add``
   Redirect TCP or UDP connections from host to guest (requires -net user).
+  IPV6 addresses are wrapped in square brackets, 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
+  hostfwd_add net0 ::10022,ipv6-:22
 ERST
 
 #ifdef CONFIG_SLIRP
     {
         .name       = "hostfwd_remove",
         .args_type  = "arg1:s,arg2:s?",
-        .params     = "[netdev_id] [tcp|udp]:[hostaddr]:hostport",
+        .params     = "[netdev_id] [tcp|udp]:[hostaddr]:hostport\n"
+                      "[,ipv4=on|off][,ipv6=on|off]",
         .help       = "remove host-to-guest TCP or UDP redirection",
         .cmd        = hmp_hostfwd_remove,
     },
@@ -1345,6 +1353,12 @@ ERST
 SRST
 ``hostfwd_remove``
   Remove host-to-guest TCP or UDP redirection.
+  IPV6 addresses are wrapped in square brackets, IPV4 addresses are not.
+
+  Examples:
+  hostfwd_remove net0 tcp:127.0.0.1:10022
+  hostfwd_remove net0 tcp:[::1]:10022
+  hostfwd_remove net0 ::10022,ipv6
 ERST
 
     {
diff --git a/net/slirp.c b/net/slirp.c
index 4be065c30b..82d4b71bef 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -664,25 +664,55 @@ static const char *parse_protocol(const char *str, bool *is_udp,
     return p;
 }
 
-static int parse_hostfwd_sockaddr(const char *str, int socktype,
+static int parse_hostfwd_sockaddr(const char *str, int family, int socktype,
                                   struct sockaddr_storage *saddr,
-                                  Error **errp)
+                                  bool *v6_only, Error **errp)
 {
     struct addrinfo hints, *res = NULL, *e;
     InetSocketAddress *addr = g_new(InetSocketAddress, 1);
     int gai_rc;
     int rc = -1;
+    Error *err = NULL;
 
     const char *optstr = inet_parse_host_port(addr, str, errp);
     if (optstr == NULL) {
         goto fail_return;
     }
 
+    if (inet_parse_ipv46(addr, optstr, errp) < 0) {
+        goto fail_return;
+    }
+
+    if (v6_only) {
+        bool v4 = addr->has_ipv4 && addr->ipv4;
+        bool v6 = addr->has_ipv6 && addr->ipv6;
+        *v6_only = v6 && !v4;
+    }
+
     memset(&hints, 0, sizeof(hints));
     hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */
     hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
     hints.ai_socktype = socktype;
-    hints.ai_family = PF_INET;
+    hints.ai_family = inet_ai_family_from_address(addr, &err);
+    if (err) {
+        error_propagate(errp, err);
+        goto fail_return;
+    }
+    if (family != PF_UNSPEC) {
+        /* Guest must use same family as host (for now). */
+        if (hints.ai_family != PF_UNSPEC && hints.ai_family != family) {
+            error_setg(errp,
+                       "unexpected address family for %s: expecting %s",
+                       str, family == PF_INET ? "ipv4" : "ipv6");
+            goto fail_return;
+        }
+        hints.ai_family = family;
+    }
+
+    /* For backward compatibility, treat an empty host spec as IPv4. */
+    if (*addr->host == '\0' && hints.ai_family == PF_UNSPEC) {
+        hints.ai_family = PF_INET;
+    }
 
     /*
      * Calling getaddrinfo for guest addresses is dubious, but addresses are
@@ -768,8 +798,8 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
         flags |= SLIRP_HOSTFWD_UDP;
     }
 
-    if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM,
-                               &host_addr, &error) < 0) {
+    if (parse_hostfwd_sockaddr(p, PF_UNSPEC, is_udp ? SOCK_DGRAM : SOCK_STREAM,
+                               &host_addr, /*v6_only=*/NULL, &error) < 0) {
         goto fail_syntax;
     }
 
@@ -794,6 +824,7 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
     Error *error = NULL;
     int flags = 0;
     int port;
+    bool v6_only;
 
     g_assert(redir_str != NULL);
     p = redir_str;
@@ -811,14 +842,19 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
         goto fail_syntax;
     }
 
-    if (parse_hostfwd_sockaddr(buf, is_udp ? SOCK_DGRAM : SOCK_STREAM,
-                               &host_addr, &error) < 0) {
+    if (parse_hostfwd_sockaddr(buf, PF_UNSPEC,
+                               is_udp ? SOCK_DGRAM : SOCK_STREAM,
+                               &host_addr, &v6_only, &error) < 0) {
         error_prepend(&error, "For host address: ");
         goto fail_syntax;
     }
+    if (v6_only) {
+        flags |= SLIRP_HOSTFWD_V6ONLY;
+    }
 
-    if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM,
-                               &guest_addr, &error) < 0) {
+    if (parse_hostfwd_sockaddr(p, host_addr.ss_family,
+                               is_udp ? SOCK_DGRAM : SOCK_STREAM,
+                               &guest_addr, /*v6_only=*/NULL, &error) < 0) {
         error_prepend(&error, "For guest address: ");
         goto fail_syntax;
     }
diff --git a/tests/acceptance/hostfwd.py b/tests/acceptance/hostfwd.py
index 9b9db142c3..f8493c5bdc 100644
--- a/tests/acceptance/hostfwd.py
+++ b/tests/acceptance/hostfwd.py
@@ -37,6 +37,17 @@ def test_qmp_hostfwd_ipv4(self):
         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')
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022,ipv4-:22'), '')
+        self.assertEquals(self.hmc('hostfwd_remove vnet tcp::65022,ipv4'),
+                          'host forwarding rule for tcp::65022,ipv4 removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022,ipv4=on-:22'),
+                          '')
+        self.assertEquals(self.hmc('hostfwd_remove vnet tcp::65022,ipv4=on'),
+                          'host forwarding rule for tcp::65022,ipv4=on removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022,ipv6=off-:22'),
+                          '')
+        self.assertEquals(self.hmc('hostfwd_remove vnet tcp::65022,ipv6=off'),
+                          'host forwarding rule for tcp::65022,ipv6=off removed\r\n')
 
     def test_qmp_hostfwd_ipv4_functional_errors(self):
         """Verify handling of various kinds of errors given valid addresses."""
@@ -89,3 +100,86 @@ 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: invalid port '0')\r\n")
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022,ipv4=abc-:22'),
+                          "Invalid host forwarding rule" + \
+                          " 'tcp::65022,ipv4=abc-:22' (For host address:" + \
+                          " error parsing 'ipv4' flag '=abc')\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 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')
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022,ipv6=on-:22'), '')
+        self.assertEquals(self.hmc('hostfwd_remove vnet tcp::65022,ipv6=on'),
+                          'host forwarding rule for tcp::65022,ipv6=on removed\r\n')
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022,ipv4=off-:22'), '')
+        self.assertEquals(self.hmc('hostfwd_remove vnet tcp::65022,ipv4=off'),
+                          'host forwarding rule for tcp::65022,ipv4=off 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': Address already in use\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: address resolution failed for" + \
+                          " '[:::]:66': Name or service not known)\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]:66-[fe80::1]:0'),
+                          "Invalid host forwarding rule ':[::1]:66-[fe80::1]:0'" + \
+                          " (For guest address: invalid port '0')\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'" + \
+                          " (For guest address: address resolution failed for" + \
+                          " '1.2.3.4:66': Address family for hostname not supported)\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'" + \
+                          " (For guest address: address resolution failed for" + \
+                          " '[fe80::1]:66': Address family for hostname not" + \
+                          " supported)\r\n")
+        self.assertEquals(self.hmc('hostfwd_add vnet tcp::65022,ipv6=abc-:22'),
+                          "Invalid host forwarding rule 'tcp::65022,ipv6=abc-:22'" + \
+                          " (For host address: error parsing 'ipv6' flag '=abc')\r\n")
-- 
2.31.1.295.g9ea45b61b8-goog

Re: [PATCH v6 0/4] Add support for ipv6 host forwarding

[Doug Evans]

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

Ping.

On Wed, Apr 14, 2021 at 8:39 PM Doug Evans <dje@google.com> wrote:

> 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/4.
>
> Changes from v5:
>
> 1/4 slirp: Advance libslirp submodule to current master
> NOTE TO REVIEWERS: It may be a better use of everyone's time if a
> maintainer takes on advancing QEMU's libslirp to libslirp's master.
> Beyond that, I really don't know what to do except submit this patch as
> is currently provided.
>
> 2/4: util/qemu-sockets.c: Split host:port parsing out of inet_parse
>
> Also split out parsing of ipv4=on|off, ipv6=on|off
>
> 3/4: net/slirp.c: Refactor address parsing
>
> Use InetSocketAddress and getaddrinfo().
> Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.
>
> 4/4: net: Extend host forwarding to support IPv6
>
> Recognize ipv4=,ipv6= options.
>
> Note: v5's 3/5 "Recognize []:port (empty ipv6 address)" has been deleted:
> the churn on this patch series needs to be reduced.
> This change is not required, and can easily be done in a later patch.
>
> 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 (4):
>   slirp: Advance libslirp submodule to add ipv6 host-forward support
>   util/qemu-sockets.c: Split host:port parsing out of inet_parse
>   net/slirp.c: Refactor address parsing
>   net: Extend host forwarding to support IPv6
>
>  hmp-commands.hx             |  18 ++-
>  include/qemu/sockets.h      |   5 +
>  net/slirp.c                 | 236 ++++++++++++++++++++++++++----------
>  slirp                       |   2 +-
>  tests/acceptance/hostfwd.py | 185 ++++++++++++++++++++++++++++
>  util/qemu-sockets.c         |  82 +++++++++----
>  6 files changed, 436 insertions(+), 92 deletions(-)
>  create mode 100644 tests/acceptance/hostfwd.py
>
> --
> 2.31.1.295.g9ea45b61b8-goog
>
>

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

Re: [PATCH v6 0/4] Add support for ipv6 host forwarding

[Doug Evans]

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

Ping.

On Wed, Apr 28, 2021 at 8:37 PM Doug Evans <dje@google.com> wrote:

> Ping.
>
> On Wed, Apr 14, 2021 at 8:39 PM Doug Evans <dje@google.com> wrote:
>
>> 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/4.
>>
>> Changes from v5:
>>
>> 1/4 slirp: Advance libslirp submodule to current master
>> NOTE TO REVIEWERS: It may be a better use of everyone's time if a
>> maintainer takes on advancing QEMU's libslirp to libslirp's master.
>> Beyond that, I really don't know what to do except submit this patch as
>> is currently provided.
>>
>> 2/4: util/qemu-sockets.c: Split host:port parsing out of inet_parse
>>
>> Also split out parsing of ipv4=on|off, ipv6=on|off
>>
>> 3/4: net/slirp.c: Refactor address parsing
>>
>> Use InetSocketAddress and getaddrinfo().
>> Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.
>>
>> 4/4: net: Extend host forwarding to support IPv6
>>
>> Recognize ipv4=,ipv6= options.
>>
>> Note: v5's 3/5 "Recognize []:port (empty ipv6 address)" has been deleted:
>> the churn on this patch series needs to be reduced.
>> This change is not required, and can easily be done in a later patch.
>>
>> 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 (4):
>>   slirp: Advance libslirp submodule to add ipv6 host-forward support
>>   util/qemu-sockets.c: Split host:port parsing out of inet_parse
>>   net/slirp.c: Refactor address parsing
>>   net: Extend host forwarding to support IPv6
>>
>>  hmp-commands.hx             |  18 ++-
>>  include/qemu/sockets.h      |   5 +
>>  net/slirp.c                 | 236 ++++++++++++++++++++++++++----------
>>  slirp                       |   2 +-
>>  tests/acceptance/hostfwd.py | 185 ++++++++++++++++++++++++++++
>>  util/qemu-sockets.c         |  82 +++++++++----
>>  6 files changed, 436 insertions(+), 92 deletions(-)
>>  create mode 100644 tests/acceptance/hostfwd.py
>>
>> --
>> 2.31.1.295.g9ea45b61b8-goog
>>
>>

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

Re: [PATCH v6 0/4] Add support for ipv6 host forwarding

[Philippe Mathieu-Daudé]

Cc'ing Marc-André

On 5/5/21 5:21 PM, Doug Evans wrote:
> Ping.
> 
> On Wed, Apr 28, 2021 at 8:37 PM Doug Evans <dje@google.com
> <mailto:dje@google.com>> wrote:
> 
>     Ping.
> 
>     On Wed, Apr 14, 2021 at 8:39 PM Doug Evans <dje@google.com
>     <mailto:dje@google.com>> wrote:
> 
>         This patchset takes the original patch from Maxim,
>         https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html <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/4.
> 
>         Changes from v5:
> 
>         1/4 slirp: Advance libslirp submodule to current master
>         NOTE TO REVIEWERS: It may be a better use of everyone's time if a
>         maintainer takes on advancing QEMU's libslirp to libslirp's master.
>         Beyond that, I really don't know what to do except submit this
>         patch as
>         is currently provided.
> 
>         2/4: util/qemu-sockets.c: Split host:port parsing out of inet_parse
> 
>         Also split out parsing of ipv4=on|off, ipv6=on|off
> 
>         3/4: net/slirp.c: Refactor address parsing
> 
>         Use InetSocketAddress and getaddrinfo().
>         Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.
> 
>         4/4: net: Extend host forwarding to support IPv6
> 
>         Recognize ipv4=,ipv6= options.
> 
>         Note: v5's 3/5 "Recognize []:port (empty ipv6 address)" has been
>         deleted:
>         the churn on this patch series needs to be reduced.
>         This change is not required, and can easily be done in a later
>         patch.
> 
>         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 (4):
>           slirp: Advance libslirp submodule to add ipv6 host-forward support
>           util/qemu-sockets.c: Split host:port parsing out of inet_parse
>           net/slirp.c: Refactor address parsing
>           net: Extend host forwarding to support IPv6
> 
>          hmp-commands.hx             |  18 ++-
>          include/qemu/sockets.h      |   5 +
>          net/slirp.c                 | 236
>         ++++++++++++++++++++++++++----------
>          slirp                       |   2 +-
>          tests/acceptance/hostfwd.py | 185 ++++++++++++++++++++++++++++
>          util/qemu-sockets.c         |  82 +++++++++----
>          6 files changed, 436 insertions(+), 92 deletions(-)
>          create mode 100644 tests/acceptance/hostfwd.py
> 
>         -- 
>         2.31.1.295.g9ea45b61b8-goog
> 

Re: [PATCH v6 0/4] Add support for ipv6 host forwarding

[Philippe Mathieu-Daudé]

Well Maxim review would help too, so Cc'ing him.

On 5/5/21 6:13 PM, Philippe Mathieu-Daudé wrote:
> Cc'ing Marc-André
> 
> On 5/5/21 5:21 PM, Doug Evans wrote:
>> Ping.
>>
>> On Wed, Apr 28, 2021 at 8:37 PM Doug Evans <dje@google.com
>> <mailto:dje@google.com>> wrote:
>>
>>     Ping.
>>
>>     On Wed, Apr 14, 2021 at 8:39 PM Doug Evans <dje@google.com
>>     <mailto:dje@google.com>> wrote:
>>
>>         This patchset takes the original patch from Maxim,
>>         https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html <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/4.
>>
>>         Changes from v5:
>>
>>         1/4 slirp: Advance libslirp submodule to current master
>>         NOTE TO REVIEWERS: It may be a better use of everyone's time if a
>>         maintainer takes on advancing QEMU's libslirp to libslirp's master.
>>         Beyond that, I really don't know what to do except submit this
>>         patch as
>>         is currently provided.
>>
>>         2/4: util/qemu-sockets.c: Split host:port parsing out of inet_parse
>>
>>         Also split out parsing of ipv4=on|off, ipv6=on|off
>>
>>         3/4: net/slirp.c: Refactor address parsing
>>
>>         Use InetSocketAddress and getaddrinfo().
>>         Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.
>>
>>         4/4: net: Extend host forwarding to support IPv6
>>
>>         Recognize ipv4=,ipv6= options.
>>
>>         Note: v5's 3/5 "Recognize []:port (empty ipv6 address)" has been
>>         deleted:
>>         the churn on this patch series needs to be reduced.
>>         This change is not required, and can easily be done in a later
>>         patch.
>>
>>         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 (4):
>>           slirp: Advance libslirp submodule to add ipv6 host-forward support
>>           util/qemu-sockets.c: Split host:port parsing out of inet_parse
>>           net/slirp.c: Refactor address parsing
>>           net: Extend host forwarding to support IPv6
>>
>>          hmp-commands.hx             |  18 ++-
>>          include/qemu/sockets.h      |   5 +
>>          net/slirp.c                 | 236
>>         ++++++++++++++++++++++++++----------
>>          slirp                       |   2 +-
>>          tests/acceptance/hostfwd.py | 185 ++++++++++++++++++++++++++++
>>          util/qemu-sockets.c         |  82 +++++++++----
>>          6 files changed, 436 insertions(+), 92 deletions(-)
>>          create mode 100644 tests/acceptance/hostfwd.py
>>
>>         -- 
>>         2.31.1.295.g9ea45b61b8-goog
>>
>