hmp-commands.hx | 32 +++++ include/net/slirp.h | 2 + net/slirp.c | 310 +++++++++++++++++++++++++++++++++++--------- qapi/net.json | 4 + slirp | 2 +- 5 files changed, 285 insertions(+), 65 deletions(-)
Add support for ipv6 host forwarding This patchset takes the original patch from Maxim, https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html and updates it. New option: -ipv6-hostfwd New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove These are the ipv6 equivalents of their ipv4 counterparts. The libslirp part of the patch has been committed upstream, and will require adding it to qemu. https://gitlab.freedesktop.org/slirp/libslirp/-/commit/0624fbe7d39b5433d7084a5096d1effc1df74e39 [plus the subsequent merge commit] 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 (3): slirp: Placeholder for libslirp ipv6 hostfwd support net/slirp.c: Refactor address parsing net: Add -ipv6-hostfwd option, ipv6_hostfwd_add/remove commands hmp-commands.hx | 32 +++++ include/net/slirp.h | 2 + net/slirp.c | 310 +++++++++++++++++++++++++++++++++++--------- qapi/net.json | 4 + slirp | 2 +- 5 files changed, 285 insertions(+), 65 deletions(-) -- 2.30.0.365.g02bc693789-goog
On Wed, Feb 03, 2021 at 03:35:36PM -0800, dje--- via wrote: > Add support for ipv6 host forwarding > > This patchset takes the original patch from Maxim, > https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html > and updates it. > > New option: -ipv6-hostfwd > > New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove > > These are the ipv6 equivalents of their ipv4 counterparts. Before I noticed this v3, I send a reply to your v2 sugesting that we don't need to add any new commands/options. We can use existing inet_parse() helper function to parse the address info and transparently support IPv4/6 in the existing commands and options. This matches normal practice elsewhere in QEMU for IP dual stack. 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 :|
On Thu, Feb 4, 2021 at 2:03 AM Daniel P. Berrangé <berrange@redhat.com> wrote: > On Wed, Feb 03, 2021 at 03:35:36PM -0800, dje--- via wrote: > > Add support for ipv6 host forwarding > > > > This patchset takes the original patch from Maxim, > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html > > and updates it. > > > > New option: -ipv6-hostfwd > > > > New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove > > > > These are the ipv6 equivalents of their ipv4 counterparts. > > Before I noticed this v3, I send a reply to your v2 sugesting > that we don't need to add any new commands/options. We can > use existing inet_parse() helper function to parse the address > info and transparently support IPv4/6 in the existing commands > and options. This matches normal practice elsewhere in QEMU > for IP dual stack. > I'm all for this, fwiw.
On Thu, Feb 4, 2021 at 10:25 AM Doug Evans <dje@google.com> wrote: > On Thu, Feb 4, 2021 at 2:03 AM Daniel P. Berrangé <berrange@redhat.com> > wrote: > >> On Wed, Feb 03, 2021 at 03:35:36PM -0800, dje--- via wrote: >> > Add support for ipv6 host forwarding >> > >> > This patchset takes the original patch from Maxim, >> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html >> > and updates it. >> > >> > New option: -ipv6-hostfwd >> > >> > New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove >> > >> > These are the ipv6 equivalents of their ipv4 counterparts. >> >> Before I noticed this v3, I send a reply to your v2 sugesting >> that we don't need to add any new commands/options. We can >> use existing inet_parse() helper function to parse the address >> info and transparently support IPv4/6 in the existing commands >> and options. This matches normal practice elsewhere in QEMU >> for IP dual stack. >> > > I'm all for this, fwiw. > I should say I'm all for not adding new commands/options. Looking at inet_parse() it cannot be used as-is. The question then becomes: Will refactoring it buy enough?
On Tue, Feb 09, 2021 at 06:16:57PM -0800, Doug Evans wrote: > On Thu, Feb 4, 2021 at 10:25 AM Doug Evans <dje@google.com> wrote: > > > On Thu, Feb 4, 2021 at 2:03 AM Daniel P. Berrangé <berrange@redhat.com> > > wrote: > > > >> On Wed, Feb 03, 2021 at 03:35:36PM -0800, dje--- via wrote: > >> > Add support for ipv6 host forwarding > >> > > >> > This patchset takes the original patch from Maxim, > >> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html > >> > and updates it. > >> > > >> > New option: -ipv6-hostfwd > >> > > >> > New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove > >> > > >> > These are the ipv6 equivalents of their ipv4 counterparts. > >> > >> Before I noticed this v3, I send a reply to your v2 sugesting > >> that we don't need to add any new commands/options. We can > >> use existing inet_parse() helper function to parse the address > >> info and transparently support IPv4/6 in the existing commands > >> and options. This matches normal practice elsewhere in QEMU > >> for IP dual stack. > >> > > > > I'm all for this, fwiw. > > > > > I should say I'm all for not adding new commands/options. > Looking at inet_parse() it cannot be used as-is. > The question then becomes: Will refactoring it buy enough? What's the problem your hitting with inet_parse ? 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 :|
On Wed, Feb 10, 2021 at 1:31 AM Daniel P. Berrangé <berrange@redhat.com> wrote: > On Tue, Feb 09, 2021 at 06:16:57PM -0800, Doug Evans wrote: > > On Thu, Feb 4, 2021 at 10:25 AM Doug Evans <dje@google.com> wrote: > > > > > On Thu, Feb 4, 2021 at 2:03 AM Daniel P. Berrangé <berrange@redhat.com > > > > > wrote: > > > > > >> On Wed, Feb 03, 2021 at 03:35:36PM -0800, dje--- via wrote: > > >> > Add support for ipv6 host forwarding > > >> > > > >> > This patchset takes the original patch from Maxim, > > >> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html > > >> > and updates it. > > >> > > > >> > New option: -ipv6-hostfwd > > >> > > > >> > New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove > > >> > > > >> > These are the ipv6 equivalents of their ipv4 counterparts. > > >> > > >> Before I noticed this v3, I send a reply to your v2 sugesting > > >> that we don't need to add any new commands/options. We can > > >> use existing inet_parse() helper function to parse the address > > >> info and transparently support IPv4/6 in the existing commands > > >> and options. This matches normal practice elsewhere in QEMU > > >> for IP dual stack. > > >> > > > > > > I'm all for this, fwiw. > > > > > > > > > I should say I'm all for not adding new commands/options. > > Looking at inet_parse() it cannot be used as-is. > > The question then becomes: Will refactoring it buy enough? > > What's the problem your hitting with inet_parse ? > First, this is the inet_parse() function we're talking about, right? int inet_parse(InetSocketAddress *addr, const char *str, Error **errp) https://gitlab.com/qemu-project/qemu/-/blob/master/util/qemu-sockets.c#L618
On Wed, Feb 10, 2021 at 08:31:40AM -0800, Doug Evans wrote: > On Wed, Feb 10, 2021 at 1:31 AM Daniel P. Berrangé <berrange@redhat.com> > wrote: > > > On Tue, Feb 09, 2021 at 06:16:57PM -0800, Doug Evans wrote: > > > On Thu, Feb 4, 2021 at 10:25 AM Doug Evans <dje@google.com> wrote: > > > > > > > On Thu, Feb 4, 2021 at 2:03 AM Daniel P. Berrangé <berrange@redhat.com > > > > > > > wrote: > > > > > > > >> On Wed, Feb 03, 2021 at 03:35:36PM -0800, dje--- via wrote: > > > >> > Add support for ipv6 host forwarding > > > >> > > > > >> > This patchset takes the original patch from Maxim, > > > >> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html > > > >> > and updates it. > > > >> > > > > >> > New option: -ipv6-hostfwd > > > >> > > > > >> > New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove > > > >> > > > > >> > These are the ipv6 equivalents of their ipv4 counterparts. > > > >> > > > >> Before I noticed this v3, I send a reply to your v2 sugesting > > > >> that we don't need to add any new commands/options. We can > > > >> use existing inet_parse() helper function to parse the address > > > >> info and transparently support IPv4/6 in the existing commands > > > >> and options. This matches normal practice elsewhere in QEMU > > > >> for IP dual stack. > > > >> > > > > > > > > I'm all for this, fwiw. > > > > > > > > > > > > > I should say I'm all for not adding new commands/options. > > > Looking at inet_parse() it cannot be used as-is. > > > The question then becomes: Will refactoring it buy enough? > > > > What's the problem your hitting with inet_parse ? > > > > > First, this is the inet_parse() function we're talking about, right? > > int inet_parse(InetSocketAddress *addr, const char *str, Error **errp) > > https://gitlab.com/qemu-project/qemu/-/blob/master/util/qemu-sockets.c#L618 Yes, that's right. 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 :|
On Wed, Feb 10, 2021 at 8:49 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> On Wed, Feb 10, 2021 at 08:31:40AM -0800, Doug Evans wrote:
> > On Wed, Feb 10, 2021 at 1:31 AM Daniel P. Berrangé <berrange@redhat.com>
> > wrote:
> >
> > > On Tue, Feb 09, 2021 at 06:16:57PM -0800, Doug Evans wrote:
> > > > On Thu, Feb 4, 2021 at 10:25 AM Doug Evans <dje@google.com> wrote:
> > > >
> > > > > On Thu, Feb 4, 2021 at 2:03 AM Daniel P. Berrangé <
> berrange@redhat.com
> > > >
> > > > > wrote:
> > > > >
> > > > >> On Wed, Feb 03, 2021 at 03:35:36PM -0800, dje--- via wrote:
> > > > >> > Add support for ipv6 host forwarding
> > > > >> >
> > > > >> > This patchset takes the original patch from Maxim,
> > > > >> >
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html
> > > > >> > and updates it.
> > > > >> >
> > > > >> > New option: -ipv6-hostfwd
> > > > >> >
> > > > >> > New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove
> > > > >> >
> > > > >> > These are the ipv6 equivalents of their ipv4 counterparts.
> > > > >>
> > > > >> Before I noticed this v3, I send a reply to your v2 sugesting
> > > > >> that we don't need to add any new commands/options. We can
> > > > >> use existing inet_parse() helper function to parse the address
> > > > >> info and transparently support IPv4/6 in the existing commands
> > > > >> and options. This matches normal practice elsewhere in QEMU
> > > > >> for IP dual stack.
> > > > >>
> > > > >
> > > > > I'm all for this, fwiw.
> > > > >
> > > >
> > > >
> > > > I should say I'm all for not adding new commands/options.
> > > > Looking at inet_parse() it cannot be used as-is.
> > > > The question then becomes: Will refactoring it buy enough?
> > >
> > > What's the problem your hitting with inet_parse ?
> > >
> >
> >
> > First, this is the inet_parse() function we're talking about, right?
> >
> > int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
> >
> >
> https://gitlab.com/qemu-project/qemu/-/blob/master/util/qemu-sockets.c#L618
>
> Yes, that's right.
>
Thanks, just wanted to be sure.
The syntax it supports is not the same as what's needed for host forwarding.
inet_parse: address:port,option1,option2 (where options are to=,ipv4,etc).
hostfwd: address:port-address:port
If we wanted to have a utility that parsed "address:port" for v4+v6 then
we'd have to split the "address:port" part out of inet_parse.
Plus the way inet_parse() parses the address, which is fine for its
purposes, is with sscanf.
Alas the terminating character is not the same (',' vs '-').
IWBN to retain passing sscanf a constant format string so that the compiler
can catch various errors,
and if one keeps that then any kind of refactor loses some appeal.
[Though one could require all callers to accept either ',' or '-' as the
delimiter.]
On Wed, Feb 10, 2021 at 02:40:17PM -0800, Doug Evans wrote:
> On Wed, Feb 10, 2021 at 8:49 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
>
> > On Wed, Feb 10, 2021 at 08:31:40AM -0800, Doug Evans wrote:
> > > On Wed, Feb 10, 2021 at 1:31 AM Daniel P. Berrangé <berrange@redhat.com>
> > > wrote:
> > >
> > > > On Tue, Feb 09, 2021 at 06:16:57PM -0800, Doug Evans wrote:
> > > > > On Thu, Feb 4, 2021 at 10:25 AM Doug Evans <dje@google.com> wrote:
> > > > >
> > > > > > On Thu, Feb 4, 2021 at 2:03 AM Daniel P. Berrangé <
> > berrange@redhat.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > >> On Wed, Feb 03, 2021 at 03:35:36PM -0800, dje--- via wrote:
> > > > > >> > Add support for ipv6 host forwarding
> > > > > >> >
> > > > > >> > This patchset takes the original patch from Maxim,
> > > > > >> >
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html
> > > > > >> > and updates it.
> > > > > >> >
> > > > > >> > New option: -ipv6-hostfwd
> > > > > >> >
> > > > > >> > New commands: ipv6_hostfwd_add, ipv6_hostfwd_remove
> > > > > >> >
> > > > > >> > These are the ipv6 equivalents of their ipv4 counterparts.
> > > > > >>
> > > > > >> Before I noticed this v3, I send a reply to your v2 sugesting
> > > > > >> that we don't need to add any new commands/options. We can
> > > > > >> use existing inet_parse() helper function to parse the address
> > > > > >> info and transparently support IPv4/6 in the existing commands
> > > > > >> and options. This matches normal practice elsewhere in QEMU
> > > > > >> for IP dual stack.
> > > > > >>
> > > > > >
> > > > > > I'm all for this, fwiw.
> > > > > >
> > > > >
> > > > >
> > > > > I should say I'm all for not adding new commands/options.
> > > > > Looking at inet_parse() it cannot be used as-is.
> > > > > The question then becomes: Will refactoring it buy enough?
> > > >
> > > > What's the problem your hitting with inet_parse ?
> > > >
> > >
> > >
> > > First, this is the inet_parse() function we're talking about, right?
> > >
> > > int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
> > >
> > >
> > https://gitlab.com/qemu-project/qemu/-/blob/master/util/qemu-sockets.c#L618
> >
> > Yes, that's right.
> >
>
>
> Thanks, just wanted to be sure.
>
> The syntax it supports is not the same as what's needed for host forwarding.
> inet_parse: address:port,option1,option2 (where options are to=,ipv4,etc).
> hostfwd: address:port-address:port
> If we wanted to have a utility that parsed "address:port" for v4+v6 then
> we'd have to split the "address:port" part out of inet_parse.
>
> Plus the way inet_parse() parses the address, which is fine for its
> purposes, is with sscanf.
> Alas the terminating character is not the same (',' vs '-').
> IWBN to retain passing sscanf a constant format string so that the compiler
> can catch various errors,
> and if one keeps that then any kind of refactor loses some appeal.
> [Though one could require all callers to accept either ',' or '-' as the
> delimiter.]
I think the key useful part to keep common impl for is the handling
of the [] brackets for IPv6 raw addrs. I'd suggest we try to pull the
"address:port" part out into a new inet_addr_parse() helper that can be
called from inet_pase and from slirp.
inet_parse() can split on the first ",", and then call inet_addr_parse
on the first segment.
slirp can split on "-", and call inet_addr_parse with both segments.
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 :|
On Thu, Feb 11, 2021 at 1:12 AM Daniel P. Berrangé <berrange@redhat.com> wrote: > [...] > > I think the key useful part to keep common impl for is the handling > of the [] brackets for IPv6 raw addrs. I'd suggest we try to pull the > "address:port" part out into a new inet_addr_parse() helper that can be > called from inet_pase and from slirp. > > inet_parse() can split on the first ",", and then call inet_addr_parse > on the first segment. > > slirp can split on "-", and call inet_addr_parse with both segments. > v4 here: https://lists.nongnu.org/archive/html/qemu-devel/2021-02/msg06011.html
Patchew URL: https://patchew.org/QEMU/20210203233539.1990032-1-dje@google.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210203233539.1990032-1-dje@google.com
Subject: [PATCH v3 0/3]
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20210203213729.1940893-1-dje@google.com -> patchew/20210203213729.1940893-1-dje@google.com
* [new tag] patchew/20210203233539.1990032-1-dje@google.com -> patchew/20210203233539.1990032-1-dje@google.com
Switched to a new branch 'test'
998dd12 net: Add -ipv6-hostfwd option, ipv6_hostfwd_add/remove commands
e6a45e5 net/slirp.c: Refactor address parsing
6faf9a9 slirp: Placeholder for libslirp ipv6 hostfwd support
=== OUTPUT BEGIN ===
1/3 Checking commit 6faf9a93b1cf (slirp: Placeholder for libslirp ipv6 hostfwd support)
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 2 lines checked
Patch 1/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/3 Checking commit e6a45e50689f (net/slirp.c: Refactor address parsing)
3/3 Checking commit 998dd12e51aa (net: Add -ipv6-hostfwd option, ipv6_hostfwd_add/remove commands)
ERROR: line over 90 characters
#145: FILE: net/slirp.c:748:
+ *errmsg = g_strdup_printf("Missing %s address separator after IPv6 address", kind);
total: 1 errors, 0 warnings, 250 lines checked
Patch 3/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210203233539.1990032-1-dje@google.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
© 2016 - 2024 Red Hat, Inc.