[draft] fix ipv6 neighbour discovery by installing neighbour entries

As we are already installing per-endpoint routes, this is simple to install IPv6 neighbour entries in sync. This fixes the neighbour discovery in case endpoint routes and host routing are enabled together (in the opposite case we drop ICMPv6 NA messages from pods as they are destined to link-local addresses, see also #23910 for a similar problem).
cilium · Apr 6, 2023 · 45f8c19 · 45f8c19
1 parent db3fa1a
commit 45f8c19
Show file tree

Hide file tree

Showing 11 changed files with 94 additions and 9 deletions.
diff --git a/Makefile b/Makefile
@@ -528,7 +528,7 @@ kind-install-cilium: kind-ready ## Install a local Cilium version into the clust
 	# https://github.com/cilium/cilium-cli/issues/1070
 	cilium install \
 		--chart-directory=$(ROOT_DIR)/install/kubernetes/cilium \
-		--helm-values=$(ROOT_DIR)/contrib/testing/kind-values.yaml \
+		--helm-values=$(ROOT_DIR)/contrib/testing/kind-values-ep-routes.yaml \
 		--version= \
 		>/dev/null 2>&1 &
 

diff --git a/contrib/scripts/kind.sh b/contrib/scripts/kind.sh
@@ -5,7 +5,7 @@
 set -euo pipefail
 
 default_controlplanes=1
-default_workers=1
+default_workers=2
 default_cluster_name=""
 default_image=""
 default_kubeproxy_mode="iptables"

diff --git a/contrib/testing/kind-values-ep-routes.yaml b/contrib/testing/kind-values-ep-routes.yaml
@@ -0,0 +1,41 @@
+debug:
+  enabled: true
+image:
+  override: "localhost:5000/cilium/cilium-dev:local"
+  pullPolicy: Never
+operator:
+  image:
+    override: "localhost:5000/cilium/operator-generic:local"
+    pullPolicy: Never
+    suffix: ""
+  nodeSelector:
+    kubernetes.io/os: linux
+    kubernetes.io/hostname: kind-worker
+ipam:
+  mode: kubernetes
+ipv6:
+  enabled: true
+ipv4:
+  enabled: true
+bpf:
+  monitorAggregation: none
+  masquerade: true
+enableIPv6Masquerade: false
+livenessProbe:
+  failureThreshold: 9999
+readinessProbe:
+  failureThreshold: 9999
+startupProbe:
+  failureThreshold: 9999
+endpointRoutes:
+  enabled: true
+nodePort:
+  enabled: true
+hostPort:
+  enabled: true
+tunnel: vxlan
+autoDirectNodeRoutes: false
+ipv4NativeRoutingCIDR: "10.217.0.0/16"
+hostFirewall:
+  enabled: true
+kubeProxyReplacement: strict
diff --git a/daemon/cmd/kube_proxy_replacement.go b/daemon/cmd/kube_proxy_replacement.go
@@ -456,9 +456,6 @@ func finishKubeProxyReplacementInit() error {
 		// Non-BPF masquerade requires netfilter and hence CT.
 		case option.Config.IptablesMasqueradingEnabled():
 			msg = fmt.Sprintf("BPF host routing requires %s.", option.EnableBPFMasquerade)
-		// All cases below still need to be implemented ...
-		case option.Config.EnableEndpointRoutes && option.Config.EnableIPv6:
-			msg = fmt.Sprintf("BPF host routing is currently not supported with %s when IPv6 is enabled.", option.EnableEndpointRoutes)
 		default:
 			if probes.HaveProgramHelper(ebpf.SchedCLS, asm.FnRedirectNeigh) != nil ||
 				probes.HaveProgramHelper(ebpf.SchedCLS, asm.FnRedirectPeer) != nil {

diff --git a/pkg/datapath/loader/loader.go b/pkg/datapath/loader/loader.go
@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"os/exec"
 	"path"
 	"sync"
 
@@ -94,6 +95,11 @@ func upsertEndpointRoute(ep datapath.Endpoint, ip net.IPNet) error {
 	return route.Upsert(endpointRoute)
 }
 
+func upsertEndpointNeigbourEntry(ip, ifname, mac string) error {
+	_, err := exec.Command("ip", "-6", "neigh", "replace", ip, "dev", ifname, "lladdr", mac).Output()
+	return err
+}
+
 func removeEndpointRoute(ep datapath.Endpoint, ip net.IPNet) error {
 	return route.Delete(route.Route{
 		Prefix: ip,
@@ -102,6 +108,11 @@ func removeEndpointRoute(ep datapath.Endpoint, ip net.IPNet) error {
 	})
 }
 
+func removeEndpointNeigbourEntry(ip, ifname, mac string) error {
+	_, err := exec.Command("ip", "-6", "neigh", "del", ip, "dev", ifname, "lladdr", mac).Output()
+	return err
+}
+
 // We need this function when patching an object file for which symbols were
 // already substituted. During the first symbol substitutions, string symbols
 // were replaced such that:
@@ -332,6 +343,13 @@ func (l *Loader) reloadDatapath(ctx context.Context, ep datapath.Endpoint, dirs
 			if err := upsertEndpointRoute(ep, *iputil.AddrToIPNet(ip)); err != nil {
 				scopedLog.WithError(err).Warn("Failed to upsert route")
 			}
+
+			if mac, err := ep.GetMAC().ToString(); err == nil {
+				if err := upsertEndpointNeigbourEntry(ip.String(), ep.InterfaceName(), mac); err != nil {
+					scopedLog.WithError(err).Warn("Failed to upsert neighbour entry")
+				}
+				scopedLog.Warn(fmt.Sprintf("Upserted neighbour entry %s-%s-%s", ip.String(), ep.InterfaceName(), mac))
+			}
 		}
 	}
 
@@ -474,6 +492,9 @@ func (l *Loader) Unload(ep datapath.Endpoint) {
 
 		if ip := ep.IPv6Address(); ip.IsValid() {
 			removeEndpointRoute(ep, *iputil.AddrToIPNet(ip))
+			if mac, err := ep.GetMAC().ToString(); err == nil {
+				removeEndpointNeigbourEntry(ip.String(), ep.InterfaceName(), mac)
+			}
 		}
 	}
 }

diff --git a/pkg/datapath/loader/template.go b/pkg/datapath/loader/template.go
@@ -30,7 +30,8 @@ var (
 	templateIPv4 = [4]byte{192, 0, 2, 3}
 	templateIPv6 = [16]byte{0x20, 0x01, 0xdb, 0x8, 0x0b, 0xad, 0xca, 0xfe, 0x60, 0x0d, 0xbe, 0xe2, 0x0b, 0xad, 0xca, 0xfe}
 
-	templateMAC = mac.MAC([]byte{0x02, 0x00, 0x60, 0x0D, 0xF0, 0x0D})
+	templateNodeMAC = mac.MAC([]byte{0x02, 0x00, 0x60, 0x0D, 0xF0, 0x0D})
+	templateMAC     = mac.MAC([]byte{0x02, 0x00, 0x60, 0x0D, 0xF0, 0x0C})
 
 	elfMapPrefixes = []string{
 		policymap.MapName,
@@ -101,6 +102,12 @@ func (t *templateCfg) GetIdentityLocked() identity.NumericIdentity {
 // GetNodeMAC returns a well-known dummy MAC address which may be later
 // substituted in the ELF.
 func (t *templateCfg) GetNodeMAC() mac.MAC {
+	return templateNodeMAC
+}
+
+// GetNodeMAC returns a well-known dummy MAC address which may be later
+// substituted in the ELF.
+func (t *templateCfg) GetMAC() mac.MAC {
 	return templateMAC
 }
 

diff --git a/pkg/datapath/types/config.go b/pkg/datapath/types/config.go
@@ -46,6 +46,7 @@ type LoadTimeConfiguration interface {
 	IPv4Address() netip.Addr
 	IPv6Address() netip.Addr
 	GetNodeMAC() mac.MAC
+	GetMAC() mac.MAC
 }
 
 // CompileTimeConfiguration provides datapath implementations a clean interface

diff --git a/pkg/endpoint/cache.go b/pkg/endpoint/cache.go
@@ -138,6 +138,7 @@ func (ep *epInfoCache) IPv6Address() netip.Addr {
 // StateDir returns the directory for the endpoint's (next) state.
 func (ep *epInfoCache) StateDir() string    { return ep.epdir }
 func (ep *epInfoCache) GetNodeMAC() mac.MAC { return ep.mac }
+func (ep *epInfoCache) GetMAC() mac.MAC     { return ep.lxcMAC }
 
 func (ep *epInfoCache) ConntrackLocalLocked() bool {
 	return ep.conntrackLocal

diff --git a/pkg/endpoint/endpoint.go b/pkg/endpoint/endpoint.go
@@ -591,6 +591,11 @@ func (e *Endpoint) GetNodeMAC() mac.MAC {
 	return e.nodeMAC
 }
 
+// GetMAC returns the MAC address of the XXX from this endpoint's perspective.
+func (e *Endpoint) GetMAC() mac.MAC {
+	return e.mac
+}
+
 func (e *Endpoint) HasSidecarProxy() bool {
 	return e.hasSidecarProxy
 }

diff --git a/pkg/mac/mac.go b/pkg/mac/mac.go
@@ -37,6 +37,14 @@ func (m MAC) String() string {
 	return net.HardwareAddr(m).String()
 }
 
+// ToString returns the string representation of m and error if the mac is broken
+func (m MAC) ToString() (string, error) {
+	if m == nil || len(m) != 6 {
+		return "", fmt.Errorf("invalid MAC address %v", m)
+	}
+	return net.HardwareAddr(m).String(), nil
+}
+
 // ParseMAC parses s only as an IEEE 802 MAC-48.
 func ParseMAC(s string) (MAC, error) {
 	ha, err := net.ParseMAC(s)

diff --git a/pkg/testutils/endpoint.go b/pkg/testutils/endpoint.go
@@ -23,6 +23,7 @@ type TestEndpoint struct {
 	Id       uint64
 	Identity *identity.Identity
 	Opts     *option.IntOptions
+	NodeMAC  mac.MAC
 	MAC      mac.MAC
 	IPv6     netip.Addr
 	isHost   bool
@@ -34,7 +35,8 @@ func NewTestEndpoint() TestEndpoint {
 	return TestEndpoint{
 		Id:       42,
 		Identity: defaultIdentity,
-		MAC:      mac.MAC([]byte{0x02, 0x00, 0x60, 0x0D, 0xF0, 0x0D}),
+		NodeMAC:  mac.MAC([]byte{0x02, 0x00, 0x60, 0x0D, 0xF0, 0x0D}),
+		MAC:      mac.MAC([]byte{0x02, 0x00, 0x60, 0x0D, 0xF0, 0x0C}),
 		Opts:     opts,
 	}
 }
@@ -45,7 +47,8 @@ func NewTestHostEndpoint() TestEndpoint {
 	return TestEndpoint{
 		Id:       65535,
 		Identity: hostIdentity,
-		MAC:      mac.MAC([]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06}),
+		NodeMAC:  mac.MAC([]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06}),
+		MAC:      mac.MAC([]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x07}),
 		Opts:     opts,
 		isHost:   true,
 	}
@@ -64,7 +67,8 @@ func (e *TestEndpoint) StringID() string                            { return "42
 func (e *TestEndpoint) GetIdentity() identity.NumericIdentity       { return e.Identity.ID }
 func (e *TestEndpoint) GetIdentityLocked() identity.NumericIdentity { return e.Identity.ID }
 func (e *TestEndpoint) GetSecurityIdentity() *identity.Identity     { return e.Identity }
-func (e *TestEndpoint) GetNodeMAC() mac.MAC                         { return e.MAC }
+func (e *TestEndpoint) GetNodeMAC() mac.MAC                         { return e.NodeMAC }
+func (e *TestEndpoint) GetMAC() mac.MAC                             { return e.MAC }
 func (e *TestEndpoint) GetOptions() *option.IntOptions              { return e.Opts }
 func (e *TestEndpoint) IsHost() bool                                { return e.isHost }