Re: [PATCH v2 2/2] net: Add -ipv6-hostfwd option, ipv6_hostfwd_add/remove commands

[Daniel P . Berrangé]

On Wed, Feb 03, 2021 at 01:37:29PM -0800, dje--- via wrote:
> These are identical to their ipv4 counterparts, but for ipv6.
> 
> Signed-off-by: Doug Evans 
> ---
>  hmp-commands.hx |  28 ++
>  include/net/slirp.h |   2 +
>  net/slirp.c | 129 +++-
>  qapi/net.json   |   4 ++
>  4 files changed, 161 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d4001f9c5d..bd51173472 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1392,6 +1392,34 @@ SRST
>Remove host-to-guest TCP or UDP redirection.
>  ERST
>  
> +#ifdef CONFIG_SLIRP
> +{
> +.name   = "ipv6_hostfwd_add",
> +.args_type  = "arg1:s,arg2:s?",
> +.params = "[netdev_id] 
> [tcp|udp]:[hostaddr6]:hostport-[guestaddr6]:guestport",
> +.help   = "redirect TCP6 or UDP6 connections from host to guest 
> (requires -net user)",
> +.cmd= hmp_ipv6_hostfwd_add,
> +},
> +#endif
> +SRST
> +``ipv6_hostfwd_add``
> +  Redirect TCP6 or UDP6 connections from host to guest (requires -net user).
> +ERST
> +
> +#ifdef CONFIG_SLIRP
> +{
> +.name   = "ipv6_hostfwd_remove",
> +.args_type  = "arg1:s,arg2:s?",
> +.params = "[netdev_id] [tcp|udp]:[hostaddr6]:hostport",
> +.help   = "remove host-to-guest TCP6 or UDP6 redirection",
> +.cmd= hmp_ipv6_hostfwd_remove,
> +},
> +#endif
> +SRST
> +``ipv6_hostfwd_remove``
> +  Remove host-to-guest TCP6 or UDP6 redirection.
> +ERST

DO we really need new commands for this ? It seems to me that we
can reliably distinction IPv4 vs v6 from the address format, and
thus existing commands can be adapted to support both.

This is the way other command line options and monitor commands
work for IPv4 vs IPv6 elsewhere in QEMU, so I think consistency
is beneficial.  We already have the helper method inet_parse()
that can do this parsing.

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: [PATCH v2 2/2] net: Add -ipv6-hostfwd option, ipv6_hostfwd_add/remove commands

[Doug Evans]

On Wed, Feb 3, 2021 at 2:20 PM Samuel Thibault 
wrote:

> Doug Evans, le mer. 03 févr. 2021 13:37:29 -0800, a ecrit:
> > @@ -1392,6 +1392,34 @@ SRST
> >Remove host-to-guest TCP or UDP redirection.
> >  ERST
> >
> > +#ifdef CONFIG_SLIRP
> > +{
> > +.name   = "ipv6_hostfwd_add",
> > +.args_type  = "arg1:s,arg2:s?",
> > +.params = "[netdev_id]
> [tcp|udp]:[hostaddr6]:hostport-[guestaddr6]:guestport",
>
> Perhaps explicit that the IPv6 address should be enclosed with [] ?
>


Yeah, totally open to suggestions for what to write.
I wasn't sure how to do that without getting klunky,


> > +/* Ignore the part between the ']' and addr_sep. */
> > +if (get_str_sep(buf, sizeof(buf), &p, addr_sep) < 0) {
>
> Mmm, I would say that we do not want to just ignore it, and rather make
> sure that it is empty, so that we can possibly make extensions later
> without breaking existing misuse.
>


Completely agree.

Re: [PATCH v2 2/2] net: Add -ipv6-hostfwd option, ipv6_hostfwd_add/remove commands

[Samuel Thibault]

Doug Evans, le mer. 03 févr. 2021 13:37:29 -0800, a ecrit:
> @@ -1392,6 +1392,34 @@ SRST
>Remove host-to-guest TCP or UDP redirection.
>  ERST
>  
> +#ifdef CONFIG_SLIRP
> +{
> +.name   = "ipv6_hostfwd_add",
> +.args_type  = "arg1:s,arg2:s?",
> +.params = "[netdev_id] 
> [tcp|udp]:[hostaddr6]:hostport-[guestaddr6]:guestport",

Perhaps explicit that the IPv6 address should be enclosed with [] ?

> +/* Ignore the part between the ']' and addr_sep. */
> +if (get_str_sep(buf, sizeof(buf), &p, addr_sep) < 0) {

Mmm, I would say that we do not want to just ignore it, and rather make
sure that it is empty, so that we can possibly make extensions later
without breaking existing misuse.

Samuel