All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/1] slirp: Add domainname option to slirp's DHCP server
@ 2018-02-16 12:55 Benjamin Drung
  2018-02-16 12:55 ` [Qemu-devel] [PATCH 1/1] " Benjamin Drung
  2018-02-17 21:16 ` [Qemu-devel] [PATCH 0/1] " Samuel Thibault
  0 siblings, 2 replies; 20+ messages in thread
From: Benjamin Drung @ 2018-02-16 12:55 UTC (permalink / raw)
  To: Samuel Thibault, Jan Kiszka; +Cc: qemu-devel, Benjamin Drung

Hi,

The DHCP server in QEMU does neither support specifying a domainname nor
a classless static route option [1]. I implemented the former (which was easy),
but I am not sure if and how to implement the classless static route option.

An example classless static route option configured in ISC's DHCP server look
like

  option rfc3442-classless-static-routes 24, 10, 0, 2, 10, 0, 2, 2, 16, 192, 168, 10, 0, 2, 2;

This translates to the routes:

10.0.2.0/24 via 10.0.2.2
192.168.0.0/16 via 10.0.2.2

Should the command line option just behave like the ISC's DHCP server and
allow specifying the byte stream passed to the client? E.g.

  -net staticroutes=24.10.0.2.10.0.2.2.16.192.168.10.0.2.2

Or should the command line option be simpler, but how should it be specified
then? Maybe

  -net staticroute=10.0.2.0/24via10.0.2.2,staticroute=192.168.0.0/16via10.0.2.2

I am open for your ideas.

[1] https://tools.ietf.org/html/rfc3442

Benjamin Drung (1):
  slirp: Add domainname option to slirp's DHCP server

 net/slirp.c      | 7 ++++---
 qapi/net.json    | 3 +++
 qemu-options.hx  | 7 +++++--
 slirp/bootp.c    | 8 ++++++++
 slirp/libslirp.h | 2 +-
 slirp/slirp.c    | 4 +++-
 slirp/slirp.h    | 1 +
 7 files changed, 25 insertions(+), 7 deletions(-)

-- 
2.14.1

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

* [Qemu-devel] [PATCH 1/1] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 12:55 [Qemu-devel] [PATCH 0/1] slirp: Add domainname option to slirp's DHCP server Benjamin Drung
@ 2018-02-16 12:55 ` Benjamin Drung
  2018-02-16 15:15   ` Eric Blake
  2018-02-17 21:16 ` [Qemu-devel] [PATCH 0/1] " Samuel Thibault
  1 sibling, 1 reply; 20+ messages in thread
From: Benjamin Drung @ 2018-02-16 12:55 UTC (permalink / raw)
  To: Samuel Thibault, Jan Kiszka; +Cc: qemu-devel, Benjamin Drung

This patch will allow the user to include the domainname option in
replies from the built-in DHCP server.

Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
---
 net/slirp.c      | 7 ++++---
 qapi/net.json    | 3 +++
 qemu-options.hx  | 7 +++++--
 slirp/bootp.c    | 8 ++++++++
 slirp/libslirp.h | 2 +-
 slirp/slirp.c    | 4 +++-
 slirp/slirp.h    | 1 +
 7 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 8991816bbf..9511ff3bb7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *bootfile, const char *vdhcp_start,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
-                          const char **dnssearch, Error **errp)
+                          const char **dnssearch, const char *vdomainname,
+                          Error **errp)
 {
     /* default settings according to historic slirp */
     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -371,7 +372,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     s->slirp = slirp_init(restricted, ipv4, net, mask, host,
                           ipv6, ip6_prefix, vprefix6_len, ip6_host,
                           vhostname, tftp_export, bootfile, dhcp,
-                          dns, ip6_dns, dnssearch, s);
+                          dns, ip6_dns, dnssearch, vdomainname, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
     for (config = slirp_configs; config; config = config->next) {
@@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, errp);
+                         user->smbserver, dnssearch, user->domainname, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index 1238ba5de1..5a93fe6856 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -160,6 +160,8 @@
 # @dnssearch: list of DNS suffixes to search, passed as DHCP option
 #             to the guest
 #
+# @domainname: guest-visible domain name of the virtual nameserver
+#
 # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
 #               2.6). The network prefix is given in the usual
 #               hexadecimal IPv6 address notation.
@@ -197,6 +199,7 @@
     '*dhcpstart': 'str',
     '*dns':       'str',
     '*dnssearch': ['String'],
+    '*domainname': 'str',
     '*ipv6-prefix':      'str',
     '*ipv6-prefixlen':   'int',
     '*ipv6-host':        'str',
diff --git a/qemu-options.hx b/qemu-options.hx
index 5050a49a5e..06983db7be 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
     "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
-    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n"
-    "         [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
+    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
 #ifndef _WIN32
                                              "[,smb=dir[,smbserver=addr]]\n"
 #endif
@@ -2116,6 +2116,9 @@ Example:
 qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
 @end example
 
+@item domainname=@var{domain}
+Specifies the client domain name reported by the built-in DHCP server.
+
 @item tftp=@var{dir}
 When using the user mode network stack, activate a built-in TFTP
 server. The files in @var{dir} will be exposed as the root of a TFTP server.
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5dd1a415b5..91896f7400 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
             q += val;
         }
 
+        if (*slirp->vdomainname) {
+            val = strlen(slirp->vdomainname);
+            *q++ = RFC1533_DOMAINNAME;
+            *q++ = val;
+            memcpy(q, slirp->vdomainname, val);
+            q += val;
+        }
+
         if (slirp->vdnssearch) {
             size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
             val = slirp->vdnssearch_len;
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 540b3e5903..740408a96e 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque);
+                  const char *vdomainname, void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
 void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1cb6b07004..4f29753444 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque)
+                  const char *vdomainname, void *opaque)
 {
     Slirp *slirp = g_malloc0(sizeof(Slirp));
 
@@ -317,6 +317,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
     }
     slirp->tftp_prefix = g_strdup(tftp_path);
     slirp->bootp_filename = g_strdup(bootfile);
+    slirp->vdomainname = g_strdup(vdomainname);
     slirp->vdhcp_startaddr = vdhcp_start;
     slirp->vnameserver_addr = vnameserver;
     slirp->vnameserver_addr6 = vnameserver6;
@@ -349,6 +350,7 @@ void slirp_cleanup(Slirp *slirp)
     g_free(slirp->vdnssearch);
     g_free(slirp->tftp_prefix);
     g_free(slirp->bootp_filename);
+    g_free(slirp->vdomainname);
     g_free(slirp);
 }
 
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 06febfc78b..10b410898a 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -193,6 +193,7 @@ struct Slirp {
     char *bootp_filename;
     size_t vdnssearch_len;
     uint8_t *vdnssearch;
+    char *vdomainname;
 
     /* tcp states */
     struct socket tcb;
-- 
2.14.1

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

* Re: [Qemu-devel] [PATCH 1/1] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 12:55 ` [Qemu-devel] [PATCH 1/1] " Benjamin Drung
@ 2018-02-16 15:15   ` Eric Blake
  2018-02-16 15:18     ` Benjamin Drung
                       ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Eric Blake @ 2018-02-16 15:15 UTC (permalink / raw)
  To: Benjamin Drung, Samuel Thibault, Jan Kiszka; +Cc: qemu-devel

On 02/16/2018 06:55 AM, Benjamin Drung wrote:
> This patch will allow the user to include the domainname option in
> replies from the built-in DHCP server.
> 
> Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
> ---

> +++ b/qapi/net.json
> @@ -160,6 +160,8 @@
>   # @dnssearch: list of DNS suffixes to search, passed as DHCP option
>   #             to the guest
>   #
> +# @domainname: guest-visible domain name of the virtual nameserver
> +#

Missing a '(since 2.12)' tag.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

* Re: [Qemu-devel] [PATCH 1/1] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 15:15   ` Eric Blake
@ 2018-02-16 15:18     ` Benjamin Drung
  2018-02-16 17:55     ` [Qemu-devel] [PATCH v2] " Benjamin Drung
  2018-02-26 15:52     ` [Qemu-devel] [PATCH v3] " Benjamin Drung
  2 siblings, 0 replies; 20+ messages in thread
From: Benjamin Drung @ 2018-02-16 15:18 UTC (permalink / raw)
  To: Eric Blake, Samuel Thibault, Jan Kiszka; +Cc: qemu-devel

Am Freitag, den 16.02.2018, 09:15 -0600 schrieb Eric Blake:
> On 02/16/2018 06:55 AM, Benjamin Drung wrote:
> > This patch will allow the user to include the domainname option in
> > replies from the built-in DHCP server.
> > 
> > Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
> > ---
> > +++ b/qapi/net.json
> > @@ -160,6 +160,8 @@
> >   # @dnssearch: list of DNS suffixes to search, passed as DHCP
> > option
> >   #             to the guest
> >   #
> > +# @domainname: guest-visible domain name of the virtual nameserver
> > +#
> 
> Missing a '(since 2.12)' tag.

Okay, I will add that. Is there a chance to get this patch backported?

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung@profitbricks.com
URL: https://www.profitbricks.de

Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss, Matthias Steinberg

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

* [Qemu-devel] [PATCH v2] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 15:15   ` Eric Blake
  2018-02-16 15:18     ` Benjamin Drung
@ 2018-02-16 17:55     ` Benjamin Drung
  2018-02-16 19:23       ` Philippe Mathieu-Daudé
  2018-02-26 15:52     ` [Qemu-devel] [PATCH v3] " Benjamin Drung
  2 siblings, 1 reply; 20+ messages in thread
From: Benjamin Drung @ 2018-02-16 17:55 UTC (permalink / raw)
  To: Samuel Thibault, Jan Kiszka, Eric Blake; +Cc: qemu-devel, Benjamin Drung

This patch will allow the user to include the domainname option in
replies from the built-in DHCP server.

Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
---
 net/slirp.c      | 7 ++++---
 qapi/net.json    | 4 ++++
 qemu-options.hx  | 7 +++++--
 slirp/bootp.c    | 8 ++++++++
 slirp/libslirp.h | 2 +-
 slirp/slirp.c    | 4 +++-
 slirp/slirp.h    | 1 +
 7 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 8991816bbf..9511ff3bb7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *bootfile, const char *vdhcp_start,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
-                          const char **dnssearch, Error **errp)
+                          const char **dnssearch, const char *vdomainname,
+                          Error **errp)
 {
     /* default settings according to historic slirp */
     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -371,7 +372,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     s->slirp = slirp_init(restricted, ipv4, net, mask, host,
                           ipv6, ip6_prefix, vprefix6_len, ip6_host,
                           vhostname, tftp_export, bootfile, dhcp,
-                          dns, ip6_dns, dnssearch, s);
+                          dns, ip6_dns, dnssearch, vdomainname, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
     for (config = slirp_configs; config; config = config->next) {
@@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, errp);
+                         user->smbserver, dnssearch, user->domainname, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index 1238ba5de1..9dfd34cafa 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -160,6 +160,9 @@
 # @dnssearch: list of DNS suffixes to search, passed as DHCP option
 #             to the guest
 #
+# @domainname: guest-visible domain name of the virtual nameserver
+#              (since 2.12)
+#
 # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
 #               2.6). The network prefix is given in the usual
 #               hexadecimal IPv6 address notation.
@@ -197,6 +200,7 @@
     '*dhcpstart': 'str',
     '*dns':       'str',
     '*dnssearch': ['String'],
+    '*domainname': 'str',
     '*ipv6-prefix':      'str',
     '*ipv6-prefixlen':   'int',
     '*ipv6-host':        'str',
diff --git a/qemu-options.hx b/qemu-options.hx
index 5050a49a5e..06983db7be 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
     "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
-    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n"
-    "         [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
+    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
 #ifndef _WIN32
                                              "[,smb=dir[,smbserver=addr]]\n"
 #endif
@@ -2116,6 +2116,9 @@ Example:
 qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
 @end example
 
+@item domainname=@var{domain}
+Specifies the client domain name reported by the built-in DHCP server.
+
 @item tftp=@var{dir}
 When using the user mode network stack, activate a built-in TFTP
 server. The files in @var{dir} will be exposed as the root of a TFTP server.
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5dd1a415b5..91896f7400 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
             q += val;
         }
 
+        if (*slirp->vdomainname) {
+            val = strlen(slirp->vdomainname);
+            *q++ = RFC1533_DOMAINNAME;
+            *q++ = val;
+            memcpy(q, slirp->vdomainname, val);
+            q += val;
+        }
+
         if (slirp->vdnssearch) {
             size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
             val = slirp->vdnssearch_len;
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 540b3e5903..740408a96e 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque);
+                  const char *vdomainname, void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
 void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1cb6b07004..4f29753444 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque)
+                  const char *vdomainname, void *opaque)
 {
     Slirp *slirp = g_malloc0(sizeof(Slirp));
 
@@ -317,6 +317,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
     }
     slirp->tftp_prefix = g_strdup(tftp_path);
     slirp->bootp_filename = g_strdup(bootfile);
+    slirp->vdomainname = g_strdup(vdomainname);
     slirp->vdhcp_startaddr = vdhcp_start;
     slirp->vnameserver_addr = vnameserver;
     slirp->vnameserver_addr6 = vnameserver6;
@@ -349,6 +350,7 @@ void slirp_cleanup(Slirp *slirp)
     g_free(slirp->vdnssearch);
     g_free(slirp->tftp_prefix);
     g_free(slirp->bootp_filename);
+    g_free(slirp->vdomainname);
     g_free(slirp);
 }
 
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 06febfc78b..10b410898a 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -193,6 +193,7 @@ struct Slirp {
     char *bootp_filename;
     size_t vdnssearch_len;
     uint8_t *vdnssearch;
+    char *vdomainname;
 
     /* tcp states */
     struct socket tcb;
-- 
2.14.1

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

* Re: [Qemu-devel] [PATCH v2] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 17:55     ` [Qemu-devel] [PATCH v2] " Benjamin Drung
@ 2018-02-16 19:23       ` Philippe Mathieu-Daudé
  2018-02-16 19:32         ` Benjamin Drung
                           ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-16 19:23 UTC (permalink / raw)
  To: Benjamin Drung, Samuel Thibault, Jan Kiszka, Eric Blake; +Cc: qemu-devel

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

Hi Benjamin,

On 02/16/2018 02:55 PM, Benjamin Drung wrote:
> This patch will allow the user to include the domainname option in
> replies from the built-in DHCP server.
> 
> Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
> ---
>  net/slirp.c      | 7 ++++---
>  qapi/net.json    | 4 ++++
>  qemu-options.hx  | 7 +++++--
>  slirp/bootp.c    | 8 ++++++++
>  slirp/libslirp.h | 2 +-
>  slirp/slirp.c    | 4 +++-
>  slirp/slirp.h    | 1 +
>  7 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 8991816bbf..9511ff3bb7 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>                            const char *bootfile, const char *vdhcp_start,
>                            const char *vnameserver, const char *vnameserver6,
>                            const char *smb_export, const char *vsmbserver,
> -                          const char **dnssearch, Error **errp)
> +                          const char **dnssearch, const char *vdomainname,
> +                          Error **errp)
>  {
>      /* default settings according to historic slirp */
>      struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
> @@ -371,7 +372,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>      s->slirp = slirp_init(restricted, ipv4, net, mask, host,
>                            ipv6, ip6_prefix, vprefix6_len, ip6_host,
>                            vhostname, tftp_export, bootfile, dhcp,
> -                          dns, ip6_dns, dnssearch, s);
> +                          dns, ip6_dns, dnssearch, vdomainname, s);
>      QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
>  
>      for (config = slirp_configs; config; config = config->next) {
> @@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>                           user->ipv6_host, user->hostname, user->tftp,
>                           user->bootfile, user->dhcpstart,
>                           user->dns, user->ipv6_dns, user->smb,
> -                         user->smbserver, dnssearch, errp);
> +                         user->smbserver, dnssearch, user->domainname, errp);
>  
>      while (slirp_configs) {
>          config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index 1238ba5de1..9dfd34cafa 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -160,6 +160,9 @@
>  # @dnssearch: list of DNS suffixes to search, passed as DHCP option
>  #             to the guest
>  #
> +# @domainname: guest-visible domain name of the virtual nameserver
> +#              (since 2.12)
> +#
>  # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
>  #               2.6). The network prefix is given in the usual
>  #               hexadecimal IPv6 address notation.
> @@ -197,6 +200,7 @@
>      '*dhcpstart': 'str',
>      '*dns':       'str',
>      '*dnssearch': ['String'],
> +    '*domainname': 'str',
>      '*ipv6-prefix':      'str',
>      '*ipv6-prefixlen':   'int',
>      '*ipv6-host':        'str',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5050a49a5e..06983db7be 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>      "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
>      "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
>      "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
> -    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n"
> -    "         [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
> +    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
> +    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
>  #ifndef _WIN32
>                                               "[,smb=dir[,smbserver=addr]]\n"
>  #endif
> @@ -2116,6 +2116,9 @@ Example:
>  qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
>  @end example
>  
> +@item domainname=@var{domain}
> +Specifies the client domain name reported by the built-in DHCP server.
> +
>  @item tftp=@var{dir}
>  When using the user mode network stack, activate a built-in TFTP
>  server. The files in @var{dir} will be exposed as the root of a TFTP server.
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 5dd1a415b5..91896f7400 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>              q += val;
>          }
>  
> +        if (*slirp->vdomainname) {

This is safe but harder to read/review...

> +            val = strlen(slirp->vdomainname);
> +            *q++ = RFC1533_DOMAINNAME;
> +            *q++ = val;
> +            memcpy(q, slirp->vdomainname, val);
> +            q += val;
> +        }
> +
>          if (slirp->vdnssearch) {
>              size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
>              val = slirp->vdnssearch_len;
> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> index 540b3e5903..740408a96e 100644
> --- a/slirp/libslirp.h
> +++ b/slirp/libslirp.h
> @@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>                    const char *tftp_path, const char *bootfile,
>                    struct in_addr vdhcp_start, struct in_addr vnameserver,
>                    struct in6_addr vnameserver6, const char **vdnssearch,
> -                  void *opaque);
> +                  const char *vdomainname, void *opaque);
>  void slirp_cleanup(Slirp *slirp);
>  
>  void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 1cb6b07004..4f29753444 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>                    const char *tftp_path, const char *bootfile,
>                    struct in_addr vdhcp_start, struct in_addr vnameserver,
>                    struct in6_addr vnameserver6, const char **vdnssearch,
> -                  void *opaque)
> +                  const char *vdomainname, void *opaque)
>  {
>      Slirp *slirp = g_malloc0(sizeof(Slirp));
>  
> @@ -317,6 +317,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>      }
>      slirp->tftp_prefix = g_strdup(tftp_path);
>      slirp->bootp_filename = g_strdup(bootfile);

I'd use this check:

   if (vdomainname) {
       if (!*vdomainname) {
           error_report("'domainname' parameter cannot be empty");
           exit(EXIT_FAILURE);
       }

> +    slirp->vdomainname = g_strdup(vdomainname);

   }

and simply check "if (slirp->vdomainname) { ..." in bootp_reply().

Regards,

Phil.

>      slirp->vdhcp_startaddr = vdhcp_start;
>      slirp->vnameserver_addr = vnameserver;
>      slirp->vnameserver_addr6 = vnameserver6;
> @@ -349,6 +350,7 @@ void slirp_cleanup(Slirp *slirp)
>      g_free(slirp->vdnssearch);
>      g_free(slirp->tftp_prefix);
>      g_free(slirp->bootp_filename);
> +    g_free(slirp->vdomainname);
>      g_free(slirp);
>  }
>  
> diff --git a/slirp/slirp.h b/slirp/slirp.h
> index 06febfc78b..10b410898a 100644
> --- a/slirp/slirp.h
> +++ b/slirp/slirp.h
> @@ -193,6 +193,7 @@ struct Slirp {
>      char *bootp_filename;
>      size_t vdnssearch_len;
>      uint8_t *vdnssearch;
> +    char *vdomainname;
>  
>      /* tcp states */
>      struct socket tcb;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [Qemu-devel] [PATCH v2] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 19:23       ` Philippe Mathieu-Daudé
@ 2018-02-16 19:32         ` Benjamin Drung
  2018-02-27 16:04         ` Benjamin Drung
  2018-02-27 16:06         ` [Qemu-devel] [PATCH 1/2 v4] " Benjamin Drung
  2 siblings, 0 replies; 20+ messages in thread
From: Benjamin Drung @ 2018-02-16 19:32 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka, Eric Blake
  Cc: qemu-devel

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

Hi Philippe,

Am Freitag, den 16.02.2018, 16:23 -0300 schrieb Philippe Mathieu-Daudé:
> Hi Benjamin,
> 
> On 02/16/2018 02:55 PM, Benjamin Drung wrote:
> > This patch will allow the user to include the domainname option in
> > replies from the built-in DHCP server.
> > 
> > Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
> > ---
> >  net/slirp.c      | 7 ++++---
> >  qapi/net.json    | 4 ++++
> >  qemu-options.hx  | 7 +++++--
> >  slirp/bootp.c    | 8 ++++++++
> >  slirp/libslirp.h | 2 +-
> >  slirp/slirp.c    | 4 +++-
> >  slirp/slirp.h    | 1 +
> >  7 files changed, 26 insertions(+), 7 deletions(-)
> > 
> > diff --git a/net/slirp.c b/net/slirp.c
> > index 8991816bbf..9511ff3bb7 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer,
> > const char *model,
> >                            const char *bootfile, const char
> > *vdhcp_start,
> >                            const char *vnameserver, const char
> > *vnameserver6,
> >                            const char *smb_export, const char
> > *vsmbserver,
> > -                          const char **dnssearch, Error **errp)
> > +                          const char **dnssearch, const char
> > *vdomainname,
> > +                          Error **errp)
> >  {
> >      /* default settings according to historic slirp */
> >      struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /*
> > 10.0.2.0 */
> > @@ -371,7 +372,7 @@ static int net_slirp_init(NetClientState *peer,
> > const char *model,
> >      s->slirp = slirp_init(restricted, ipv4, net, mask, host,
> >                            ipv6, ip6_prefix, vprefix6_len,
> > ip6_host,
> >                            vhostname, tftp_export, bootfile, dhcp,
> > -                          dns, ip6_dns, dnssearch, s);
> > +                          dns, ip6_dns, dnssearch, vdomainname,
> > s);
> >      QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
> >  
> >      for (config = slirp_configs; config; config = config->next) {
> > @@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, const
> > char *name,
> >                           user->ipv6_host, user->hostname, user-
> > >tftp,
> >                           user->bootfile, user->dhcpstart,
> >                           user->dns, user->ipv6_dns, user->smb,
> > -                         user->smbserver, dnssearch, errp);
> > +                         user->smbserver, dnssearch, user-
> > >domainname, errp);
> >  
> >      while (slirp_configs) {
> >          config = slirp_configs;
> > diff --git a/qapi/net.json b/qapi/net.json
> > index 1238ba5de1..9dfd34cafa 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -160,6 +160,9 @@
> >  # @dnssearch: list of DNS suffixes to search, passed as DHCP
> > option
> >  #             to the guest
> >  #
> > +# @domainname: guest-visible domain name of the virtual nameserver
> > +#              (since 2.12)
> > +#
> >  # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
> >  #               2.6). The network prefix is given in the usual
> >  #               hexadecimal IPv6 address notation.
> > @@ -197,6 +200,7 @@
> >      '*dhcpstart': 'str',
> >      '*dns':       'str',
> >      '*dnssearch': ['String'],
> > +    '*domainname': 'str',
> >      '*ipv6-prefix':      'str',
> >      '*ipv6-prefixlen':   'int',
> >      '*ipv6-host':        'str',
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 5050a49a5e..06983db7be 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
> >      "-netdev
> > user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
> >      "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-
> > host=addr]\n"
> >      "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\
> > n"
> > -    "         [,dns=addr][,ipv6-
> > dns=addr][,dnssearch=domain][,tftp=dir]\n"
> > -    "         [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
> > +    "         [,dns=addr][,ipv6-
> > dns=addr][,dnssearch=domain][,domainname=domain]\n"
> > +    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=ru
> > le]"
> >  #ifndef _WIN32
> >                                               "[,smb=dir[,smbserver
> > =addr]]\n"
> >  #endif
> > @@ -2116,6 +2116,9 @@ Example:
> >  qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org
> > [...]
> >  @end example
> >  
> > +@item domainname=@var{domain}
> > +Specifies the client domain name reported by the built-in DHCP
> > server.
> > +
> >  @item tftp=@var{dir}
> >  When using the user mode network stack, activate a built-in TFTP
> >  server. The files in @var{dir} will be exposed as the root of a
> > TFTP server.
> > diff --git a/slirp/bootp.c b/slirp/bootp.c
> > index 5dd1a415b5..91896f7400 100644
> > --- a/slirp/bootp.c
> > +++ b/slirp/bootp.c
> > @@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const
> > struct bootp_t *bp)
> >              q += val;
> >          }
> >  
> > +        if (*slirp->vdomainname) {
> 
> This is safe but harder to read/review...

I borrowed that logic from client_hostname.

> > +            val = strlen(slirp->vdomainname);
> > +            *q++ = RFC1533_DOMAINNAME;
> > +            *q++ = val;
> > +            memcpy(q, slirp->vdomainname, val);
> > +            q += val;
> > +        }
> > +
> >          if (slirp->vdnssearch) {
> >              size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp-
> > >bp_vend);
> >              val = slirp->vdnssearch_len;
> > diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> > index 540b3e5903..740408a96e 100644
> > --- a/slirp/libslirp.h
> > +++ b/slirp/libslirp.h
> > @@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool
> > in_enabled, struct in_addr vnetwork,
> >                    const char *tftp_path, const char *bootfile,
> >                    struct in_addr vdhcp_start, struct in_addr
> > vnameserver,
> >                    struct in6_addr vnameserver6, const char
> > **vdnssearch,
> > -                  void *opaque);
> > +                  const char *vdomainname, void *opaque);
> >  void slirp_cleanup(Slirp *slirp);
> >  
> >  void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
> > diff --git a/slirp/slirp.c b/slirp/slirp.c
> > index 1cb6b07004..4f29753444 100644
> > --- a/slirp/slirp.c
> > +++ b/slirp/slirp.c
> > @@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool
> > in_enabled, struct in_addr vnetwork,
> >                    const char *tftp_path, const char *bootfile,
> >                    struct in_addr vdhcp_start, struct in_addr
> > vnameserver,
> >                    struct in6_addr vnameserver6, const char
> > **vdnssearch,
> > -                  void *opaque)
> > +                  const char *vdomainname, void *opaque)
> >  {
> >      Slirp *slirp = g_malloc0(sizeof(Slirp));
> >  
> > @@ -317,6 +317,7 @@ Slirp *slirp_init(int restricted, bool
> > in_enabled, struct in_addr vnetwork,
> >      }
> >      slirp->tftp_prefix = g_strdup(tftp_path);
> >      slirp->bootp_filename = g_strdup(bootfile);
> 
> I'd use this check:
> 
>    if (vdomainname) {
>        if (!*vdomainname) {
>            error_report("'domainname' parameter cannot be empty");
>            exit(EXIT_FAILURE);
>        }

Do we want to bail out if domainname is set to empty instead of just
ignoring an empty domainname?

> > +    slirp->vdomainname = g_strdup(vdomainname);
> 
>    }
> 
> and simply check "if (slirp->vdomainname) { ..." in bootp_reply().
> 
> Regards,
> 
> Phil.
> 
> >      slirp->vdhcp_startaddr = vdhcp_start;
> >      slirp->vnameserver_addr = vnameserver;
> >      slirp->vnameserver_addr6 = vnameserver6;
> > @@ -349,6 +350,7 @@ void slirp_cleanup(Slirp *slirp)
> >      g_free(slirp->vdnssearch);
> >      g_free(slirp->tftp_prefix);
> >      g_free(slirp->bootp_filename);
> > +    g_free(slirp->vdomainname);
> >      g_free(slirp);
> >  }
> >  
> > diff --git a/slirp/slirp.h b/slirp/slirp.h
> > index 06febfc78b..10b410898a 100644
> > --- a/slirp/slirp.h
> > +++ b/slirp/slirp.h
> > @@ -193,6 +193,7 @@ struct Slirp {
> >      char *bootp_filename;
> >      size_t vdnssearch_len;
> >      uint8_t *vdnssearch;
> > +    char *vdomainname;
> >  
> >      /* tcp states */
> >      struct socket tcb;
> > 
> 
> 
-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung@profitbricks.com
URL: https://www.profitbricks.de

Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss, Matthias Steinberg

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [Qemu-devel] [PATCH 0/1] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 12:55 [Qemu-devel] [PATCH 0/1] slirp: Add domainname option to slirp's DHCP server Benjamin Drung
  2018-02-16 12:55 ` [Qemu-devel] [PATCH 1/1] " Benjamin Drung
@ 2018-02-17 21:16 ` Samuel Thibault
  2018-02-27 16:15   ` Benjamin Drung
  1 sibling, 1 reply; 20+ messages in thread
From: Samuel Thibault @ 2018-02-17 21:16 UTC (permalink / raw)
  To: Benjamin Drung; +Cc: Jan Kiszka, Eric Blake, Markus Armbruster, qemu-devel

Hello,

Benjamin Drung, on ven. 16 févr. 2018 13:55:03 +0100, wrote:
> Or should the command line option be simpler, but how should it be specified
> then? Maybe
> 
>   -net staticroute=10.0.2.0/24via10.0.2.2,staticroute=192.168.0.0/16via10.0.2.2

I guess 

>   -net staticroute=10.0.2.0/24:10.0.2.2,staticroute=192.168.0.0/16:10.0.2.2

would be more mainstream.

I'm also wondering to which extent we want to extend our dhcp server,
when a tap device can be used to plug (or proxy) an actual dhcp serveR.

samuel

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

* [Qemu-devel] [PATCH v3] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 15:15   ` Eric Blake
  2018-02-16 15:18     ` Benjamin Drung
  2018-02-16 17:55     ` [Qemu-devel] [PATCH v2] " Benjamin Drung
@ 2018-02-26 15:52     ` Benjamin Drung
  2 siblings, 0 replies; 20+ messages in thread
From: Benjamin Drung @ 2018-02-26 15:52 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka, Eric Blake
  Cc: qemu-devel, Benjamin Drung

This patch will allow the user to include the domainname option in
replies from the built-in DHCP server.

Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
---
 net/slirp.c      |  7 ++++---
 qapi/net.json    |  4 ++++
 qemu-options.hx  |  7 +++++--
 slirp/bootp.c    |  8 ++++++++
 slirp/libslirp.h |  2 +-
 slirp/slirp.c    | 10 +++++++++-
 slirp/slirp.h    |  1 +
 7 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 8991816bbf..9511ff3bb7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *bootfile, const char *vdhcp_start,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
-                          const char **dnssearch, Error **errp)
+                          const char **dnssearch, const char *vdomainname,
+                          Error **errp)
 {
     /* default settings according to historic slirp */
     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -371,7 +372,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     s->slirp = slirp_init(restricted, ipv4, net, mask, host,
                           ipv6, ip6_prefix, vprefix6_len, ip6_host,
                           vhostname, tftp_export, bootfile, dhcp,
-                          dns, ip6_dns, dnssearch, s);
+                          dns, ip6_dns, dnssearch, vdomainname, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
     for (config = slirp_configs; config; config = config->next) {
@@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, errp);
+                         user->smbserver, dnssearch, user->domainname, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index 1238ba5de1..9dfd34cafa 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -160,6 +160,9 @@
 # @dnssearch: list of DNS suffixes to search, passed as DHCP option
 #             to the guest
 #
+# @domainname: guest-visible domain name of the virtual nameserver
+#              (since 2.12)
+#
 # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
 #               2.6). The network prefix is given in the usual
 #               hexadecimal IPv6 address notation.
@@ -197,6 +200,7 @@
     '*dhcpstart': 'str',
     '*dns':       'str',
     '*dnssearch': ['String'],
+    '*domainname': 'str',
     '*ipv6-prefix':      'str',
     '*ipv6-prefixlen':   'int',
     '*ipv6-host':        'str',
diff --git a/qemu-options.hx b/qemu-options.hx
index 8ccd5dcaa6..c000ef454e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
     "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
-    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n"
-    "         [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
+    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
 #ifndef _WIN32
                                              "[,smb=dir[,smbserver=addr]]\n"
 #endif
@@ -2116,6 +2116,9 @@ Example:
 qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
 @end example
 
+@item domainname=@var{domain}
+Specifies the client domain name reported by the built-in DHCP server.
+
 @item tftp=@var{dir}
 When using the user mode network stack, activate a built-in TFTP
 server. The files in @var{dir} will be exposed as the root of a TFTP server.
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5dd1a415b5..9e7b53ba94 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
             q += val;
         }
 
+        if (slirp->vdomainname) {
+            val = strlen(slirp->vdomainname);
+            *q++ = RFC1533_DOMAINNAME;
+            *q++ = val;
+            memcpy(q, slirp->vdomainname, val);
+            q += val;
+        }
+
         if (slirp->vdnssearch) {
             size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
             val = slirp->vdnssearch_len;
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 540b3e5903..740408a96e 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque);
+                  const char *vdomainname, void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
 void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1cb6b07004..a6ed258fc2 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque)
+                  const char *vdomainname, void *opaque)
 {
     Slirp *slirp = g_malloc0(sizeof(Slirp));
 
@@ -317,6 +317,13 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
     }
     slirp->tftp_prefix = g_strdup(tftp_path);
     slirp->bootp_filename = g_strdup(bootfile);
+    if (vdomainname) {
+        if (!*vdomainname) {
+            error_report("'domainname' parameter cannot be empty");
+            exit(EXIT_FAILURE);
+        }
+        slirp->vdomainname = g_strdup(vdomainname);
+    }
     slirp->vdhcp_startaddr = vdhcp_start;
     slirp->vnameserver_addr = vnameserver;
     slirp->vnameserver_addr6 = vnameserver6;
@@ -349,6 +356,7 @@ void slirp_cleanup(Slirp *slirp)
     g_free(slirp->vdnssearch);
     g_free(slirp->tftp_prefix);
     g_free(slirp->bootp_filename);
+    g_free(slirp->vdomainname);
     g_free(slirp);
 }
 
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 06febfc78b..10b410898a 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -193,6 +193,7 @@ struct Slirp {
     char *bootp_filename;
     size_t vdnssearch_len;
     uint8_t *vdnssearch;
+    char *vdomainname;
 
     /* tcp states */
     struct socket tcb;
-- 
2.14.1

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

* Re: [Qemu-devel] [PATCH v2] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 19:23       ` Philippe Mathieu-Daudé
  2018-02-16 19:32         ` Benjamin Drung
@ 2018-02-27 16:04         ` Benjamin Drung
  2018-02-27 16:06         ` [Qemu-devel] [PATCH 1/2 v4] " Benjamin Drung
  2 siblings, 0 replies; 20+ messages in thread
From: Benjamin Drung @ 2018-02-27 16:04 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka, Eric Blake
  Cc: qemu-devel

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

Hi,

Am Freitag, den 16.02.2018, 16:23 -0300 schrieb Philippe Mathieu-Daudé:
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> > index 1cb6b07004..4f29753444 100644
> > --- a/slirp/slirp.c
> > +++ b/slirp/slirp.c
> > @@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool
> > in_enabled, struct in_addr vnetwork,
> >                    const char *tftp_path, const char *bootfile,
> >                    struct in_addr vdhcp_start, struct in_addr
> > vnameserver,
> >                    struct in6_addr vnameserver6, const char
> > **vdnssearch,
> > -                  void *opaque)
> > +                  const char *vdomainname, void *opaque)
> >  {
> >      Slirp *slirp = g_malloc0(sizeof(Slirp));
> >  
> > @@ -317,6 +317,7 @@ Slirp *slirp_init(int restricted, bool
> > in_enabled, struct in_addr vnetwork,
> >      }
> >      slirp->tftp_prefix = g_strdup(tftp_path);
> >      slirp->bootp_filename = g_strdup(bootfile);
> 
> I'd use this check:
> 
>    if (vdomainname) {
>        if (!*vdomainname) {
>            error_report("'domainname' parameter cannot be empty");
>            exit(EXIT_FAILURE);
>        }

I implemented that check in patch v3. In patch v4 I moved the check
logic into net_slirp_init() in net/slirp.c where all the other checks
live.

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung@profitbricks.com
URL: https://www.profitbricks.de

Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss, Matthias Steinberg

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* [Qemu-devel] [PATCH 1/2 v4] slirp: Add domainname option to slirp's DHCP server
  2018-02-16 19:23       ` Philippe Mathieu-Daudé
  2018-02-16 19:32         ` Benjamin Drung
  2018-02-27 16:04         ` Benjamin Drung
@ 2018-02-27 16:06         ` Benjamin Drung
  2018-02-27 16:06           ` [Qemu-devel] [PATCH 2/2] slirp: Add classless static routes support to " Benjamin Drung
  2018-03-04 20:43           ` [Qemu-devel] [PATCH 1/2 v4] slirp: Add domainname option to slirp's " Samuel Thibault
  2 siblings, 2 replies; 20+ messages in thread
From: Benjamin Drung @ 2018-02-27 16:06 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka, Eric Blake
  Cc: qemu-devel, Benjamin Drung

This patch will allow the user to include the domainname option in
replies from the built-in DHCP server.

Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
---
 net/slirp.c      | 12 +++++++++---
 qapi/net.json    |  4 ++++
 qemu-options.hx  |  7 +++++--
 slirp/bootp.c    |  8 ++++++++
 slirp/libslirp.h |  2 +-
 slirp/slirp.c    |  4 +++-
 slirp/slirp.h    |  1 +
 7 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 8991816bbf..8c08e5644f 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *bootfile, const char *vdhcp_start,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
-                          const char **dnssearch, Error **errp)
+                          const char **dnssearch, const char *vdomainname,
+                          Error **errp)
 {
     /* default settings according to historic slirp */
     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -359,6 +360,11 @@ static int net_slirp_init(NetClientState *peer, const char *model,
         ip6_dns.s6_addr[15] |= 3;
     }
 
+    if (vdomainname && !*vdomainname) {
+        error_setg(errp, "'domainname' parameter cannot be empty");
+        return -1;
+    }
+
 
     nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
 
@@ -371,7 +377,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     s->slirp = slirp_init(restricted, ipv4, net, mask, host,
                           ipv6, ip6_prefix, vprefix6_len, ip6_host,
                           vhostname, tftp_export, bootfile, dhcp,
-                          dns, ip6_dns, dnssearch, s);
+                          dns, ip6_dns, dnssearch, vdomainname, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
     for (config = slirp_configs; config; config = config->next) {
@@ -958,7 +964,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, errp);
+                         user->smbserver, dnssearch, user->domainname, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index 1238ba5de1..9dfd34cafa 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -160,6 +160,9 @@
 # @dnssearch: list of DNS suffixes to search, passed as DHCP option
 #             to the guest
 #
+# @domainname: guest-visible domain name of the virtual nameserver
+#              (since 2.12)
+#
 # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
 #               2.6). The network prefix is given in the usual
 #               hexadecimal IPv6 address notation.
@@ -197,6 +200,7 @@
     '*dhcpstart': 'str',
     '*dns':       'str',
     '*dnssearch': ['String'],
+    '*domainname': 'str',
     '*ipv6-prefix':      'str',
     '*ipv6-prefixlen':   'int',
     '*ipv6-host':        'str',
diff --git a/qemu-options.hx b/qemu-options.hx
index 8ccd5dcaa6..c000ef454e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
     "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
-    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n"
-    "         [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
+    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
 #ifndef _WIN32
                                              "[,smb=dir[,smbserver=addr]]\n"
 #endif
@@ -2116,6 +2116,9 @@ Example:
 qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
 @end example
 
+@item domainname=@var{domain}
+Specifies the client domain name reported by the built-in DHCP server.
+
 @item tftp=@var{dir}
 When using the user mode network stack, activate a built-in TFTP
 server. The files in @var{dir} will be exposed as the root of a TFTP server.
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 5dd1a415b5..9e7b53ba94 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
             q += val;
         }
 
+        if (slirp->vdomainname) {
+            val = strlen(slirp->vdomainname);
+            *q++ = RFC1533_DOMAINNAME;
+            *q++ = val;
+            memcpy(q, slirp->vdomainname, val);
+            q += val;
+        }
+
         if (slirp->vdnssearch) {
             size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
             val = slirp->vdnssearch_len;
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 540b3e5903..740408a96e 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque);
+                  const char *vdomainname, void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
 void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1cb6b07004..4f29753444 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  void *opaque)
+                  const char *vdomainname, void *opaque)
 {
     Slirp *slirp = g_malloc0(sizeof(Slirp));
 
@@ -317,6 +317,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
     }
     slirp->tftp_prefix = g_strdup(tftp_path);
     slirp->bootp_filename = g_strdup(bootfile);
+    slirp->vdomainname = g_strdup(vdomainname);
     slirp->vdhcp_startaddr = vdhcp_start;
     slirp->vnameserver_addr = vnameserver;
     slirp->vnameserver_addr6 = vnameserver6;
@@ -349,6 +350,7 @@ void slirp_cleanup(Slirp *slirp)
     g_free(slirp->vdnssearch);
     g_free(slirp->tftp_prefix);
     g_free(slirp->bootp_filename);
+    g_free(slirp->vdomainname);
     g_free(slirp);
 }
 
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 06febfc78b..10b410898a 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -193,6 +193,7 @@ struct Slirp {
     char *bootp_filename;
     size_t vdnssearch_len;
     uint8_t *vdnssearch;
+    char *vdomainname;
 
     /* tcp states */
     struct socket tcb;
-- 
2.14.1

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

* [Qemu-devel] [PATCH 2/2] slirp: Add classless static routes support to DHCP server
  2018-02-27 16:06         ` [Qemu-devel] [PATCH 1/2 v4] " Benjamin Drung
@ 2018-02-27 16:06           ` Benjamin Drung
  2018-03-04 21:06             ` Samuel Thibault
  2018-03-04 20:43           ` [Qemu-devel] [PATCH 1/2 v4] slirp: Add domainname option to slirp's " Samuel Thibault
  1 sibling, 1 reply; 20+ messages in thread
From: Benjamin Drung @ 2018-02-27 16:06 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka, Eric Blake
  Cc: qemu-devel, Benjamin Drung

This patch will allow the user to specify classless static routes for
the replies from the built-in DHCP server.

Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
---
 net/slirp.c      | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 qapi/net.json    |  4 ++++
 qemu-options.hx  | 13 +++++++++++-
 slirp/bootp.c    | 23 ++++++++++++++++++++
 slirp/bootp.h    |  2 ++
 slirp/libslirp.h |  9 +++++++-
 slirp/slirp.c    |  7 +++++-
 slirp/slirp.h    |  2 ++
 8 files changed, 119 insertions(+), 6 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 8c08e5644f..78df361485 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -158,7 +158,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
                           const char **dnssearch, const char *vdomainname,
-                          Error **errp)
+                          const StringList *vroutes, Error **errp)
 {
     /* default settings according to historic slirp */
     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -177,8 +177,12 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     char buf[20];
     uint32_t addr;
     int shift;
+    int i = 0;
     char *end;
+    unsigned int route_count = 0;
     struct slirp_config_str *config;
+    struct StaticRoute *routes = NULL;
+    const StringList *iter;
 
     if (!ipv4 && (vnetwork || vhost || vnameserver)) {
         error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
@@ -365,6 +369,58 @@ static int net_slirp_init(NetClientState *peer, const char *model,
         return -1;
     }
 
+    iter = vroutes;
+    while (iter) {
+        route_count++;
+        iter = iter->next;
+    }
+    routes = g_malloc(route_count * sizeof(StaticRoute));
+
+    iter = vroutes;
+    while(iter != NULL) {
+        char buf2[20];
+        const char *gateway = iter->value->str;
+        const char *mask;
+        char *end;
+        long mask_width;
+
+        // Split "subnet/mask:gateway" into its components
+        if (get_str_sep(buf2, sizeof(buf2), &gateway, ':') < 0) {
+            error_setg(errp, "Failed to parse route: No colon found in '%s'",
+                       iter->value->str);
+            return -1;
+        }
+        mask = buf2;
+        if (get_str_sep(buf, sizeof(buf), &mask, '/') < 0) {
+            error_setg(errp, "Failed to parse route: No slash found in '%s'",
+                       mask);
+            return -1;
+        }
+        if (!inet_aton(buf, &routes[i].subnet)) {
+            error_setg(errp, "Failed to parse route subnet '%s'", buf);
+            return -1;
+        }
+
+        mask_width = strtol(mask, &end, 10);
+        if (*end != '\0') {
+            error_setg(errp,
+                       "Failed to parse netmask '%s' (trailing chars)", mask);
+            return -1;
+        } else if (mask_width < 0 || mask_width > 32) {
+            error_setg(errp,
+                       "Invalid netmask provided (must be in range 0-32)");
+            return -1;
+        }
+        routes[i].mask_width = (uint8_t)mask_width;
+
+        if (!inet_aton(gateway, &routes[i].gateway)) {
+            error_setg(errp, "Failed to parse route gateway '%s'", gateway);
+            return -1;
+        }
+
+        iter = iter->next;
+        i++;
+    }
 
     nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
 
@@ -377,7 +433,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     s->slirp = slirp_init(restricted, ipv4, net, mask, host,
                           ipv6, ip6_prefix, vprefix6_len, ip6_host,
                           vhostname, tftp_export, bootfile, dhcp,
-                          dns, ip6_dns, dnssearch, vdomainname, s);
+                          dns, ip6_dns, dnssearch, vdomainname,
+                          route_count, routes, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
     for (config = slirp_configs; config; config = config->next) {
@@ -409,6 +466,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     return 0;
 
 error:
+    g_free(routes);
     qemu_del_net_client(nc);
     return -1;
 }
@@ -964,7 +1022,8 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, user->domainname, errp);
+                         user->smbserver, dnssearch, user->domainname,
+                         user->route, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index 9dfd34cafa..8a85debd92 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -163,6 +163,9 @@
 # @domainname: guest-visible domain name of the virtual nameserver
 #              (since 2.12)
 #
+# @route: guest-visible static classless route of the virtual nameserver
+#         (since 2.12)
+#
 # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
 #               2.6). The network prefix is given in the usual
 #               hexadecimal IPv6 address notation.
@@ -201,6 +204,7 @@
     '*dns':       'str',
     '*dnssearch': ['String'],
     '*domainname': 'str',
+    '*route':     ['String'],
     '*ipv6-prefix':      'str',
     '*ipv6-prefixlen':   'int',
     '*ipv6-host':        'str',
diff --git a/qemu-options.hx b/qemu-options.hx
index c000ef454e..e4bb5919ab 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1907,7 +1907,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
     "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
-    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,route=addr/mask:gateway][,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
 #ifndef _WIN32
                                              "[,smb=dir[,smbserver=addr]]\n"
 #endif
@@ -2119,6 +2119,17 @@ qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
 @item domainname=@var{domain}
 Specifies the client domain name reported by the built-in DHCP server.
 
+@item route=@var{addr}/@var{mask}:@var{gateway}
+Provides an entry for the classless static routes list sent by the built-in
+DHCP server. More than one route can be transmitted by specifying
+this option multiple times. If supported, this will cause the guest to
+automatically set the given static routes instead of the given default gateway.
+
+Example:
+@example
+qemu -net user,route=10.0.2.0/24:10.0.2.2,route=192.168.0.0/16:10.0.2.2 [...]
+@end example
+
 @item tftp=@var{dir}
 When using the user mode network stack, activate a built-in TFTP
 server. The files in @var{dir} will be exposed as the root of a TFTP server.
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 9e7b53ba94..a694a43189 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -306,6 +306,29 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
             q += val;
         }
 
+        if (slirp->route_count > 0) {
+            uint8_t option_length = 0;
+            uint8_t significant_octets;
+
+            for (int i = 0; i < slirp->route_count; i++) {
+                significant_octets = slirp->routes[i].mask_width / 8
+                                     + (slirp->routes[i].mask_width % 8 > 0);
+                option_length += significant_octets + 5;
+            }
+
+            *q++ = RFC3442_CLASSLESS_STATIC_ROUTE;
+            *q++ = option_length;
+            for (int i = 0; i < slirp->route_count; i++) {
+                significant_octets = slirp->routes[i].mask_width / 8
+                                     + (slirp->routes[i].mask_width % 8 > 0);
+                *q++ = slirp->routes[i].mask_width;
+                memcpy(q, &slirp->routes[i].subnet.s_addr, significant_octets);
+                q += significant_octets;
+                memcpy(q, &slirp->routes[i].gateway.s_addr, 4);
+                q += 4;
+            }
+        }
+
         if (slirp->vdnssearch) {
             size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
             val = slirp->vdnssearch_len;
diff --git a/slirp/bootp.h b/slirp/bootp.h
index 394525733e..60bef4e80d 100644
--- a/slirp/bootp.h
+++ b/slirp/bootp.h
@@ -71,6 +71,8 @@
 #define RFC2132_RENEWAL_TIME    58
 #define RFC2132_REBIND_TIME     59
 
+#define RFC3442_CLASSLESS_STATIC_ROUTE	121
+
 #define DHCPDISCOVER		1
 #define DHCPOFFER		2
 #define DHCPREQUEST		3
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 740408a96e..3d2e395b08 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -5,6 +5,12 @@
 
 typedef struct Slirp Slirp;
 
+typedef struct StaticRoute {
+    struct in_addr subnet;
+    uint8_t mask_width;
+    struct in_addr gateway;
+} StaticRoute;
+
 int get_dns_addr(struct in_addr *pdns_addr);
 int get_dns6_addr(struct in6_addr *pdns6_addr, uint32_t *scope_id);
 
@@ -16,7 +22,8 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  const char *vdomainname, void *opaque);
+                  const char *vdomainname, unsigned int routes_count,
+                  const StaticRoute *vroutes, void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
 void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 4f29753444..4e5f249eeb 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -286,7 +286,8 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  const char *vdomainname, void *opaque)
+                  const char *vdomainname, unsigned int route_count,
+                  const StaticRoute *vroutes, void *opaque)
 {
     Slirp *slirp = g_malloc0(sizeof(Slirp));
 
@@ -321,6 +322,9 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
     slirp->vdhcp_startaddr = vdhcp_start;
     slirp->vnameserver_addr = vnameserver;
     slirp->vnameserver_addr6 = vnameserver6;
+    slirp->route_count = route_count;
+    slirp->routes = g_malloc(route_count * sizeof(StaticRoute));
+    memcpy(slirp->routes, vroutes, route_count * sizeof(StaticRoute));
 
     if (vdnssearch) {
         translate_dnssearch(slirp, vdnssearch);
@@ -351,6 +355,7 @@ void slirp_cleanup(Slirp *slirp)
     g_free(slirp->tftp_prefix);
     g_free(slirp->bootp_filename);
     g_free(slirp->vdomainname);
+    g_free(slirp->routes);
     g_free(slirp);
 }
 
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 10b410898a..3c81b7f7cc 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -194,6 +194,8 @@ struct Slirp {
     size_t vdnssearch_len;
     uint8_t *vdnssearch;
     char *vdomainname;
+    unsigned int route_count;
+    struct StaticRoute *routes;
 
     /* tcp states */
     struct socket tcb;
-- 
2.14.1

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

* Re: [Qemu-devel] [PATCH 0/1] slirp: Add domainname option to slirp's DHCP server
  2018-02-17 21:16 ` [Qemu-devel] [PATCH 0/1] " Samuel Thibault
@ 2018-02-27 16:15   ` Benjamin Drung
  0 siblings, 0 replies; 20+ messages in thread
From: Benjamin Drung @ 2018-02-27 16:15 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: Jan Kiszka, Eric Blake, Markus Armbruster, qemu-devel

Am Samstag, den 17.02.2018, 22:16 +0100 schrieb Samuel Thibault:
> Hello,
> 
> Benjamin Drung, on ven. 16 févr. 2018 13:55:03 +0100, wrote:
> > Or should the command line option be simpler, but how should it be
> > specified
> > then? Maybe
> > 
> >   -net
> > staticroute=10.0.2.0/24via10.0.2.2,staticroute=192.168.0.0/16via10.
> > 0.2.2
> 
> I guess 
> 
> >   -net
> > staticroute=10.0.2.0/24:10.0.2.2,staticroute=192.168.0.0/16:10.0.2.
> > 2
> 
> would be more mainstream.

Okay. I used this syntax to implement the static route support. See the
"slirp: Add classless static routes support to DHCP server" patch.

> I'm also wondering to which extent we want to extend our dhcp server,
> when a tap device can be used to plug (or proxy) an actual dhcp
> serveR.

I am using slirp's DHCP server to test custom build live images
locally. This should be simple to setup and run in contrast to our
"real" production environment with real hardware and actual DHCP
severs.

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung@profitbricks.com
URL: https://www.profitbricks.de

Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss, Matthias Steinberg

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

* Re: [Qemu-devel] [PATCH 1/2 v4] slirp: Add domainname option to slirp's DHCP server
  2018-02-27 16:06         ` [Qemu-devel] [PATCH 1/2 v4] " Benjamin Drung
  2018-02-27 16:06           ` [Qemu-devel] [PATCH 2/2] slirp: Add classless static routes support to " Benjamin Drung
@ 2018-03-04 20:43           ` Samuel Thibault
  1 sibling, 0 replies; 20+ messages in thread
From: Samuel Thibault @ 2018-03-04 20:43 UTC (permalink / raw)
  To: Benjamin Drung
  Cc: Philippe Mathieu-Daudé, Jan Kiszka, Eric Blake, qemu-devel

Benjamin Drung, on mar. 27 févr. 2018 17:06:01 +0100, wrote:
> This patch will allow the user to include the domainname option in
> replies from the built-in DHCP server.
> 
> Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

and applied to my tree.

Samuel

> ---
>  net/slirp.c      | 12 +++++++++---
>  qapi/net.json    |  4 ++++
>  qemu-options.hx  |  7 +++++--
>  slirp/bootp.c    |  8 ++++++++
>  slirp/libslirp.h |  2 +-
>  slirp/slirp.c    |  4 +++-
>  slirp/slirp.h    |  1 +
>  7 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 8991816bbf..8c08e5644f 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>                            const char *bootfile, const char *vdhcp_start,
>                            const char *vnameserver, const char *vnameserver6,
>                            const char *smb_export, const char *vsmbserver,
> -                          const char **dnssearch, Error **errp)
> +                          const char **dnssearch, const char *vdomainname,
> +                          Error **errp)
>  {
>      /* default settings according to historic slirp */
>      struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
> @@ -359,6 +360,11 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>          ip6_dns.s6_addr[15] |= 3;
>      }
>  
> +    if (vdomainname && !*vdomainname) {
> +        error_setg(errp, "'domainname' parameter cannot be empty");
> +        return -1;
> +    }
> +
>  
>      nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
>  
> @@ -371,7 +377,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>      s->slirp = slirp_init(restricted, ipv4, net, mask, host,
>                            ipv6, ip6_prefix, vprefix6_len, ip6_host,
>                            vhostname, tftp_export, bootfile, dhcp,
> -                          dns, ip6_dns, dnssearch, s);
> +                          dns, ip6_dns, dnssearch, vdomainname, s);
>      QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
>  
>      for (config = slirp_configs; config; config = config->next) {
> @@ -958,7 +964,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>                           user->ipv6_host, user->hostname, user->tftp,
>                           user->bootfile, user->dhcpstart,
>                           user->dns, user->ipv6_dns, user->smb,
> -                         user->smbserver, dnssearch, errp);
> +                         user->smbserver, dnssearch, user->domainname, errp);
>  
>      while (slirp_configs) {
>          config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index 1238ba5de1..9dfd34cafa 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -160,6 +160,9 @@
>  # @dnssearch: list of DNS suffixes to search, passed as DHCP option
>  #             to the guest
>  #
> +# @domainname: guest-visible domain name of the virtual nameserver
> +#              (since 2.12)
> +#
>  # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
>  #               2.6). The network prefix is given in the usual
>  #               hexadecimal IPv6 address notation.
> @@ -197,6 +200,7 @@
>      '*dhcpstart': 'str',
>      '*dns':       'str',
>      '*dnssearch': ['String'],
> +    '*domainname': 'str',
>      '*ipv6-prefix':      'str',
>      '*ipv6-prefixlen':   'int',
>      '*ipv6-host':        'str',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8ccd5dcaa6..c000ef454e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>      "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
>      "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
>      "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
> -    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n"
> -    "         [,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
> +    "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
> +    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
>  #ifndef _WIN32
>                                               "[,smb=dir[,smbserver=addr]]\n"
>  #endif
> @@ -2116,6 +2116,9 @@ Example:
>  qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
>  @end example
>  
> +@item domainname=@var{domain}
> +Specifies the client domain name reported by the built-in DHCP server.
> +
>  @item tftp=@var{dir}
>  When using the user mode network stack, activate a built-in TFTP
>  server. The files in @var{dir} will be exposed as the root of a TFTP server.
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 5dd1a415b5..9e7b53ba94 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>              q += val;
>          }
>  
> +        if (slirp->vdomainname) {
> +            val = strlen(slirp->vdomainname);
> +            *q++ = RFC1533_DOMAINNAME;
> +            *q++ = val;
> +            memcpy(q, slirp->vdomainname, val);
> +            q += val;
> +        }
> +
>          if (slirp->vdnssearch) {
>              size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
>              val = slirp->vdnssearch_len;
> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> index 540b3e5903..740408a96e 100644
> --- a/slirp/libslirp.h
> +++ b/slirp/libslirp.h
> @@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>                    const char *tftp_path, const char *bootfile,
>                    struct in_addr vdhcp_start, struct in_addr vnameserver,
>                    struct in6_addr vnameserver6, const char **vdnssearch,
> -                  void *opaque);
> +                  const char *vdomainname, void *opaque);
>  void slirp_cleanup(Slirp *slirp);
>  
>  void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 1cb6b07004..4f29753444 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -286,7 +286,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>                    const char *tftp_path, const char *bootfile,
>                    struct in_addr vdhcp_start, struct in_addr vnameserver,
>                    struct in6_addr vnameserver6, const char **vdnssearch,
> -                  void *opaque)
> +                  const char *vdomainname, void *opaque)
>  {
>      Slirp *slirp = g_malloc0(sizeof(Slirp));
>  
> @@ -317,6 +317,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
>      }
>      slirp->tftp_prefix = g_strdup(tftp_path);
>      slirp->bootp_filename = g_strdup(bootfile);
> +    slirp->vdomainname = g_strdup(vdomainname);
>      slirp->vdhcp_startaddr = vdhcp_start;
>      slirp->vnameserver_addr = vnameserver;
>      slirp->vnameserver_addr6 = vnameserver6;
> @@ -349,6 +350,7 @@ void slirp_cleanup(Slirp *slirp)
>      g_free(slirp->vdnssearch);
>      g_free(slirp->tftp_prefix);
>      g_free(slirp->bootp_filename);
> +    g_free(slirp->vdomainname);
>      g_free(slirp);
>  }
>  
> diff --git a/slirp/slirp.h b/slirp/slirp.h
> index 06febfc78b..10b410898a 100644
> --- a/slirp/slirp.h
> +++ b/slirp/slirp.h
> @@ -193,6 +193,7 @@ struct Slirp {
>      char *bootp_filename;
>      size_t vdnssearch_len;
>      uint8_t *vdnssearch;
> +    char *vdomainname;
>  
>      /* tcp states */
>      struct socket tcb;
> -- 
> 2.14.1
> 

-- 
Samuel
<b> il faut combien de chevaux pour tirer une doloréan à 88 morph ?
***b vient de remarque que 88 mph c'est 142 km/h
<y> aaaaah
<y> c'est pour ça qu'ils limitent à 130 km/h sur les autoroutes
<y> c'est pour éviter que les gens voyagent dans le temps
<b> probablement

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

* Re: [Qemu-devel] [PATCH 2/2] slirp: Add classless static routes support to DHCP server
  2018-02-27 16:06           ` [Qemu-devel] [PATCH 2/2] slirp: Add classless static routes support to " Benjamin Drung
@ 2018-03-04 21:06             ` Samuel Thibault
  2018-03-08 18:57               ` [Qemu-devel] [PATCH 2/2 v2] " Benjamin Drung
  0 siblings, 1 reply; 20+ messages in thread
From: Samuel Thibault @ 2018-03-04 21:06 UTC (permalink / raw)
  To: Benjamin Drung
  Cc: Philippe Mathieu-Daudé, Jan Kiszka, Eric Blake, qemu-devel

Benjamin Drung, on mar. 27 févr. 2018 17:06:02 +0100, wrote:
> +    int i = 0;

Rather unsigned?
>      char *end;
> +    unsigned int route_count = 0;
>      struct slirp_config_str *config;
> +    struct StaticRoute *routes = NULL;
> +    const StringList *iter;
>  
>      if (!ipv4 && (vnetwork || vhost || vnameserver)) {
>          error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
> @@ -365,6 +369,58 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>          return -1;
>      }
>  
> +    iter = vroutes;

Rather initialize route_count to 0 here.
> +    while (iter) {
> +        route_count++;
> +        iter = iter->next;
> +    }



> +    iter = vroutes;

Rather initialize i to 0 here.

> +    while(iter != NULL) {

> +        // Split "subnet/mask:gateway" into its components
> +        if (get_str_sep(buf2, sizeof(buf2), &gateway, ':') < 0) {
[etc.]

You should make the gateway host.s_addr by default, so the gateway or
even :gateway part can be optional.

> -    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
> +    "         [,route=addr/mask:gateway][,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"

And document optionality in the help, of course.

> +        if (slirp->route_count > 0) {
> +            uint8_t option_length = 0;
> +            uint8_t significant_octets;
> +
> +            for (int i = 0; i < slirp->route_count; i++) {
> +                significant_octets = slirp->routes[i].mask_width / 8
> +                                     + (slirp->routes[i].mask_width % 8 > 0);

Rather use (slirp->routes[i].mask_width + 7) / 8 (and ditto below)

> +                option_length += significant_octets + 5;
> +            }
> +
> +            *q++ = RFC3442_CLASSLESS_STATIC_ROUTE;

I'd say instead of computing the option_length twice, create a variable
uint8_t *size which you make point at the size byte here, and you can
fill it after the loop.

Apart from that it looks good.

Samuel

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

* [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
  2018-03-04 21:06             ` Samuel Thibault
@ 2018-03-08 18:57               ` Benjamin Drung
  2018-03-08 19:46                 ` Eric Blake
  0 siblings, 1 reply; 20+ messages in thread
From: Benjamin Drung @ 2018-03-08 18:57 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka, Eric Blake
  Cc: qemu-devel, Benjamin Drung

This patch will allow the user to specify classless static routes for
the replies from the built-in DHCP server.

Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
---
 net/slirp.c      | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 qapi/net.json    |  4 ++++
 qemu-options.hx  | 14 +++++++++++-
 slirp/bootp.c    | 20 +++++++++++++++++
 slirp/bootp.h    |  2 ++
 slirp/libslirp.h |  9 +++++++-
 slirp/slirp.c    |  7 +++++-
 slirp/slirp.h    |  2 ++
 8 files changed, 120 insertions(+), 6 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 8c08e5644f..237ca1f3c9 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -158,7 +158,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
                           const char **dnssearch, const char *vdomainname,
-                          Error **errp)
+                          const StringList *vroutes, Error **errp)
 {
     /* default settings according to historic slirp */
     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -177,8 +177,12 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     char buf[20];
     uint32_t addr;
     int shift;
+    unsigned int i;
     char *end;
+    unsigned int route_count;
     struct slirp_config_str *config;
+    struct StaticRoute *routes = NULL;
+    const StringList *iter;
 
     if (!ipv4 && (vnetwork || vhost || vnameserver)) {
         error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
@@ -365,6 +369,61 @@ static int net_slirp_init(NetClientState *peer, const char *model,
         return -1;
     }
 
+    iter = vroutes;
+    route_count = 0;
+    while (iter) {
+        route_count++;
+        iter = iter->next;
+    }
+    routes = g_malloc(route_count * sizeof(StaticRoute));
+
+    i = 0;
+    iter = vroutes;
+    while(iter != NULL) {
+        char buf2[20];
+        const char *gateway = iter->value->str;
+        const char *mask;
+        char *end;
+        long mask_width;
+
+        // Split "subnet/mask[:gateway]" into its components
+        if (get_str_sep(buf2, sizeof(buf2), &gateway, ':') < 0) {
+            // Fallback to default gateway
+            routes[i].gateway.s_addr = host.s_addr;
+            mask = gateway;
+        } else {
+            if (!inet_aton(gateway, &routes[i].gateway)) {
+                error_setg(errp, "Failed to parse route gateway '%s'", gateway);
+                return -1;
+            }
+            mask = buf2;
+        }
+
+        if (get_str_sep(buf, sizeof(buf), &mask, '/') < 0) {
+            error_setg(errp, "Failed to parse route: No slash found in '%s'",
+                       mask);
+            return -1;
+        }
+        if (!inet_aton(buf, &routes[i].subnet)) {
+            error_setg(errp, "Failed to parse route subnet '%s'", buf);
+            return -1;
+        }
+
+        mask_width = strtol(mask, &end, 10);
+        if (*end != '\0') {
+            error_setg(errp,
+                       "Failed to parse netmask '%s' (trailing chars)", mask);
+            return -1;
+        } else if (mask_width < 0 || mask_width > 32) {
+            error_setg(errp,
+                       "Invalid netmask provided (must be in range 0-32)");
+            return -1;
+        }
+        routes[i].mask_width = (uint8_t)mask_width;
+
+        iter = iter->next;
+        i++;
+    }
 
     nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
 
@@ -377,7 +436,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     s->slirp = slirp_init(restricted, ipv4, net, mask, host,
                           ipv6, ip6_prefix, vprefix6_len, ip6_host,
                           vhostname, tftp_export, bootfile, dhcp,
-                          dns, ip6_dns, dnssearch, vdomainname, s);
+                          dns, ip6_dns, dnssearch, vdomainname,
+                          route_count, routes, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
     for (config = slirp_configs; config; config = config->next) {
@@ -409,6 +469,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     return 0;
 
 error:
+    g_free(routes);
     qemu_del_net_client(nc);
     return -1;
 }
@@ -964,7 +1025,8 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, user->domainname, errp);
+                         user->smbserver, dnssearch, user->domainname,
+                         user->route, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index 36589857f4..29118a2f01 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -163,6 +163,9 @@
 # @domainname: guest-visible domain name of the virtual nameserver
 #              (since 2.12)
 #
+# @route: guest-visible static classless route of the virtual nameserver
+#         (since 2.12)
+#
 # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
 #               2.6). The network prefix is given in the usual
 #               hexadecimal IPv6 address notation.
@@ -201,6 +204,7 @@
     '*dns':       'str',
     '*dnssearch': ['String'],
     '*domainname': 'str',
+    '*route':     ['String'],
     '*ipv6-prefix':      'str',
     '*ipv6-prefixlen':   'int',
     '*ipv6-host':        'str',
diff --git a/qemu-options.hx b/qemu-options.hx
index 8e88ba6b36..6025f64dfd 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1904,7 +1904,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
     "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
-    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,route=addr/mask[:gateway]][,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
 #ifndef _WIN32
                                              "[,smb=dir[,smbserver=addr]]\n"
 #endif
@@ -2137,6 +2137,18 @@ qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
 @item domainname=@var{domain}
 Specifies the client domain name reported by the built-in DHCP server.
 
+@item route=@var{addr}/@var{mask}[:@var{gateway}]
+Provides an entry for the classless static routes list sent by the built-in
+DHCP server. More than one route can be transmitted by specifying
+this option multiple times. If supported, this will cause the guest to
+automatically set the given static routes instead of the given default gateway.
+If @var{gateway} is not specified, the default gateway will be used.
+
+Example:
+@example
+qemu -net user,route=10.0.2.0/24,route=192.168.0.0/16 [...]
+@end example
+
 @item tftp=@var{dir}
 When using the user mode network stack, activate a built-in TFTP
 server. The files in @var{dir} will be exposed as the root of a TFTP server.
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 9e7b53ba94..5ca3b74e3b 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -160,6 +160,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
     int dhcp_msg_type, val;
     uint8_t *q;
     uint8_t client_ethaddr[ETH_ALEN];
+    uint8_t significant_octets[slirp->route_count];
 
     /* extract exact DHCP msg type */
     dhcp_decode(bp, &dhcp_msg_type, &preq_addr);
@@ -306,6 +307,25 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
             q += val;
         }
 
+        if (slirp->route_count > 0) {
+            uint8_t option_length = 0;
+
+            for (int i = 0; i < slirp->route_count; i++) {
+                significant_octets[i] = (slirp->routes[i].mask_width + 7) / 8;
+                option_length += significant_octets[i] + 5;
+            }
+
+            *q++ = RFC3442_CLASSLESS_STATIC_ROUTE;
+            *q++ = option_length;
+            for (int i = 0; i < slirp->route_count; i++) {
+                *q++ = slirp->routes[i].mask_width;
+                memcpy(q, &slirp->routes[i].subnet.s_addr, significant_octets[i]);
+                q += significant_octets[i];
+                memcpy(q, &slirp->routes[i].gateway.s_addr, 4);
+                q += 4;
+            }
+        }
+
         if (slirp->vdnssearch) {
             size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
             val = slirp->vdnssearch_len;
diff --git a/slirp/bootp.h b/slirp/bootp.h
index 394525733e..60bef4e80d 100644
--- a/slirp/bootp.h
+++ b/slirp/bootp.h
@@ -71,6 +71,8 @@
 #define RFC2132_RENEWAL_TIME    58
 #define RFC2132_REBIND_TIME     59
 
+#define RFC3442_CLASSLESS_STATIC_ROUTE	121
+
 #define DHCPDISCOVER		1
 #define DHCPOFFER		2
 #define DHCPREQUEST		3
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 740408a96e..3d2e395b08 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -5,6 +5,12 @@
 
 typedef struct Slirp Slirp;
 
+typedef struct StaticRoute {
+    struct in_addr subnet;
+    uint8_t mask_width;
+    struct in_addr gateway;
+} StaticRoute;
+
 int get_dns_addr(struct in_addr *pdns_addr);
 int get_dns6_addr(struct in6_addr *pdns6_addr, uint32_t *scope_id);
 
@@ -16,7 +22,8 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  const char *vdomainname, void *opaque);
+                  const char *vdomainname, unsigned int routes_count,
+                  const StaticRoute *vroutes, void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
 void slirp_pollfds_fill(GArray *pollfds, uint32_t *timeout);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 4f29753444..4e5f249eeb 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -286,7 +286,8 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
-                  const char *vdomainname, void *opaque)
+                  const char *vdomainname, unsigned int route_count,
+                  const StaticRoute *vroutes, void *opaque)
 {
     Slirp *slirp = g_malloc0(sizeof(Slirp));
 
@@ -321,6 +322,9 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
     slirp->vdhcp_startaddr = vdhcp_start;
     slirp->vnameserver_addr = vnameserver;
     slirp->vnameserver_addr6 = vnameserver6;
+    slirp->route_count = route_count;
+    slirp->routes = g_malloc(route_count * sizeof(StaticRoute));
+    memcpy(slirp->routes, vroutes, route_count * sizeof(StaticRoute));
 
     if (vdnssearch) {
         translate_dnssearch(slirp, vdnssearch);
@@ -351,6 +355,7 @@ void slirp_cleanup(Slirp *slirp)
     g_free(slirp->tftp_prefix);
     g_free(slirp->bootp_filename);
     g_free(slirp->vdomainname);
+    g_free(slirp->routes);
     g_free(slirp);
 }
 
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 10b410898a..3c81b7f7cc 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -194,6 +194,8 @@ struct Slirp {
     size_t vdnssearch_len;
     uint8_t *vdnssearch;
     char *vdomainname;
+    unsigned int route_count;
+    struct StaticRoute *routes;
 
     /* tcp states */
     struct socket tcb;
-- 
2.14.1

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

* Re: [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
  2018-03-08 18:57               ` [Qemu-devel] [PATCH 2/2 v2] " Benjamin Drung
@ 2018-03-08 19:46                 ` Eric Blake
  2018-03-08 20:07                   ` Benjamin Drung
  0 siblings, 1 reply; 20+ messages in thread
From: Eric Blake @ 2018-03-08 19:46 UTC (permalink / raw)
  To: Benjamin Drung, Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka
  Cc: qemu-devel

On 03/08/2018 12:57 PM, Benjamin Drung wrote:
> This patch will allow the user to specify classless static routes for
> the replies from the built-in DHCP server.
> 
> Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
> ---

For future patches, when sending a v2, it's best to document here (after 
the --- separator) what changed from v1.  It's also a good idea to send 
a fresh thread rather than tying your v2 in-reply-to your v1, so that it 
doesn't get buried in an old conversation.

More submission hints at https://wiki.qemu.org/Contribute/SubmitAPatch

>   net/slirp.c      | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>   qapi/net.json    |  4 ++++
>   qemu-options.hx  | 14 +++++++++++-
>   slirp/bootp.c    | 20 +++++++++++++++++
>   slirp/bootp.h    |  2 ++
>   slirp/libslirp.h |  9 +++++++-
>   slirp/slirp.c    |  7 +++++-
>   slirp/slirp.h    |  2 ++
>   8 files changed, 120 insertions(+), 6 deletions(-)

Reviewing just the QMP interface:


> +++ b/qapi/net.json
> @@ -163,6 +163,9 @@
>   # @domainname: guest-visible domain name of the virtual nameserver
>   #              (since 2.12)
>   #
> +# @route: guest-visible static classless route of the virtual nameserver
> +#         (since 2.12)
> +#
>   # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
>   #               2.6). The network prefix is given in the usual
>   #               hexadecimal IPv6 address notation.
> @@ -201,6 +204,7 @@
>       '*dns':       'str',
>       '*dnssearch': ['String'],
>       '*domainname': 'str',
> +    '*route':     ['String'],

I know we've used ['String'] for previous members, but that's rather 
heavyweight - it transmits over QMP as:

"dnssearch": [ { "str": "foo" }, { "str": "bar" } ]

Nicer is ['str'], which transmits as:

"route": [ "foo", "bar" ]

so the question boils down to whether cross-member consistency is more 
important than making your additions concise.

> +++ b/qemu-options.hx
> @@ -1904,7 +1904,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>       "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
>       "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
>       "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
> -    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
> +    "         [,route=addr/mask[:gateway]][,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"

Urgh - your QMP interface HAS to be further parsed to get to the useful 
information.  While it's nice to have compact syntax on the command 
line, it is really worth thinking about making information easier to 
consume (that is, NO further parsing required once the information is in 
JSON format).  Would it be any better to send things over the wire as:

"route": [ { "addr": "...", "mask": 24, "gateway": "..." } ]

instead of cramming all the information into a single string?  But based 
on the way this also maps to the command line, you may not have a choice 
without a lot more code complexity.

>   #ifndef _WIN32
>                                                "[,smb=dir[,smbserver=addr]]\n"
>   #endif
> @@ -2137,6 +2137,18 @@ qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
>   @item domainname=@var{domain}
>   Specifies the client domain name reported by the built-in DHCP server.
>   
> +@item route=@var{addr}/@var{mask}[:@var{gateway}]
> +Provides an entry for the classless static routes list sent by the built-in
> +DHCP server. More than one route can be transmitted by specifying
> +this option multiple times. If supported, this will cause the guest to
> +automatically set the given static routes instead of the given default gateway.
> +If @var{gateway} is not specified, the default gateway will be used.
> +
> +Example:
> +@example
> +qemu -net user,route=10.0.2.0/24,route=192.168.0.0/16 [...]
> +@end example

Can we please spell that '--net', along the lines of 
https://wiki.qemu.org/BiteSizedTasks#Consistent_option_usage_in_documentation

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

* Re: [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
  2018-03-08 19:46                 ` Eric Blake
@ 2018-03-08 20:07                   ` Benjamin Drung
  2018-03-08 20:21                     ` Eric Blake
  0 siblings, 1 reply; 20+ messages in thread
From: Benjamin Drung @ 2018-03-08 20:07 UTC (permalink / raw)
  To: Eric Blake, Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka
  Cc: qemu-devel

Am Donnerstag, den 08.03.2018, 13:46 -0600 schrieb Eric Blake:
> On 03/08/2018 12:57 PM, Benjamin Drung wrote:
> > This patch will allow the user to specify classless static routes
> > for
> > the replies from the built-in DHCP server.
> > 
> > Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
> > ---
> 
> For future patches, when sending a v2, it's best to document here
> (after 
> the --- separator) what changed from v1.  It's also a good idea to
> send 
> a fresh thread rather than tying your v2 in-reply-to your v1, so that
> it 
> doesn't get buried in an old conversation.
> 
> More submission hints at https://wiki.qemu.org/Contribute/SubmitAPatch

Thanks. I will do that with the next iteration. Patch v2 addressed all
remarks from Samuel Thibault.

> > +++ b/qapi/net.json
> > @@ -163,6 +163,9 @@
> >   # @domainname: guest-visible domain name of the virtual
> > nameserver
> >   #              (since 2.12)
> >   #
> > +# @route: guest-visible static classless route of the virtual
> > nameserver
> > +#         (since 2.12)
> > +#
> >   # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since
> >   #               2.6). The network prefix is given in the usual
> >   #               hexadecimal IPv6 address notation.
> > @@ -201,6 +204,7 @@
> >       '*dns':       'str',
> >       '*dnssearch': ['String'],
> >       '*domainname': 'str',
> > +    '*route':     ['String'],
> 
> I know we've used ['String'] for previous members, but that's rather 
> heavyweight - it transmits over QMP as:
> 
> "dnssearch": [ { "str": "foo" }, { "str": "bar" } ]
> 
> Nicer is ['str'], which transmits as:
> 
> "route": [ "foo", "bar" ]
> 
> so the question boils down to whether cross-member consistency is
> more 
> important than making your additions concise.

Agreed that ['str'] is nicer. I will update the patch.

> > +++ b/qemu-options.hx
> > @@ -1904,7 +1904,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
> >       "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-
> > host=addr]\n"
> >       "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]
> > \n"
> >       "         [,dns=addr][,ipv6-
> > dns=addr][,dnssearch=domain][,domainname=domain]\n"
> > -    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=ru
> > le]"
> > +    "         [,route=addr/mask[:gateway]][,tftp=dir][,bootfile=f]
> > [,hostfwd=rule][,guestfwd=rule]"
> 
> Urgh - your QMP interface HAS to be further parsed to get to the
> useful 
> information.  While it's nice to have compact syntax on the command 
> line, it is really worth thinking about making information easier to 
> consume (that is, NO further parsing required once the information is
> in 
> JSON format).  Would it be any better to send things over the wire
> as:
> 
> "route": [ { "addr": "...", "mask": 24, "gateway": "..." } ]

That's looks good.

> instead of cramming all the information into a single string?  But
> based 
> on the way this also maps to the command line, you may not have a
> choice 
> without a lot more code complexity.

Can you point me to an example where similar parsing is done?

> >   #ifndef _WIN32
> >                                                "[,smb=dir[,smbserve
> > r=addr]]\n"
> >   #endif
> > @@ -2137,6 +2137,18 @@ qemu -net
> > user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
> >   @item domainname=@var{domain}
> >   Specifies the client domain name reported by the built-in DHCP
> > server.
> >   
> > +@item route=@var{addr}/@var{mask}[:@var{gateway}]
> > +Provides an entry for the classless static routes list sent by the
> > built-in
> > +DHCP server. More than one route can be transmitted by specifying
> > +this option multiple times. If supported, this will cause the
> > guest to
> > +automatically set the given static routes instead of the given
> > default gateway.
> > +If @var{gateway} is not specified, the default gateway will be
> > used.
> > +
> > +Example:
> > +@example
> > +qemu -net user,route=10.0.2.0/24,route=192.168.0.0/16 [...]
> > +@end example
> 
> Can we please spell that '--net', along the lines of 
> https://wiki.qemu.org/BiteSizedTasks#Consistent_option_usage_in_docum
> entation

I can change it, but then the documentation is inconsistent. There
are 75 lines with '-net' in qemu-options.hx, but only two lines
with '--net'.

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung@profitbricks.com
URL: https://www.profitbricks.de

Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss, Matthias Steinberg

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

* Re: [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
  2018-03-08 20:07                   ` Benjamin Drung
@ 2018-03-08 20:21                     ` Eric Blake
  2018-03-14 19:07                       ` Benjamin Drung
  0 siblings, 1 reply; 20+ messages in thread
From: Eric Blake @ 2018-03-08 20:21 UTC (permalink / raw)
  To: Benjamin Drung, Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka
  Cc: qemu-devel

On 03/08/2018 02:07 PM, Benjamin Drung wrote:
> Am Donnerstag, den 08.03.2018, 13:46 -0600 schrieb Eric Blake:
>> On 03/08/2018 12:57 PM, Benjamin Drung wrote:
>>> This patch will allow the user to specify classless static routes
>>> for
>>> the replies from the built-in DHCP server.
>>>
>>> Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>
>>> ---
>>
>> For future patches, when sending a v2, it's best to document here
>> (after
>> the --- separator) what changed from v1.  It's also a good idea to
>> send
>> a fresh thread rather than tying your v2 in-reply-to your v1, so that
>> it
>> doesn't get buried in an old conversation.
>>
>> More submission hints at https://wiki.qemu.org/Contribute/SubmitAPatch
> 
> Thanks. I will do that with the next iteration. Patch v2 addressed all
> remarks from Samuel Thibault.

At this point, since Samuel is the net maintainer, I'll trust his 
judgment on what interface works best; my review is only trying to make 
sure we don't bake in a UI mistake at the last minute (although we can 
adjust things during soft freeze, if needed).


>>>        '*dnssearch': ['String'],
>>>        '*domainname': 'str',
>>> +    '*route':     ['String'],
>>
>> I know we've used ['String'] for previous members, but that's rather
>> heavyweight - it transmits over QMP as:
>>
>> "dnssearch": [ { "str": "foo" }, { "str": "bar" } ]
>>
>> Nicer is ['str'], which transmits as:
>>
>> "route": [ "foo", "bar" ]
>>
>> so the question boils down to whether cross-member consistency is
>> more
>> important than making your additions concise.
> 
> Agreed that ['str'] is nicer. I will update the patch.

The problem is that ['str'] might not work easily for the command line 
glue; I'm more familiar with how QMP exposes things than with the 
command line parsing, and Markus, who is trying to improve command line 
parsing to share more common infrastructure with QMP, might have better 
comments on the topic, except that he's on leave for a few weeks and 
won't respond until after 2.12 is frozen.  Using ['String'] for 
consistency is therefore okay, if you can't get ['str'] working quickly.


>>> @@ -1904,7 +1904,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>>>        "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-
>>> host=addr]\n"

Here's an example where we made the command line smart.  ipv6-net takes 
TWO pieces of information: addr/int; on the QMP side, we spelled it 
'*ipv6-prefix':'str' + 'ipv6-prefixlen':'int'.  So somewhere in the 
command line parsing code for --net (which I'm less familiar with), 
there is some glue code taking the compact representation and splitting 
it into the more verbose but more direct QMP representation - well, that 
is, if we are converting it into QMP form at all (part of the problem is 
that our command line and runtime control don't always share code, 
although we're trying to get better at that).

>>> +    "         [,route=addr/mask[:gateway]][,tftp=dir][,bootfile=f]
>>> [,hostfwd=rule][,guestfwd=rule]"
>>
>> Urgh - your QMP interface HAS to be further parsed to get to the
>> useful
>> information.  While it's nice to have compact syntax on the command
>> line, it is really worth thinking about making information easier to
>> consume (that is, NO further parsing required once the information is
>> in
>> JSON format).  Would it be any better to send things over the wire
>> as:
>>
>> "route": [ { "addr": "...", "mask": 24, "gateway": "..." } ]
> 
> That's looks good.

Okay, doing that would mean using something like:

{ 'struct': 'RouteEntry', 'data': { 'addr': 'str', '*mask': 'int', 
'*gateway': 'str' } }
...
'route': [ 'RouteEntry' ]

(but reuse, rather than inventing a new type, if one of the existing QMP 
types already resembles what I proposed for RouteEntry)

The command line can still use route=addr/mask:gateway syntax, parse it 
down into components, then compile the QMP array of already-parsed 
structs (rather than making QMP take a direct ['String'] that still 
needs further parsing).  It may take more glue code, but the idea is 
that all the glue code should live on the front end, so that the QMP 
backend should be easy to work with.

> 
>> instead of cramming all the information into a single string?  But
>> based
>> on the way this also maps to the command line, you may not have a
>> choice
>> without a lot more code complexity.
> 
> Can you point me to an example where similar parsing is done?

Hopefully my hint about command-line ipv6-net gets you started (as I 
said, I'm less familiar with the specifics of net code, so much as 
taking the interface point of view here).

>>> +@example
>>> +qemu -net user,route=10.0.2.0/24,route=192.168.0.0/16 [...]
>>> +@end example
>>
>> Can we please spell that '--net', along the lines of
>> https://wiki.qemu.org/BiteSizedTasks#Consistent_option_usage_in_docum
>> entation
> 
> I can change it, but then the documentation is inconsistent. There
> are 75 lines with '-net' in qemu-options.hx, but only two lines
> with '--net'.

Yeah, there's that.  But hopefully someone will tackle the bite-sized 
task to get things consistent, and once they do, leaving fewer places 
that still need to be switched is nice.  I can live with either spelling 
until the bite-sized task is tackled, and am not implying you have to 
take on that task yourself, so much as pointing it out so you can choose 
if it is worth the nicer spelling for new content, or leaving it as-is 
to feel more consistent in the short term.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

* Re: [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
  2018-03-08 20:21                     ` Eric Blake
@ 2018-03-14 19:07                       ` Benjamin Drung
  0 siblings, 0 replies; 20+ messages in thread
From: Benjamin Drung @ 2018-03-14 19:07 UTC (permalink / raw)
  To: Eric Blake, Philippe Mathieu-Daudé, Samuel Thibault, Jan Kiszka
  Cc: qemu-devel

Am Donnerstag, den 08.03.2018, 14:21 -0600 schrieb Eric Blake:
> On 03/08/2018 02:07 PM, Benjamin Drung wrote:
> > Am Donnerstag, den 08.03.2018, 13:46 -0600 schrieb Eric Blake:
> > > On 03/08/2018 12:57 PM, Benjamin Drung wrote:
> > > >        '*dnssearch': ['String'],
> > > >        '*domainname': 'str',
> > > > +    '*route':     ['String'],
> > > 
> > > I know we've used ['String'] for previous members, but that's
> > > rather
> > > heavyweight - it transmits over QMP as:
> > > 
> > > "dnssearch": [ { "str": "foo" }, { "str": "bar" } ]
> > > 
> > > Nicer is ['str'], which transmits as:
> > > 
> > > "route": [ "foo", "bar" ]
> > > 
> > > so the question boils down to whether cross-member consistency is
> > > more
> > > important than making your additions concise.
> > 
> > Agreed that ['str'] is nicer. I will update the patch.
> 
> The problem is that ['str'] might not work easily for the command
> line 
> glue; I'm more familiar with how QMP exposes things than with the 
> command line parsing, and Markus, who is trying to improve command
> line 
> parsing to share more common infrastructure with QMP, might have
> better 
> comments on the topic, except that he's on leave for a few weeks and 
> won't respond until after 2.12 is frozen.  Using ['String'] for 
> consistency is therefore okay, if you can't get ['str'] working
> quickly.

['str'] works and was easily changeable.

> > > > @@ -1904,7 +1904,7 @@ DEF("netdev", HAS_ARG,
> > > > QEMU_OPTION_netdev,
> > > >        "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-
> > > > host=addr]\n"
> 
> Here's an example where we made the command line smart.  ipv6-net
> takes 
> TWO pieces of information: addr/int; on the QMP side, we spelled it 
> '*ipv6-prefix':'str' + 'ipv6-prefixlen':'int'.  So somewhere in the 
> command line parsing code for --net (which I'm less familiar with), 
> there is some glue code taking the compact representation and
> splitting 
> it into the more verbose but more direct QMP representation - well,
> that 
> is, if we are converting it into QMP form at all (part of the problem
> is 
> that our command line and runtime control don't always share code, 
> although we're trying to get better at that).

The split is done net_client_init() before iterating over the options.
This is equivalent to having following command line parameter:

  [,ipv6-prefix=addr][,ipv6-prefixlen=int]

instead of

  [,ipv6-net=addr[/int]]

> > > > +    "         [,route=addr/mask[:gateway]][,tftp=dir][,bootfil
> > > > e=f]
> > > > [,hostfwd=rule][,guestfwd=rule]"
> > > 
> > > Urgh - your QMP interface HAS to be further parsed to get to the
> > > useful
> > > information.  While it's nice to have compact syntax on the
> > > command
> > > line, it is really worth thinking about making information easier
> > > to
> > > consume (that is, NO further parsing required once the
> > > information is
> > > in
> > > JSON format).  Would it be any better to send things over the
> > > wire
> > > as:
> > > 
> > > "route": [ { "addr": "...", "mask": 24, "gateway": "..." } ]
> > 
> > That's looks good.
> 
> Okay, doing that would mean using something like:
> 
> { 'struct': 'RouteEntry', 'data': { 'addr': 'str', '*mask': 'int', 
> '*gateway': 'str' } }
> ...
> 'route': [ 'RouteEntry' ]
> 
> (but reuse, rather than inventing a new type, if one of the existing
> QMP 
> types already resembles what I proposed for RouteEntry)
> 
> The command line can still use route=addr/mask:gateway syntax, parse
> it 
> down into components, then compile the QMP array of already-parsed 
> structs (rather than making QMP take a direct ['String'] that still 
> needs further parsing).  It may take more glue code, but the idea is 
> that all the glue code should live on the front end, so that the QMP 
> backend should be easy to work with.

The problem is that the opts visitor code only supports lists with a
single mandatory scalar member. Bigger changes are needed to support
splitting 'route'. I have looked into the code for several hours, but
found no simple and non-ugly way to add the splitting without major
changes to the code. I am willing to adapt the patch if someone changes
the opts visitor to support lists with multiple members.

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung@profitbricks.com
URL: https://www.profitbricks.de

Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss, Matthias Steinberg

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

end of thread, other threads:[~2018-03-14 19:08 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-16 12:55 [Qemu-devel] [PATCH 0/1] slirp: Add domainname option to slirp's DHCP server Benjamin Drung
2018-02-16 12:55 ` [Qemu-devel] [PATCH 1/1] " Benjamin Drung
2018-02-16 15:15   ` Eric Blake
2018-02-16 15:18     ` Benjamin Drung
2018-02-16 17:55     ` [Qemu-devel] [PATCH v2] " Benjamin Drung
2018-02-16 19:23       ` Philippe Mathieu-Daudé
2018-02-16 19:32         ` Benjamin Drung
2018-02-27 16:04         ` Benjamin Drung
2018-02-27 16:06         ` [Qemu-devel] [PATCH 1/2 v4] " Benjamin Drung
2018-02-27 16:06           ` [Qemu-devel] [PATCH 2/2] slirp: Add classless static routes support to " Benjamin Drung
2018-03-04 21:06             ` Samuel Thibault
2018-03-08 18:57               ` [Qemu-devel] [PATCH 2/2 v2] " Benjamin Drung
2018-03-08 19:46                 ` Eric Blake
2018-03-08 20:07                   ` Benjamin Drung
2018-03-08 20:21                     ` Eric Blake
2018-03-14 19:07                       ` Benjamin Drung
2018-03-04 20:43           ` [Qemu-devel] [PATCH 1/2 v4] slirp: Add domainname option to slirp's " Samuel Thibault
2018-02-26 15:52     ` [Qemu-devel] [PATCH v3] " Benjamin Drung
2018-02-17 21:16 ` [Qemu-devel] [PATCH 0/1] " Samuel Thibault
2018-02-27 16:15   ` Benjamin Drung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.