[libvirt] [PATCH 1/3] Only check for IP forwarding, do not enable it

[Benjamin Cama]

Hi everyone,

Le lundi 15 octobre 2012 à 09:36 -0700, Cole Robinson a écrit :
> On 10/15/2012 12:04 PM, Cole Robinson wrote:
> > On 10/15/2012 10:54 AM, Michal Privoznik wrote:
> >> On 15.10.2012 12:26, Benjamin Cama wrote:
> >>>  
> >>>  static int
> >>>  networkSetIPv6Sysctls(virNetworkObjPtr network)
> >>> @@ -2140,11 +2172,9 @@ networkStartNetworkVirtual(struct network_driver 
> >>> *driver,
> >>>      if (virNetDevSetOnline(network->def->bridge, 1) < 0)
> >>>          goto err2;
> >>>  
> >>> -    /* If forwardType != NONE, turn on global IP forwarding */
> >>> +    /* If forwardType != NONE, check for IP forwarding */
> >>>      if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE &&
> >>> -        networkEnableIpForwarding(v4present, v6present) < 0) {
> >>> -        virReportSystemError(errno, "%s",
> >>> -                             _("failed to enable IP forwarding"));
> >>> +        networkCheckIpForwarding(v4present, v6present) < 0) {
> >>>          goto err3;
> >>>      }
> >>>  
> >>>
> >> Well, I am not sure if we can do this. What would happen if some of our
> >> users rely on this already? I mean, it's there since ages.
> >>
> >> Michal
> > Indeed this kinda defeats the purpose of the default virtual network that
> > should 'just work' out of the box. Maybe we could add some libvirtd.conf
> > option to enable this check-if-set behavior, but we can't change the default 
> > here.
> 
> We've had this discussion before:
> 
> 
>   http://www.redhat.com/archives/libvir-list/2010-October/msg00030.html
> 
> and in particular this response:
> 
>   http://www.redhat.com/archives/libvir-list/2010-October/msg00183.html

Thanks for the links.

> In the end, the presence of a network with a forward mode that requires
> L3 packet forwarding indicates tacit approval for ip_forward to be
> turned on. The problem in the past has been that the default network
> (which has <forward mode='nat'>) was a part of *all* libvirt installs.
> That is now separated into its own sub-package, though.
> 
> So, the "config option" is to simply not install the default network (or
> to remove it if it's there).

I understand that changing the behavior of a function that has been
“just working” for years sounds unacceptable. It's just that for IPv6,
enabling forwarding has far more consequences than for IPv4. But I
understand that my use case may be rare enough not to change the default
behavior. Still, I would like to implement some big warning when
changing the forwarding state. I will work on that.

Regards,
-- 
Benjamin Cama <benjamin.cama at telecom-bretagne.eu>