Skip to content

Commit

Permalink
Merge pull request #8875 from sridhartigera/bpf-rt-mgr-fix
Browse files Browse the repository at this point in the history 
Fix skipping v6 external and dsr cidr
  • Loading branch information
@sridhartigera
sridhartigera committed Jun 19, 2024
2 parents 3646463 + b360b39 commit 98081ea
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 43 deletions.
1 change: 0 additions & 1 deletion felix/bpf/xdp/attach.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ func (ap *AttachPoint) AttachProgram() (bpf.AttachResult, error) {
globals.Jumps[tcdefs.ProgIndexPolicy] = uint32(ap.PolicyIdxV4)
}
if ap.HookLayoutV6 != nil {
log.Infof("Sridhar layout6 %+v", ap.HookLayoutV6)
for p, i := range ap.HookLayoutV6 {
globals.JumpsV6[p] = uint32(i)
}
Expand Down
6 changes: 1 addition & 5 deletions felix/config/param_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -559,15 +559,11 @@ func (c *CIDRListParam) Parse(raw string) (result interface{}, err error) {
if len(val) == 0 {
continue
}
ip, net, e := cnet.ParseCIDROrIP(val)
_, net, e := cnet.ParseCIDROrIP(val)
if e != nil {
err = c.parseFailed(in, "invalid CIDR or IP "+val)
return
}
if ip.Version() != 4 {
err = c.parseFailed(in, "invalid CIDR or IP (not v4)")
return
}
resultSlice = append(resultSlice, net.String())
}
return resultSlice, nil
Expand Down
1 change: 0 additions & 1 deletion felix/config/param_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ var _ = DescribeTable("CIDR list parameter parsing",
Entry("Single CIDR", "1.1.1.1/32", []string{"1.1.1.1/32"}, true),
Entry("Single CIDR subnet", "1.1.1.1/24", []string{"1.1.1.0/24"}, true),
Entry("Mix of IP and CIDRs", "1.1.1.1/24, 2.2.2.2", []string{"1.1.1.0/24", "2.2.2.2/32"}, true),
Entry("Reject IPv6", "aabc::1111/32", []string{}, false),
)

var _ = DescribeTable("KeyValue list parameter parsing",
Expand Down
10 changes: 0 additions & 10 deletions felix/dataplane/linux/bpf_route_mgr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package intdataplane

import (
"net"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -110,10 +109,6 @@ func newBPFRouteManager(config *Config, maps *bpfmap.IPMaps, ipFamily proto.IPVe

dirtyCIDRs := set.New[ip.CIDR]()
for _, cidrStr := range config.ExternalNodesCidrs {
if strings.Contains(cidrStr, ":") {
log.WithField("cidr", cidrStr).Debug("Ignoring IPv6 external CIDR")
continue
}
cidr, err := ip.ParseCIDROrIP(cidrStr)
if err != nil {
log.WithError(err).WithField("cidr", cidr).Error(
Expand All @@ -131,11 +126,6 @@ func newBPFRouteManager(config *Config, maps *bpfmap.IPMaps, ipFamily proto.IPVe
noDsrCIDRs := ip.NewCIDRTrie()
something := new(struct{})
for _, cidrStr := range config.BPFDSROptoutCIDRs {
if strings.Contains(cidrStr, ":") {
log.WithField("cidr", cidrStr).Debug("Ignoring IPv6 DSR optout CIDR")
continue
}

cidr, err := ip.ParseCIDROrIP(cidrStr)
if err != nil {
log.WithError(err).WithField("cidr", cidr).Error(
Expand Down
116 changes: 90 additions & 26 deletions felix/fv/bpf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ var _ = describeBPFTests(withTunnel("ipip"), withProto("udp"), withConnTimeLoadB
var _ = describeBPFTests(withTunnel("ipip"), withProto("tcp"))
var _ = describeBPFTests(withTunnel("ipip"), withProto("udp"))
var _ = describeBPFTests(withProto("tcp"), withDSR())
var _ = describeBPFTests(withProto("udp"), withDSR())
var _ = describeBPFTests(withProto("udp"), withDSR(), withIPFamily(6))
var _ = describeBPFTests(withTunnel("ipip"), withProto("tcp"), withDSR())
var _ = describeBPFTests(withTunnel("ipip"), withProto("udp"), withDSR())
var _ = describeBPFTests(withTunnel("wireguard"), withProto("tcp"))
Expand Down Expand Up @@ -184,6 +184,31 @@ FELIX_0/32: local host
FELIX_1/32: remote host
FELIX_2/32: remote host`

const expectedRouteDumpV6 = `111:222::1/128: local host
111:222::1:1/128: remote host
111:222::2:1/128: remote host
FELIX_0/128: local host
FELIX_1/128: remote host
FELIX_2/128: remote host
dead:beef::/64: remote in-pool nat-out
dead:beef::1:0/122: remote workload in-pool nat-out nh FELIX_1
dead:beef::2/128: local workload in-pool nat-out idx -
dead:beef::2:0/122: remote workload in-pool nat-out nh FELIX_2
dead:beef::3/128: local workload in-pool nat-out idx -`

const expectedRouteDumpV6DSR = `111:222::1/128: local host
111:222::1:1/128: remote host
111:222::2:1/128: remote host
FELIX_0/128: local host
FELIX_1/128: remote host
FELIX_2/128: remote host
beaf::/64: remote no-dsr
dead:beef::/64: remote in-pool nat-out
dead:beef::1:0/122: remote workload in-pool nat-out nh FELIX_1
dead:beef::2/128: local workload in-pool nat-out idx -
dead:beef::2:0/122: remote workload in-pool nat-out nh FELIX_2
dead:beef::3/128: local workload in-pool nat-out idx -`

const expectedRouteDumpWithTunnelAddr = `10.65.0.0/16: remote in-pool nat-out
10.65.0.2/32: local workload in-pool nat-out idx -
10.65.0.3/32: local workload in-pool nat-out idx -
Expand Down Expand Up @@ -381,8 +406,8 @@ func describeBPFTests(opts ...bpfTestOpt) bool {
options.ExternalIPs = true
options.ExtraEnvVars["FELIX_BPFExtToServiceConnmark"] = "0x80"
options.ExtraEnvVars["FELIX_HEALTHENABLED"] = "true"
options.ExtraEnvVars["FELIX_BPFDSROptoutCIDRs"] = "245.245.0.0/16,beaf::dead/64"
if !testOpts.ipv6 {
options.ExtraEnvVars["FELIX_BPFDSROptoutCIDRs"] = "245.245.0.0/16"
options.ExtraEnvVars["FELIX_HEALTHHOST"] = "0.0.0.0"
} else {
options.ExtraEnvVars["FELIX_IPV6SUPPORT"] = "true"
Expand Down Expand Up @@ -1309,30 +1334,43 @@ func describeBPFTests(opts ...bpfTestOpt) bool {
}

It("should have correct routes", func() {
if testOpts.ipv6 {
// XXX
return
}
tunnelAddr := ""
tunnelAddrFelix1 := ""
tunnelAddrFelix2 := ""
expectedRoutes := expectedRouteDump
if testOpts.dsr {
expectedRoutes = expectedRouteDumpDSR
}
switch {
case tc.Felixes[0].ExpectedIPIPTunnelAddr != "":
tunnelAddr = tc.Felixes[0].ExpectedIPIPTunnelAddr
tunnelAddrFelix1 = tc.Felixes[1].ExpectedIPIPTunnelAddr
tunnelAddrFelix2 = tc.Felixes[2].ExpectedIPIPTunnelAddr
case tc.Felixes[0].ExpectedVXLANTunnelAddr != "":
tunnelAddr = tc.Felixes[0].ExpectedVXLANTunnelAddr
tunnelAddrFelix1 = tc.Felixes[1].ExpectedVXLANTunnelAddr
tunnelAddrFelix2 = tc.Felixes[2].ExpectedVXLANTunnelAddr
case tc.Felixes[0].ExpectedWireguardTunnelAddr != "":
tunnelAddr = tc.Felixes[0].ExpectedWireguardTunnelAddr
tunnelAddrFelix1 = tc.Felixes[1].ExpectedWireguardTunnelAddr
tunnelAddrFelix2 = tc.Felixes[2].ExpectedWireguardTunnelAddr
if testOpts.ipv6 {
switch {
case tc.Felixes[0].ExpectedVXLANV6TunnelAddr != "":
tunnelAddr = tc.Felixes[0].ExpectedVXLANV6TunnelAddr
tunnelAddrFelix1 = tc.Felixes[1].ExpectedVXLANV6TunnelAddr
tunnelAddrFelix2 = tc.Felixes[2].ExpectedVXLANV6TunnelAddr
case tc.Felixes[0].ExpectedWireguardV6TunnelAddr != "":
tunnelAddr = tc.Felixes[0].ExpectedWireguardV6TunnelAddr
tunnelAddrFelix1 = tc.Felixes[1].ExpectedWireguardV6TunnelAddr
tunnelAddrFelix2 = tc.Felixes[2].ExpectedWireguardV6TunnelAddr
}
expectedRoutes = expectedRouteDumpV6
if testOpts.dsr {
expectedRoutes = expectedRouteDumpV6DSR
}
} else {
switch {
case tc.Felixes[0].ExpectedIPIPTunnelAddr != "":
tunnelAddr = tc.Felixes[0].ExpectedIPIPTunnelAddr
tunnelAddrFelix1 = tc.Felixes[1].ExpectedIPIPTunnelAddr
tunnelAddrFelix2 = tc.Felixes[2].ExpectedIPIPTunnelAddr
case tc.Felixes[0].ExpectedVXLANTunnelAddr != "":
tunnelAddr = tc.Felixes[0].ExpectedVXLANTunnelAddr
tunnelAddrFelix1 = tc.Felixes[1].ExpectedVXLANTunnelAddr
tunnelAddrFelix2 = tc.Felixes[2].ExpectedVXLANTunnelAddr
case tc.Felixes[0].ExpectedWireguardTunnelAddr != "":
tunnelAddr = tc.Felixes[0].ExpectedWireguardTunnelAddr
tunnelAddrFelix1 = tc.Felixes[1].ExpectedWireguardTunnelAddr
tunnelAddrFelix2 = tc.Felixes[2].ExpectedWireguardTunnelAddr
}
}

if tunnelAddr != "" {
Expand All @@ -1343,7 +1381,13 @@ func describeBPFTests(opts ...bpfTestOpt) bool {
}

dumpRoutes := func() string {
out, err := tc.Felixes[0].ExecOutput("calico-bpf", "routes", "dump")
out := ""
var err error
if testOpts.ipv6 {
out, err = tc.Felixes[0].ExecOutput("calico-bpf", "routes", "-6", "dump")
} else {
out, err = tc.Felixes[0].ExecOutput("calico-bpf", "routes", "dump")
}
if err != nil {
return fmt.Sprint(err)
}
Expand All @@ -1361,13 +1405,25 @@ func describeBPFTests(opts ...bpfTestOpt) bool {
l = strings.ReplaceAll(l, felixIP(2), "FELIX_2")
l = idxRE.ReplaceAllLiteralString(l, "idx -")
if tunnelAddr != "" {
l = strings.ReplaceAll(l, tunnelAddr+"/32", "FELIX_0_TNL/32")
if testOpts.ipv6 {
l = strings.ReplaceAll(l, tunnelAddr+"/128", "FELIX_0_TNL/128")
} else {
l = strings.ReplaceAll(l, tunnelAddr+"/32", "FELIX_0_TNL/32")
}
}
if tunnelAddrFelix1 != "" {
l = strings.ReplaceAll(l, tunnelAddrFelix1+"/32", "FELIX_1_TNL/32")
if testOpts.ipv6 {
l = strings.ReplaceAll(l, tunnelAddrFelix1+"/128", "FELIX_1_TNL/128")
} else {
l = strings.ReplaceAll(l, tunnelAddrFelix1+"/32", "FELIX_1_TNL/32")
}
}
if tunnelAddrFelix2 != "" {
l = strings.ReplaceAll(l, tunnelAddrFelix2+"/32", "FELIX_2_TNL/32")
if testOpts.ipv6 {
l = strings.ReplaceAll(l, tunnelAddrFelix2+"/128", "FELIX_2_TNL/128")
} else {
l = strings.ReplaceAll(l, tunnelAddrFelix2+"/32", "FELIX_2_TNL/32")
}
}
filteredLines = append(filteredLines, l)
}
Expand Down Expand Up @@ -4238,8 +4294,16 @@ func describeBPFTests(opts ...bpfTestOpt) bool {
It("should get port unreachable via node1->node0 fwd", func() {
tcpdump := externalClient.AttachTCPDump("any")
tcpdump.SetLogEnabled(true)
matcher := fmt.Sprintf("IP %s > %s: ICMP %s udp port %d unreachable",
felixIP(1), containerIP(externalClient), felixIP(1), npPort)

var matcher string

if testOpts.ipv6 {
matcher = fmt.Sprintf("IP6 %s > %s: ICMP6, destination unreachable, unreachable port, %s udp port %d",
felixIP(1), containerIP(externalClient), felixIP(1), npPort)
} else {
matcher = fmt.Sprintf("IP %s > %s: ICMP %s udp port %d unreachable",
felixIP(1), containerIP(externalClient), felixIP(1), npPort)
}
tcpdump.AddMatcher("ICMP", regexp.MustCompile(matcher))
tcpdump.Start(testOpts.protocol, "port", strconv.Itoa(int(npPort)), "or", icmpProto)
defer tcpdump.Stop()
Expand Down Expand Up @@ -4299,7 +4363,7 @@ func describeBPFTests(opts ...bpfTestOpt) bool {
tgtWorkload.IP, w[1][1].IP, felixIP(1), npPort)
}
tcpdump.AddMatcher("ICMP", regexp.MustCompile(matcher))
tcpdump.Start(testOpts.protocol, "port", strconv.Itoa(int(npPort)), "or", "icmp")
tcpdump.Start(testOpts.protocol, "port", strconv.Itoa(int(npPort)), "or", icmpProto)
}
defer tcpdump.Stop()

Expand Down

0 comments on commit 98081ea

Please sign in to comment.