[libvirt] [PATCHv3 1/3] v2.0: allow guest to guest IPv6 without gateway definition
Gene Czarcinski
gene at czarc.net
Mon Dec 3 22:15:11 UTC 2012
On 12/03/2012 04:03 PM, Laine Stump wrote:
> On 12/03/2012 11:13 AM, Gene Czarcinski wrote:
>> This patch adds the capability for virtual guests to do IPv6
>> communication via a virtual network interface with no IPv6
>> (gateway) addresses specified. This capability currently
>> exists for IPv4.
>>
>> This patch allows creation of a completely isolated IPv6 network.
>>
>> Note that virtual guests cannot communication with the virtualization
>> host via this interface. Also note that:
>> net.ipv6.conf.<interface_name>.disable_ipv6 = 1
>>
>> Also not that starting libvirtd has set the following:
>> net.bridge.bridge-nf-call-arptables = 1
>> net.bridge.bridge-nf-call-ip6tables = 1
>> net.bridge.bridge-nf-call-iptables = 1
>> although /etc/syslog.conf has them all set to 0.
>>
>> To control this behavior so that it is not enabled by default, the parameter
>> ipv6='yes' on the <network> statement has been added.
>>
>> Documentation related to this patch has been updated.
>> The network schema has also been updated.
>> ---
>> docs/formatnetwork.html.in | 28 +++++++++++++++++++++++++++-
>> docs/schemas/network.rng | 10 ++++++++++
>> src/conf/network_conf.c | 8 ++++++++
>> src/conf/network_conf.h | 5 +++++
>> src/network/bridge_driver.c | 25 ++++++++++++++++++++-----
>> 5 files changed, 70 insertions(+), 6 deletions(-)
>>
>> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
>> index 49206dd..a3a5ced 100644
>> --- a/docs/formatnetwork.html.in
>> +++ b/docs/formatnetwork.html.in
>> @@ -33,7 +33,7 @@
>> </p>
>>
>> <pre>
>> - <network>
>> + <network ipv6='yes'>
>> <name>default</name>
>> <uuid>3e3fce45-4f53-4fa7-bb32-11f34168b82b</uuid>
>> ...</pre>
>> @@ -52,6 +52,12 @@
>> The format must be RFC 4122 compliant, eg <code>3e3fce45-4f53-4fa7-bb32-11f34168b82b</code>.
>> If omitted when defining/creating a new network, a random
>> UUID is generated. <span class="since">Since 0.3.0</span></dd>
>> + <dt><code>ipv6='yes'</code></dt>
>> + <dd>The new, optional parameter <code>ipv6='yes'</code> enables
>> + a network definition with no IPv6 gateway addresses specified
>> + to have guest-to-guest communications. For further information,
>> + see the example below for the example with no gateway addresses.
>> + <span class="since">Since 1.0.1</span></dd>
>> </dl>
>>
>> <h3><a name="elementsConnect">Connectivity</a></h3>
>> @@ -773,5 +779,25 @@
>> </forward>
>> </network></pre>
>>
>> + <h3><a name="examplesNoGateway">Network config with no gateway addresses</a></h3>
>> +
>> + <p>
>> + A valid network definition can contain no IPv4 or IPv6 addresses. Such a definition
>> + can be used for a "very private" or "very isolated" network since it will not be
>> + possible to communicate with the virtualization host via this network. However,
>> + this virtual network interface can be used for communication between virtual guest
>> + systems. This works for IPv4 and <span class="since">(Since 1.0.1)</span> IPv6.
>> + However, the new ipv6='yes' must be added for guest-to-guest IPv6
>> + communication.
>> + </p>
> I reformatted this paragraph to fit within 80 columns.
>
>> +
>> + <pre>
>> + <network ipv6='yes'>
>> + <name>nogw</name>
>> + <uuid>7a3b7497-1ec7-8aef-6d5c-38dff9109e93</uuid>
>> + <bridge name="virbr2" stp="on" delay="0" />
>> + <mac address='00:16:3E:5D:C7:9E'/>
>> + </network></pre>
>> +
>> </body>
>> </html>
>> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
>> index 4abfd91..0d67f7f 100644
>> --- a/docs/schemas/network.rng
>> +++ b/docs/schemas/network.rng
>> @@ -17,6 +17,16 @@
>> <data type="unsignedInt"/>
>> </attribute>
>> </optional>
>> + <!-- Enables IPv6 guest-to-guest communications on a network
>> + with no gateways addresses specified -->
>> + <optional>
>> + <attribute name="ipv6">
>> + <choice>
>> + <value>yes</value>
>> + <value>no</value>
>> + </choice>
>> + </attribute>
>> + </optional>
>> <interleave>
>>
>> <!-- The name of the network, used to refer to it through the API
>> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
>> index 6ce2e63..3f9e13c 100644
>> --- a/src/conf/network_conf.c
>> +++ b/src/conf/network_conf.c
>> @@ -1264,6 +1264,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>> def->uuid_specified = true;
>> }
>>
>> + /* check if definitions with no IPv6 gateway addresses is to
>> + * allow guest-to-guest communications.
>> + */
>> + if (virXPathBoolean("boolean(./@ipv6)", ctxt) == 1)
>> + def->ipv6nogw = true;
> I don't think virXPathBoolean does what you think it does (although I
> haven't figured out exactly what it *does* do, I've noticed that 1) none
> of the rest of libvirt uses it for yes/no attributes, and 2) in the
> libxml2 source code, when an XPATH_BOOLEAN is formatted into a string,
> it becomes "True" or "False". I'm changing the above lines to the following:
>
> ipv6nogwStr = virXPathString("string(./@ipv6)", ctxt);
> if (ipv6nogwStr) {
> if (STREQ(ipv6nogwStr, "yes")) {
> def->ipv6nogw = true;
> } else if (STRNEQ(ipv6nogwStr, "no")) {
> virReportError(VIR_ERR_XML_ERROR,
> _("Invalid ipv6 setting '%s' in network '%s'"),
> ipv6nogwStr, def->name);
> goto error;
> }
> VIR_FREE(ipv6nogwStr);
> }
>
> (with the necessary NULL-initialized declaration of ipv6nogwStr at the
> beginning of the function, and VIR_FREE(ipv6nogwStr) at the error: label).
>
> Depending on the amount of changes I make beyond that, I'll either
> attach an interdiff to this mail, or send an update of your patch with
> that change.
I had previously tried virXPathBoolean() and once you realize that it
returns an int which can have values of -1, 0, and 1 with -1 meaning it
was missing it seems to work OK. It also seems to with on/off or
true/false the same. Since I was setting ipv6='on' in <network>, it
recognized that. But, this works also.
>
>> +
>> /* Parse network domain information */
>> def->domain = virXPathString("string(./domain[1]/@name)", ctxt);
>>
>> @@ -1839,6 +1845,8 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
>> if (!(flags & VIR_NETWORK_XML_INACTIVE) && (def->connections > 0)) {
>> virBufferAsprintf(&buf, " connections='%d'", def->connections);
>> }
>> + if (def->ipv6nogw)
>> + virBufferAddLit(&buf, " ipv6='yes'");
>> virBufferAddLit(&buf, ">\n");
>> virBufferAdjustIndent(&buf, 2);
>> virBufferEscapeString(&buf, "<name>%s</name>\n", def->name);
>> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
>> index 3e46304..949b3d2 100644
>> --- a/src/conf/network_conf.h
>> +++ b/src/conf/network_conf.h
>> @@ -184,6 +184,11 @@ struct _virNetworkDef {
>> virMacAddr mac; /* mac address of bridge device */
>> bool mac_specified;
>>
>> + /* specified if ip6tables rules added
>> + * when no ipv6 gateway addresses specified.
>> + */
>> + bool ipv6nogw;
> Heh. Someday we should go through the driver object structures and
> change all of the ints and bitfields used as booleans into bool...
... but doing it carefully. Sometimes an int is used to
true/false/something-else ... not every answer can be binary witness
what is returned by virXPathBoolean().
>
>> +
>> int forwardType; /* One of virNetworkForwardType constants */
>> int managed; /* managed attribute for hostdev mode */
>>
>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>> index 75f3c3a..cb2997d 100644
>> --- a/src/network/bridge_driver.c
>> +++ b/src/network/bridge_driver.c
>> @@ -1617,13 +1617,18 @@ networkRemoveRoutingIptablesRules(struct network_driver *driver,
>> }
>> }
>>
>> -/* Add all once/network rules required for IPv6 (if any IPv6 addresses are defined) */
>> +/* Add all once/network rules required for IPv6.
>> + * If no IPv6 addresses are defined and <network ipv6='yes'> is
>> + * specified, then allow IPv6 commuinications between virtual systems.
> I changed "virtual systems" to "guests".
I tried to be consistent but sometimes I missed.
>> + * If any IPv6 addresses are defined, then add the rules for regular operation.
> Changed to "then add all rules for regular operation (including
> inter-guest communication)."
>
>> + */
>> static int
>> networkAddGeneralIp6tablesRules(struct network_driver *driver,
>> virNetworkObjPtr network)
>> {
>>
>> - if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0))
>> + if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0) &&
>> + !network->def->ipv6nogw)
>> return 0;
> Fixed odd indentation of !network->def->ipv6nogw, and put braces around
> the body (since the expression is multi-line.
>
>>
>> /* Catch all rules to block forwarding to/from bridges */
>> @@ -1653,6 +1658,10 @@ networkAddGeneralIp6tablesRules(struct network_driver *driver,
>> goto err3;
>> }
>>
>> + /* if no IPv6 addresses are defined, we are done. */
>> + if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0))
>> + return 0;
>> +
>> /* allow DNS over IPv6 */
>> if (iptablesAddTcpInput(driver->iptables, AF_INET6,
>> network->def->bridge, 53) < 0) {
>> @@ -1689,11 +1698,17 @@ static void
>> networkRemoveGeneralIp6tablesRules(struct network_driver *driver,
>> virNetworkObjPtr network)
>> {
>> - if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0))
>> + if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0) &&
>> + !network->def->ipv6nogw)
>> return;
>> + if (virNetworkDefGetIpByIndex(network->def, AF_INET6, 0)) {
>> + iptablesRemoveUdpInput(driver->iptables, AF_INET6, network->def->bridge, 53);
>> + iptablesRemoveTcpInput(driver->iptables, AF_INET6, network->def->bridge, 53);
>> + }
>>
>> - iptablesRemoveUdpInput(driver->iptables, AF_INET6, network->def->bridge, 53);
>> - iptablesRemoveTcpInput(driver->iptables, AF_INET6, network->def->bridge, 53);
>> + /* the following rules are there if no IPv6 address has been defined
>> + * but network->def->ipv6nogw == true
>> + */
>> iptablesRemoveForwardAllowCross(driver->iptables, AF_INET6, network->def->bridge);
>> iptablesRemoveForwardRejectIn(driver->iptables, AF_INET6, network->def->bridge);
>> iptablesRemoveForwardRejectOut(driver->iptables, AF_INET6, network->def->bridge);
> One other thing that we forgot about - a test case for
> networkxml2xmltest (xml2argv is only useful for things that affect
> dhcp). I've added a very simple test case to networkxml2xml(in|out) and
> added it to the list in networkxml2xmltest.c - it defines a network with
> no IP addresses, but with "ipv6='yes'". I also added "ipv6='no'" to
> networkxml2xmlin/isolated-network.xml to make sure that it is accepted
> by the parser and results in ipv6nogw *not* being set (it of course
> won't show up in the output).
I thought I was getting away without a new test ;)
>
> ACK with the changes I've squashed in. I've attached an interdiff of the
> changes I've made to this message, and will push as soon as someone else
> ACKs those additional changes.
>
> Thanks for the contribution! I know it's sometimes a pain to get things
> all the way to pushing, but it really does help keep the expansion
> manageable and prevent unwanted surprises for our diverse set of users.
ACK ... this is good! I appreciate your help on this.
This was a small change (I almost forgot I needed to change the schema).
I can hardly wait for your review of the DHCPv6 and conf-file patches.
BTW. While I appreciate you fixing things up, I am perfectly willing to
make corrections when (not if) you find stuff has not been done "right."
Yes, some of it might be a matter of style difference but consistency
counts when you are trying to understand what some code is doing.
Gene
More information about the libvir-list
mailing list