Enable IPv6 on OVS internal port if needed in bridging mode

[antoninbas]

Enable IPv6 on OVS internal port if needed in bridging mode

The uplink interface may have an IPv6 address, while the interface
created by OVS for the internal port may not support IPv6. For example,
such a situation has been observed in a Kind cluster, with IPv6 enabled
on the uplink but disabled by default on new interfaces:

root@kind-worker:/# sysctl net.ipv6.conf.all.disable_ipv6
net.ipv6.conf.all.disable_ipv6 = 1
root@kind-worker:/# sysctl net.ipv6.conf.default.disable_ipv6
net.ipv6.conf.default.disable_ipv6 = 1
root@kind-worker:/# sysctl net.ipv6.conf.eth0.disable_ipv6
net.ipv6.conf.eth0.disable_ipv6 = 0

When we detect that uplink addresses include an IPv6 address, we will
now ensure that IPv6 is enabled on the bridge port (using sysctl),
before attempting to move the addresses over. If it fails, we will
proceed with the rest of the initialization, but moving the IP addresses
to the bridge is very likely to be unsuccessful in that case.

We also make bridge cleanup more robust, by saving all uplink IP
addresses in the uplink config, and using the saved values to restore
the uplink interface. This ensures that cleanup can succeed, even if
bridge configuration failed half-way, as was the case in #5368.

Fixes #5368

Signed-off-by: Antonin Bas abas@vmware.com

[gran-vmv]

Is it possible to check the "sysctl parameter" or some other flag and then move IPv6 addresses on success and log a warning on failure?
Although antrea doesn't support flexible-ipam + IPv6, we still support user to config IPv6 addresses on uplink, thus why antrea agent moves IPv6 addresses from uplink to bridge interface.

[antoninbas]

@gran-vmv we can use sysctl, which is why I wrote in the comment:

Rather than try to enable IPv6 on the new interface, which requires setting a sysctl parameter, we simply skip moving the IPv6 addresses for now.

However, can you clarify why moving the IPv6 address is needed with the current code? I saw a couple of TODOs in the code for IPv6. For example, IPv6 routes are not currently handled (saved / restored):

antrea/pkg/agent/agent_linux.go

Lines 141 to 146 in 35c113b

	// Skip IPv6 routes before we support IPv6 stack.
	// TODO(gran): support IPv6
	if route.Gw.To4() == nil {
	klog.V(2).Infof("Skipped IPv6 host route: %+v", route)
	continue
	}

[gran-vmv]

@gran-vmv we can use sysctl, which is why I wrote in the comment:

Rather than try to enable IPv6 on the new interface, which requires setting a sysctl parameter, we simply skip moving the IPv6 addresses for now.

However, can you clarify why moving the IPv6 address is needed with the current code? I saw a couple of TODOs in the code for IPv6. For example, IPv6 routes are not currently handled (saved / restored):

antrea/pkg/agent/agent_linux.go

Lines 141 to 146 in 35c113b

	// Skip IPv6 routes before we support IPv6 stack.
	// TODO(gran): support IPv6
	if route.Gw.To4() == nil {
	klog.V(2).Infof("Skipped IPv6 host route: %+v", route)
	continue
	}

@gran-vmv we can use sysctl, which is why I wrote in the comment:

Rather than try to enable IPv6 on the new interface, which requires setting a sysctl parameter, we simply skip moving the IPv6 addresses for now.

However, can you clarify why moving the IPv6 address is needed with the current code? I saw a couple of TODOs in the code for IPv6. For example, IPv6 routes are not currently handled (saved / restored):

antrea/pkg/agent/agent_linux.go

Lines 141 to 146 in 35c113b

	// Skip IPv6 routes before we support IPv6 stack.
	// TODO(gran): support IPv6
	if route.Gw.To4() == nil {
	klog.V(2).Infof("Skipped IPv6 host route: %+v", route)
	continue
	}

For current change, I think it is acceptable, but we might add this in release notes since this is a behavior change.

[antoninbas]

I spent more time thinking about this today. In the end, I think that it makes more sense to do the sysctl approach right away. Not moving the IPv6 address could mean an unexpected loss of IPv6 connectivity for users. I will update the PR.

[antoninbas]

/test-flexible-ipam-e2e

[antoninbas]

/test-flexible-ipam-e2e

[gran-vmv]

/test-flexible-ipam-e2e

[jianjuns]

In commit message:

For example, such as situation

"such an situation"

[antoninbas]

/test-all
/test-flexible-ipam-e2e

[tnqn]

LGTM

[antoninbas]

I will backport to 1.13