Re: [libvirt] [PATCHv3] handle DNS over IPv6
On 01/12/2011 02:24 PM, Paweł Krześniak wrote:
2011/1/7 Daniel P. Berrange <berrange(a)redhat.com>:
> Practically no apps
> will be do this and so they typically can't make use of
> the link-local address.
I agree.
v2 of patch attached.
Can you convince your mailer to send patches with mime type text/plain,
rather than base64 encoded as application/octet-stream? Inline review
is a lot nicer. That said:
commit ce44728e232a39ff42bf87f5eb102bc76b71241d
Author: Pawel Krzesniak <pawel.krzesniak(a)gmail.com>
Date: Tue Jan 11 23:36:19 2011 +0100
bridge_driver: handle DNS over IPv6
* dnsmasq listens on all defined IPv[46] addresses for network
* Add ip6tables rules to allow DNS traffic to host
---
src/network/bridge_driver.c | 59 ++++++++++++++++++++++++++++++++++--------
1 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 4c64a74..d5524db 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -423,7 +423,7 @@ networkSaveDnsmasqHostsfile(virNetworkIpDefPtr ipdef,
return 0;
}
-
+#define FAMILIES_COUNT 2
Rather than defining this,
static int
networkBuildDnsmasqArgv(virNetworkObjPtr network,
virNetworkIpDefPtr ipdef,
@@ -431,7 +431,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
virCommandPtr cmd) {
int r, ret = -1;
int nbleases = 0;
- char *bridgeaddr;
+ char *bridgeaddr = NULL;
+ char *ipaddr = NULL;
+ int ii, jj;
+ virNetworkIpDefPtr tmpipdef;
+ int families[FAMILIES_COUNT] = { AF_INET, AF_INET6 };
just declare this as int families[] = { ... };
+ /*
+ * --interface does not actually work with dnsmasq < 2.47,
+ * due to DAD for ipv6 addresses on the interface.
+ *
+ * virCommandAddArgList(cmd, "--interface", ipdef->bridge, NULL);
+ *
+ * So listen on all defined IPv[46] addresses
+ */
+ for (ii = 0; ii < FAMILIES_COUNT; ii++) {
and iterate while ii < ARRAY_CARDINALITY(families).
One less thing to screw up if we ever add a third family.
@@ -1027,9 +1040,31 @@ networkAddGeneralIp6tablesRules(struct
network_driver *driver,
goto err3;
}
+ /* allow DNS over IPv6 */
+ if (iptablesAddTcpInput(driver->iptables, AF_INET6,
+ network->def->bridge, 53) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add ip6tables rule to allow DNS requests
from '%s'"),
+ network->def->bridge);
+ goto err4;
+ }
+
+ if (iptablesAddUdpInput(driver->iptables, AF_INET6,
+ network->def->bridge, 53) < 0) {
+ networkReportError(VIR_ERR_SYSTEM_ERROR,
+ _("failed to add ip6tables rule to allow DNS requests
from '%s'"),
+ network->def->bridge);
+ goto err5;
+ }
Is it worth a (separate) clean-up patch that provides
iptablesAddTcpUdpInput, which adds a rules for both protocol types in
one shot, rather than requiring every call site to duplicate the calls
for tcp and udp?
The patch looks reasonable to me from the coding standpoint, but is
probably worth a v4 for my nits, and you may want feedback from someone
more familiar with the networking standpoint.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)