[libvirt] [PATCHv2 1/3] v1: allow guest to guest IPv6 without gateway definition

[Gene Czarcinski]

On 12/02/2012 12:39 PM, Laine Stump wrote:
> On 11/30/2012 10:01 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.
> Yeah :-(, it's a rather unfortunate chain of events that leads to this,
> caused by the following:
>
> 1) the bridge module in the kernel has "1" compiled in as the default,
> so that's what is set when the module is first loaded,
>
> 2) the settings in sysctl.conf are set during startup of either the
> NetworkManager or network service (depending on which you've chosen).
> This is done after any bridges in the system config are started, so if
> you have a bridge defined there, the bridge module will be loaded, then
> "sysctl -p" will be run, thus making sure the setting from
> /etc/sysctl.conf is enacted. If the system has no bridges defined,
> "sysctl -p" will fall on deaf ears, since those tunables don't exist yet.
>
> 3) sometime later libvirt creates a bridge device with an ioctl().
> Unknown to libvirt, this causes the bridge module to autoload, and it of
> course starts up with the compiled-in default settings of "1".
>
> For many reasons it's not desirable for libvirt to run "sysctl -p"
> (potential side effects on other completely unrelated subsystems) or
> even manually set those few items (maybe the admin really *does* want
> that policy).
>
>> Note:  If it is desired to control this behavior by having something like
>> ipv6='yes' on the <network> statement, then this should also be done for ipv4.
> It's always nicer to not have to add another knob, and I was hoping that
> we could get by on this one without it. There were a few potential
> issues that we've discussed previously that could make it necessary to
> not turn this on by default:
>
> 1) unintentionally opening up a communications line to the host via the
> IPv6 link local address - as long as disable_ipv6 is set for the
> interface, this isn't an issue.
I did not need to do anything.  If no IPv6 addresses are specified then 
this network:
<network>
   <name>nogw</name>
   <uuid>7a3b7497-1ec7-8aef-6d5c-38dff9109e93</uuid>
   <bridge name='virbr19' stp='on' delay='0' />
   <mac address='52:54:00:08:10:43'/>
</network>

results in:
# sysctl -a | grep ipv6 | grep virbr19 | grep disable
net.ipv6.conf.virbr19.disable_ipv6 = 1
net.ipv6.conf.virbr19-nic.disable_ipv6 = 0
>
> 2) some installations don't have ip6tables - my opinion on this one is
> that ip6tables is part of the iptables package, and people shouldn't be
> building/installing just "part" of a package anyway.
Mmm.  However, there a point here.  This is the only time when not 
specifying an IPv6 address results in something done with ip6tables.  
Maybe there should be a test added to make sure that it exists and is 
executable (such as that done for dnsmasq).
>
> 3) Even if ip6tables is available on the system, the additional commands
> may fail on some systems - this is our Waterloo :-( I finally just now
> tried disabling the ipv6 module on a RHEL6 system, then running
> "ip6tables -S". I was greeted by the following error message and a non-0
> exit code:
>
>    ip6tables v1.4.7: can't initialize ip6tables table `filter': Address
> family not supported by protocol
>    Perhaps ip6tables or your kernel needs to be upgraded.
>    [root at rhel6-guest laine]# echo $?
>    3
>
> This means that the addition of this patch as-is would result in libvirt
> failing to start any network on a system that had disabled IPv6, and my
> guess is there are still a lot of those around.
>
> So we're unfortunately going to have to add a <network ipv6='yes'> to
> enable this; if you don't have the time, I'll add that tonight and repost.
Actually, since the dnsmasq "extra" RA to the real NIC bug has been 
found, a fix tested, and the promise by Simon Kelley that it will be in 
2.64 final, I do have the time ;)
>
> (I disagree that an ipv4 attribute is also needed. the rules to allow
> ipv4 have been there by default since the networks were added to
> libvirt, so we don't want them disabled by default, and up to now nobody
> has ever asked for them to be manually disabled, so there apparently
> isn't a need for it. If it ever does come up, we can add the attribute
> then, but I'd rather not bloat the grammar just for the sake of symmetry.)
I like the symmetry of the ipv4 parameter but it is certainly not worth 
any argument.

So the default is ipv6='no' and <network ipv6='yes'> will need to be 
specified for the code added by this patch to work.

Gene
>
> Other than that, the patch looks fine.
>
>> ---
>>   docs/formatnetwork.html.in  | 18 ++++++++++++++++++
>>   src/network/bridge_driver.c | 22 ++++++++++++++--------
>>   2 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
>> index 49206dd..7b3b25c 100644
>> --- a/docs/formatnetwork.html.in
>> +++ b/docs/formatnetwork.html.in
>> @@ -773,5 +773,23 @@
>>           </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.
>> +    </p>
>> +
>> +    <pre>
>> +      <network>
>> +        <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/src/network/bridge_driver.c b/src/network/bridge_driver.c
>> index 75f3c3a..0fdb635 100644
>> --- a/src/network/bridge_driver.c
>> +++ b/src/network/bridge_driver.c
>> @@ -1617,15 +1617,16 @@ 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.
>> + * Even if no IPv6 addresses are defined, allow IPv6 commuinications
>> + * between virtual systems.  If any IPv6 addresses are defined, then
>> + * add the rules for regular operation.
>> + */
>>   static int
>>   networkAddGeneralIp6tablesRules(struct network_driver *driver,
>>                                  virNetworkObjPtr network)
>>   {
>>   
>> -    if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0))
>> -        return 0;
>> -
>>       /* Catch all rules to block forwarding to/from bridges */
>>   
>>       if (iptablesAddForwardRejectOut(driver->iptables, AF_INET6,
>> @@ -1653,6 +1654,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 +1694,12 @@ static void
>>   networkRemoveGeneralIp6tablesRules(struct network_driver *driver,
>>                                     virNetworkObjPtr network)
>>   {
>> -    if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0))
>> -        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 even if no IPv6 address has been defined */
>>       iptablesRemoveForwardAllowCross(driver->iptables, AF_INET6, network->def->bridge);
>>       iptablesRemoveForwardRejectIn(driver->iptables, AF_INET6, network->def->bridge);
>>       iptablesRemoveForwardRejectOut(driver->iptables, AF_INET6, network->def->bridge);
>