On 01/12/2011 02:24 PM, PaweÅ KrzeÅniak wrote:
> 2011/1/7 Daniel P. Berrange <berrange@xxxxxxxxxx>:
>> 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@xxxxxxxxx>
> 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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list