(Sorry for the delay in responding. We're in a bit of a crunch ;-)
On 01/12/2011 04: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.
As Eric indicated, it would be more convenient if your mailer attached
the patch as an ASCII file rather than base64-encoded (I'm guessing
possibly it did that automatically because of the accented characters in
your name - perhaps there's a config parameter that would cause it to
use some other encoding when it encounters things outside basic 7bit ASCII?)
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
static int
networkBuildDnsmasqArgv(virNetworkObjPtr network,
virNetworkIpDefPtr ipdef,
I'd been thinking that, if we were going to add this capability, we may
as well eliminate the virNetworkIpDefPtr arg from the function call -
we're already iterating over all the IPs anyway, so we may as well look
for the IP that has the ranges and static hosts as we go through the list.
(As a matter of fact, doing this would make it easier to (in a future
patch, not here and now! :-) add in the ranges and static hosts from
*all* IPs on an interface, not just the first found to have them. I'm
not a dnsmasq expert, but it looks like this would require using the
"network-id" detailed in the dnsmasq man page, along with multiple
--dhcp-option arguments (and probably some other things).
@@ -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 };
I actually think it's a bit of overkill to do this. At this point, we
only have AF_INET and AF_INET6 addresses, and we want to do the same
thing with all of them, so we don't even need to look at family.
if (!(bridgeaddr = virSocketFormatAddr(&ipdef->address)))
goto cleanup;
@@ -468,20 +472,29 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
/* *no* conf file */
virCommandAddArgList(cmd, "--conf-file=", "", NULL);
- /*
- * XXX does not actually work, due to some kind of
- * race condition setting up ipv6 addresses on the
- * interface. A sleep(10) makes it work, but that's
- * clearly not practical
- *
- * virCommandAddArg(cmd, "--interface");
- * virCommandAddArg(cmd, ipdef->bridge);
- */
virCommandAddArgList(cmd,
- "--listen-address", bridgeaddr,
"--except-interface", "lo",
NULL);
+ /*
+ * --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
+ */
As mentioned above - just make the following a single loop with family
"AF_UNSPEC" so it gets all the ip addresses. By definition, every item
on the list will have either an AF_INET or an AF_INET6 address.
+ for (ii = 0; ii< FAMILIES_COUNT; ii++) {
+ for (jj = 0;
+ (tmpipdef = virNetworkDefGetIpByIndex(network->def, families[ii], jj));
+ jj++) {
+ if (!(ipaddr = virSocketFormatAddr(&tmpipdef->address)))
+ goto cleanup;
+ virCommandAddArgList(cmd, "--listen-address", ipaddr, NULL);
+ VIR_FREE(ipaddr);
+ }
+ }
+
As part of the "future patch" mentioned above to support ranges/static
hosts for all IP addresses, this next bit (about nranges) could be moved
inside the above loop. For now, if you remove the virDomainNetIpDefPtr
arg from the call to this function, you would want to check for the
presences of hosts or ranges at each iteration through the loop.
for (r = 0 ; r< ipdef->nranges ; r++) {
char *saddr = virSocketFormatAddr(&ipdef->ranges[r].start);
if (!saddr)
@@ -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;
+ }
+
+
return 0;
/* unwind in reverse order from the point of failure */
+err5:
+ iptablesRemoveTcpInput(driver->iptables, AF_INET6, network->def->bridge,
53);
+err4:
+ iptablesRemoveForwardAllowCross(driver->iptables, AF_INET6,
network->def->bridge);
err3:
iptablesRemoveForwardRejectIn(driver->iptables, AF_INET6,
network->def->bridge);
err2:
I'm fine with this 2nd part of the patch. (I don't think there's a need
for a single function to add both tcp and udp rules (as Eric suggested);
I'd rather each call correspond to a single rule.)