slirp: Add domainname option to slirp's DHCP server
cover slirp: Add domainname option to slirp's DHCP server
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(-)
Signature · 1 line
2.14.1
PATCH 1/1 slirp: Add domainname option to slirp's DHCP server
This patch will allow the user to include the domainname option in replies from the built-in 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(-)
net/slirp.c +4 -3
@@ -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;
qapi/net.json +3 -0
@@ -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',
qemu-options.hx +5 -2
@@ -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.
slirp/bootp.c +8 -0
@@ -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;
slirp/libslirp.h +1 -1
@@ -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);
slirp/slirp.c +3 -1
@@ -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); }
slirp/slirp.h +1 -0
@@ -193,6 +193,7 @@ struct Slirp {char *bootp_filename;size_t vdnssearch_len;uint8_t *vdnssearch; +char *vdomainname;/* tcp states */ struct socket tcb;
Signature · 1 line
2.14.1
qapi/net.json line 164
Quoted 13 lines · 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 <[email protected]>---
+++
b/qapi/net.jsonqapi/net.json +2
-0@@ -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.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Am Freitag, den 16.02.2018, 09:15 -0600 schrieb Eric Blake:
Quoted 16 lines · Benjamin Drung
qapi/net.json line 164
Quoted 13 lines
This patch will allow the user to include the domainname option in replies from the built-in DHCP server.
Signed-off-by Benjamin Drung <[email protected]>---
+++b/qapi/net.json(quoted diff) +0
-0@@ -160,6 +160,8 @@ # @dnssearch: list of DNS suffixes to search, passed as DHCPoption
# to the guest
#qapi/net.json +2
-0+# @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?
Signature · 14 lines
Benjamin Drung System Developer Debian & Ubuntu Developer ProfitBricks GmbH Greifswalder Str. 207 D - 10405 Berlin Email: [email protected] URL: https://www.profitbricks.de Sitz der Gesellschaft: Berlin Registergericht: Amtsgericht Charlottenburg, HRB 125506 B Geschäftsführer: Achim Weiss, Matthias Steinberg
PATCH v2 1/1 slirp: Add domainname option to slirp's DHCP server
In reply to Eric BlakeThis patch will allow the user to include the domainname option in replies from the built-in DHCP server.
--- 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(-)
net/slirp.c +4 -3
@@ -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;
qapi/net.json +4 -0
@@ -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',
qemu-options.hx +5 -2
@@ -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.
slirp/bootp.c +8 -0
@@ -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;
slirp/libslirp.h +1 -1
@@ -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);
slirp/slirp.c +3 -1
@@ -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); }
slirp/slirp.h +1 -0
@@ -193,6 +193,7 @@ struct Slirp {char *bootp_filename;size_t vdnssearch_len;uint8_t *vdnssearch; +char *vdomainname;/* tcp states */ struct socket tcb;
Signature · 1 line
2.14.1
Hi Benjamin,
Quoted 98 lines · 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 <[email protected]>--- 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(-)net/slirp.c +4
-3@@ -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;qapi/net.json +4
-0@@ -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',qemu-options.hx +5
-2@@ -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.slirp/bootp.c +0
-0
slirp/bootp.c static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) · line 301
Quoted 4 lines
slirp/bootp.c +1
-0@@ -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...
Quoted 37 lines
slirp/bootp.c +7
-0+ 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;slirp/libslirp.h +1
-1@@ -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);slirp/slirp.c +1
-1@@ -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));
slirp/slirp.c Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork, · line 319
Quoted 3 lines
slirp/slirp.c +0
-0@@ -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/slirp.c Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork, · line 320
Quoted 1 line
slirp/slirp.c +1
-0+ slirp->vdomainname =g_strdup (vdomainname);
}
and simply check "if (slirp->vdomainname) { ..." in bootp_reply().
Regards,
Phil.
Quoted 24 lines
slirp/slirp.c +1
-0slirp->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); }slirp/slirp.h +1
-0@@ -193,6 +193,7 @@ struct Slirp {char *bootp_filename;size_t vdnssearch_len;uint8_t *vdnssearch; +char *vdomainname;/* tcp states */ struct socket tcb;
Hi Philippe,
Am Freitag, den 16.02.2018, 16:23 -0300 schrieb Philippe Mathieu-Daudé:
Quoted 128 lines
Hi Benjamin,
Quoted 125 lines · 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 <[email protected]>--- 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(-)net/slirp.c +1
-1@@ -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 */
net/slirp.c +1
-1@@ -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) {net/slirp.c +1
-1@@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, constchar *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;qapi/net.json +0
-0@@ -160,6 +160,9 @@ # @dnssearch: list of DNS suffixes to search, passed as DHCPoption
# to the guest
#qapi/net.json +4
-0+# @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',qemu-options.hx +0
-0@@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, "-netdevuser,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"qemu-options.hx +1
-1- " [,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"
#endifqemu-options.hx +2
-0@@ -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 DHCPserver.
+
@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.slirp/bootp.c +0
-0@@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) q += val; }
Quoted 3 lines
slirp/bootp.c static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) · line 301
Quoted 1 line
slirp/bootp.c +1
-0+if (*slirp->vdomainname) {This is safe but harder to read/review...
I borrowed that logic from client_hostname.
Quoted 51 lines
Quoted 44 lines
slirp/bootp.c +7
-0+ 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-Quoted 1 line
bp_vend);val = slirp->vdnssearch_len;slirp/libslirp.h +1
-1@@ -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);slirp/slirp.c +1
-1@@ -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));slirp/slirp.c Slirp *slirp_init(int restricted, bool · line 320
Quoted 4 lines
slirp/slirp.c +0
-0@@ -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:
Quoted 5 lines
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?
Quoted 36 lines
slirp/slirp.c Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork, · line 320
Quoted 1 line
slirp/slirp.c +1
-0+ slirp->vdomainname =g_strdup (vdomainname);}and simply check "if (
slirp->vdomainname) { ..." inbootp_reply().Regards,
Phil.
Quoted 24 lines
slirp/slirp.c +0
-0slirp->vdhcp_startaddr = vdhcp_start; slirp->vnameserver_addr = vnameserver;slirp->vnameserver_addr6 = vnameserver6;slirp/slirp.c +1
-0@@ -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); }slirp/slirp.h +1
-0@@ -193,6 +193,7 @@ struct Slirp {char *bootp_filename;size_t vdnssearch_len;uint8_t *vdnssearch; +char *vdomainname;/* tcp states */ struct socket tcb;
Signature · 14 lines
Benjamin Drung System Developer Debian & Ubuntu Developer ProfitBricks GmbH Greifswalder Str. 207 D - 10405 Berlin Email: [email protected] URL: https://www.profitbricks.de Sitz der Gesellschaft: Berlin Registergericht: Amtsgericht Charlottenburg, HRB 125506 B Geschäftsführer: Achim Weiss, Matthias Steinberg
Hi,
Am Freitag, den 16.02.2018, 16:23 -0300 schrieb Philippe Mathieu-Daudé:
Quoted 24 lines
slirp/slirp.c +0
-0Quoted 16 lines
index
1cb6b07004..4f29753444100644slirp/slirp.c +1
-1@@ -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));slirp/slirp.c Slirp *slirp_init(int restricted, bool · line 320
Quoted 4 lines
slirp/slirp.c +0
-0@@ -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:
Quoted 5 lines
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.
Signature · 14 lines
Benjamin Drung System Developer Debian & Ubuntu Developer ProfitBricks GmbH Greifswalder Str. 207 D - 10405 Berlin Email: [email protected] URL: https://www.profitbricks.de Sitz der Gesellschaft: Berlin Registergericht: Amtsgericht Charlottenburg, HRB 125506 B Geschäftsführer: Achim Weiss, Matthias Steinberg
PATCH v4 1/2 slirp: Add domainname option to slirp's DHCP server
In reply to Philippe Mathieu-DaudéThis patch will allow the user to include the domainname option in replies from the built-in DHCP server.
--- 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(-)
net/slirp.c +9 -3
@@ -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;
qapi/net.json +4 -0
@@ -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',
qemu-options.hx +5 -2
@@ -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.
slirp/bootp.c +8 -0
@@ -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;
slirp/libslirp.h +1 -1
@@ -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);
slirp/slirp.c +3 -1
@@ -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); }
slirp/slirp.h +1 -0
@@ -193,6 +193,7 @@ struct Slirp {char *bootp_filename;size_t vdnssearch_len;uint8_t *vdnssearch; +char *vdomainname;/* tcp states */ struct socket tcb;
Signature · 1 line
2.14.1
PATCH 2/2 slirp: Add classless static routes support to DHCP server
This patch will allow the user to specify classless static routes for the replies from the built-in DHCP server.
--- 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(-)
net/slirp.c +62 -3
@@ -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;
qapi/net.json +4 -0
@@ -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',
qemu-options.hx +12 -1
@@ -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.
slirp/bootp.c +23 -0
@@ -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;
slirp/bootp.h +2 -0
@@ -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
slirp/libslirp.h +8 -1
@@ -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);
slirp/slirp.c +6 -1
@@ -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); }
slirp/slirp.h +2 -0
@@ -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;
Signature · 1 line
2.14.1
Re: [Qemu-devel] [PATCH 2/2] slirp: Add classless static routes support to DHCP server
Quoted 1 line · Benjamin Drung, on mar. 27 févr. 2018 17:06:02 +0100, wrote:
+ int i = 0;
Rather unsigned?
net/slirp.c static int net_slirp_init(NetClientState *peer, const char *model, · line 372
Quoted 13 lines
char *end;net/slirp.c +4
-0+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.
net/slirp.c static int net_slirp_init(NetClientState *peer, const char *model,
Quoted 8 lines
net/slirp.c +5
-0+while (iter) { + route_count++; + iter = iter->next; + } + iter = vroutes;
Rather initialize i to 0 here.
net/slirp.c static int net_slirp_init(NetClientState *peer, const char *model, · line 388
Quoted 4 lines
net/slirp.c +3
-0+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.
qemu-options.hx DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, · line 1910
Quoted 2 lines
qemu-options.hx +1
-1- " [,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.
slirp/bootp.c static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) · line 315
Quoted 7 lines
slirp/bootp.c +7
-0+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)
slirp/bootp.c static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) · line 319
Quoted 4 lines
slirp/bootp.c +4
-0+ 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
PATCH v2 2/2 slirp: Add classless static routes support to DHCP server
This patch will allow the user to specify classless static routes for the replies from the built-in DHCP server.
--- 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(-)
net/slirp.c +65 -3
@@ -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;
qapi/net.json +4 -0
@@ -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',
qemu-options.hx +13 -1
@@ -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.
slirp/bootp.c +20 -0
@@ -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;
slirp/bootp.h +2 -0
@@ -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
slirp/libslirp.h +8 -1
@@ -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);
slirp/slirp.c +6 -1
@@ -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); }
slirp/slirp.h +2 -0
@@ -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;
Signature · 1 line
2.14.1
Re: [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
Quoted 5 lines · 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 <[email protected]>---
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
Quoted 9 lines
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:
Quoted 12 lines
+++
b/qapi/net.jsonqapi/net.json +3
-0@@ -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.
qapi/net.json line 207
Quoted 4 lines
qapi/net.json +1
-0@@ -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.
qemu-options.hx DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, · line 1907
Quoted 7 lines
qemu-options.hx +2
-1+++ 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.
Quoted 8 lines
qemu-options.hx +1
-0#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}]
qemu-options.hx qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...] · line 2150
Quoted 10 lines
qemu-options.hx +10
-0+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
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
Am Donnerstag, den 08.03.2018, 13:46 -0600 schrieb Eric Blake:
Quoted 16 lines · Benjamin Drung
Quoted 6 lines
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 <[email protected]>---
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.
Quoted 1 line
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.
Quoted 24 lines
Quoted 14 lines
+++
b/qapi/net.json(quoted diff) +0
-0@@ -163,6 +163,9 @@ # @domainname: guest-visible domain name of the virtualnameserver
# (since 2.12)
#
+# @route: guest-visible static classless route of the virtual nameserverqapi/net.json +2
-0+# (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.qapi/net.json line 207
Quoted 4 lines
qapi/net.json +1
-0@@ -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" } ]
Quoted 7 lines
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.
Quoted 22 lines
Quoted 6 lines
(quoted diff) +1
-0+++ 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"Quoted 6 lines
" [,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:
Quoted 1 line
"route": [ { "addr": "...", "mask": 24, "gateway": "..." } ]
That's looks good.
Quoted 5 lines
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?
Quoted 21 lines
Quoted 21 lines
#ifndef _WIN32 "[,smb=dir[,smbserver=addr]]\n"
#endif(quoted diff) +0
-0@@ -2137,6 +2137,18 @@ qemu -netuser,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
(quoted diff) +2
-0+DHCP server. More than one route can be transmitted by specifying +this option multiple times. If supported, this will cause theguest 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.
+
Quoted 8 lines
qemu-options.hx qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...] · line 2150
Quoted 4 lines
qemu-options.hx +4
-0+Example: +@example +qemu -net user,route=10.0.2.0/24,route=192.168.0.0/16 [...] +@end exampleCan 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'.
Signature · 14 lines
Benjamin Drung System Developer Debian & Ubuntu Developer ProfitBricks GmbH Greifswalder Str. 207 D - 10405 Berlin Email: [email protected] URL: https://www.profitbricks.de Sitz der Gesellschaft: Berlin Registergericht: Amtsgericht Charlottenburg, HRB 125506 B Geschäftsführer: Achim Weiss, Matthias Steinberg
Re: [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
Quoted 17 lines · Benjamin Drung
Am Donnerstag, den 08.03.2018, 13:46 -0600 schrieb Eric Blake:
Quoted 16 lines · Benjamin Drung
Quoted 6 lines
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 <[email protected]>---
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.
Quoted 4 lines
Quoted 1 line
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).
Quoted 11 lines
Quoted 11 lines
Quoted 3 lines
'*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:
Quoted 7 lines
Quoted 5 lines
"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.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
(quoted diff) +0
-0@@ -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).
Quoted 12 lines
Quoted 12 lines
Quoted 2 lines
+ " [,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:
Quoted 3 lines
Quoted 1 line
"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.
Quoted 8 lines
Quoted 6 lines
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).
Quoted 11 lines
Quoted 7 lines
Quoted 3 lines
+@example
+qemu -net user,route=10.0.2.0/24,route=192.168.0.0/16 [...] +@end exampleCan we please spell that '--net', along the lines of
https://wiki.qemu.org/BiteSizedTasks#Consistent_option_usage_in_docum
entationI 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.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 2/2 v2] slirp: Add classless static routes support to DHCP server
Am Donnerstag, den 08.03.2018, 14:21 -0600 schrieb Eric Blake:
Quoted 23 lines · Benjamin Drung
Quoted 14 lines
Am Donnerstag, den 08.03.2018, 13:46 -0600 schrieb Eric Blake:
Quoted 13 lines · Benjamin Drung
Quoted 3 lines
'*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:
Quoted 7 lines
Quoted 5 lines
"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.
Quoted 11 lines
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.
Quoted 10 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
(quoted diff) +0
-0@@ -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),
Quoted 8 lines
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]]
Quoted 32 lines
Quoted 17 lines
Quoted 17 lines
Quoted 3 lines
+ " [,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:Quoted 3 lines
Quoted 1 line
"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)
Quoted 7 lines
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.
Signature · 14 lines
Benjamin Drung System Developer Debian & Ubuntu Developer ProfitBricks GmbH Greifswalder Str. 207 D - 10405 Berlin Email: [email protected] URL: https://www.profitbricks.de Sitz der Gesellschaft: Berlin Registergericht: Amtsgericht Charlottenburg, HRB 125506 B Geschäftsführer: Achim Weiss, Matthias Steinberg
Quoted 4 lines · 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 <[email protected]>
and applied to my tree.
Samuel
Quoted 177 lines
--- 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(-)net/slirp.c +9
-3@@ -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;qapi/net.json +4
-0@@ -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',qemu-options.hx +5
-2@@ -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.slirp/bootp.c +8
-0@@ -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;slirp/libslirp.h +1
-1@@ -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);slirp/slirp.c +3
-1@@ -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); }slirp/slirp.h +1
-0@@ -193,6 +193,7 @@ struct Slirp {char *bootp_filename;size_t vdnssearch_len;uint8_t *vdnssearch; +char *vdomainname;/* tcp states */ struct socket tcb;Signature · 1 line
2.14.1
Signature · 7 lines
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
PATCH v3 1/1 slirp: Add domainname option to slirp's DHCP server
In reply to Eric BlakeThis patch will allow the user to include the domainname option in replies from the built-in DHCP server.
--- 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(-)
net/slirp.c +4 -3
@@ -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;
qapi/net.json +4 -0
@@ -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',
qemu-options.hx +5 -2
@@ -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.
slirp/bootp.c +8 -0
@@ -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;
slirp/libslirp.h +1 -1
@@ -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);
slirp/slirp.c +9 -1
@@ -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); }
slirp/slirp.h +1 -0
@@ -193,6 +193,7 @@ struct Slirp {char *bootp_filename;size_t vdnssearch_len;uint8_t *vdnssearch; +char *vdomainname;/* tcp states */ struct socket tcb;
Signature · 1 line
2.14.1
Hello,
Quoted 4 lines · 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
Quoted 1 line
-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
Am Samstag, den 17.02.2018, 22:16 +0100 schrieb Samuel Thibault:
Quoted 11 lines
Hello,
Quoted 7 lines · 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
Quoted 7 lines
I guess
Quoted 3 lines
-net
staticroute=10.0.2.0/24:10.0.2.2,staticroute=192.168.0.0/16:10.0.2. 2would 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.
Quoted 3 lines
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.
Signature · 14 lines
Benjamin Drung System Developer Debian & Ubuntu Developer ProfitBricks GmbH Greifswalder Str. 207 D - 10405 Berlin Email: [email protected] URL: https://www.profitbricks.de Sitz der Gesellschaft: Berlin Registergericht: Amtsgericht Charlottenburg, HRB 125506 B Geschäftsführer: Achim Weiss, Matthias Steinberg