Skip to content

Commit

Permalink
TODO
Browse files Browse the repository at this point in the history
Signed-off-by: Dumitru Ceara <[email protected]>
  • Loading branch information
dceara committed Sep 17, 2024
1 parent 1002521 commit de54993
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 37 deletions.
1 change: 1 addition & 0 deletions go-controller/pkg/node/gateway_init_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,7 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
)
expectedTables["nat"]["OVN-KUBE-UDN-MASQUERADE"] = append(expectedTables["nat"]["OVN-KUBE-UDN-MASQUERADE"],
"-s 169.254.169.2/29 -j RETURN", // this guarantees we don't SNAT default network masqueradeIPs
"-d 172.16.1.0/24 -j RETURN", // this guarantees we don't SNAT traffic destined to the service cidr
"-s 169.254.169.0/29 -j MASQUERADE", // this guarantees we SNAT all UDN MasqueradeIPs traffic leaving the node
)
}
Expand Down
56 changes: 49 additions & 7 deletions go-controller/pkg/node/gateway_shared_intf.go
Original file line number Diff line number Diff line change
Expand Up @@ -1291,10 +1291,28 @@ func flowsForDefaultBridge(bridge *bridgeConfiguration, extraIPs []net.IP) ([]st
}

// table 0, Host -> OVN towards SVC, SNAT to special IP
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, priority=500, in_port=%s, %s, %s_dst=%s,"+
"actions=ct(commit,zone=%d,nat(src=%s),table=2)",
defaultOpenFlowCookie, ofPortHost, protoPrefix, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone, masqIP))
for _, netConfig := range bridge.patchedNetConfigs() {
if netConfig.masqCTMark == ctMarkOVN {
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, priority=500, in_port=%s, %s, %s_dst=%s, "+
"actions=ct(commit,zone=%d,nat(src=%s),table=2)",
defaultOpenFlowCookie, ofPortHost, protoPrefix, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone, masqIP))
} else {
//TODO: ugly
var mgmtMasqIP string

if utilnet.IsIPv4CIDR(svcCIDR) {
mgmtMasqIP = netConfig.v4MasqIPs.ManagementPort.IP.String()
} else {
mgmtMasqIP = netConfig.v6MasqIPs.ManagementPort.IP.String()
}
//TODO (dceara): we commit on every packet
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, priority=550, in_port=%s, %s, %s_src=%s, %s_dst=%s, "+
"actions=ct(commit,zone=%d,table=2)",
defaultOpenFlowCookie, ofPortHost, protoPrefix, protoPrefix, mgmtMasqIP, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone))
}
}

for _, netConfig := range bridge.patchedNetConfigs() {
// table 0, Reply hairpin traffic to host, coming from OVN, unSNAT
Expand Down Expand Up @@ -1387,9 +1405,33 @@ func flowsForDefaultBridge(bridge *bridgeConfiguration, extraIPs []net.IP) ([]st
defaultNetConfig := bridge.netConfig[types.DefaultNetworkName]
// table 2, dispatch from Host -> OVN
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, table=2, "+
fmt.Sprintf("cookie=%s, priority=100, table=2, "+
"actions=set_field:%s->eth_dst,output:%s", defaultOpenFlowCookie, bridgeMacAddress, defaultNetConfig.ofPortPatch))

// TODO: comment about extra match on masq ip as source
if config.IPv4Mode {
for _, netConfig := range bridge.patchedNetConfigs() {
if netConfig.masqCTMark == ctMarkOVN {
continue
}
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, priority=200, table=2, ip, ip_src=%s, "+
"actions=set_field:%s->eth_dst,output:%s", defaultOpenFlowCookie, netConfig.v4MasqIPs.ManagementPort.IP, bridgeMacAddress, netConfig.ofPortPatch))
}
}

if config.IPv6Mode {
for _, netConfig := range bridge.patchedNetConfigs() {
if netConfig.masqCTMark == ctMarkOVN {
continue
}

dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, priority=200, table=2, ip6, ipv6_src=%s, "+
"actions=set_field:%s->eth_dst,output:%s", defaultOpenFlowCookie, netConfig.v6MasqIPs.ManagementPort.IP, bridgeMacAddress, netConfig.ofPortPatch))
}
}

// table 3, dispatch from OVN -> Host
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, table=3, "+
Expand Down Expand Up @@ -1489,7 +1531,7 @@ func commonFlows(subnets []*net.IPNet, bridge *bridgeConfiguration) ([]string, e
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, priority=100, in_port=%s, dl_src=%s, ip, ip_src=%s, "+
"actions=ct(commit, zone=%d, nat(src=%s), exec(set_field:%s->ct_mark)), output:%s",
defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, netConfig.v4MasqIP.IP, config.Default.ConntrackZone,
defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, netConfig.v4MasqIPs.GatewayRouter.IP, config.Default.ConntrackZone,
physicalIP.IP, netConfig.masqCTMark, ofPortPhys))
}
}
Expand Down Expand Up @@ -1564,7 +1606,7 @@ func commonFlows(subnets []*net.IPNet, bridge *bridgeConfiguration) ([]string, e
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, priority=100, in_port=%s, dl_src=%s, ipv6, ipv6_src=%s, "+
"actions=ct(commit, zone=%d, nat(src=%s), exec(set_field:%s->ct_mark)), output:%s",
defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, netConfig.v6MasqIP.IP, config.Default.ConntrackZone,
defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, netConfig.v6MasqIPs.GatewayRouter.IP, config.Default.ConntrackZone,
physicalIP.IP, netConfig.masqCTMark, ofPortPhys))
}
}
Expand Down
35 changes: 17 additions & 18 deletions go-controller/pkg/node/gateway_udn.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ type UserDefinedNetworkGateway struct {
// masqCTMark holds the mark value for this network
// which is used for egress traffic in shared gateway mode
masqCTMark uint
// v4MasqIP holds the IPv4 masquerade IP for this network
v4MasqIP *net.IPNet
// v6MasqIP holds the IPv6 masquerade IP for this network
v6MasqIP *net.IPNet
// v4MasqIPs holds the IPv4 masquerade IPs for this network
v4MasqIPs *udn.MasqueradeIPs
// v6MasqIPs holds the IPv6 masquerade IPs for this network
v6MasqIPs *udn.MasqueradeIPs
// stores the pointer to default network's gateway so that
// we can leverage it from here to program UDN flows on breth0
// Currently we use the openflowmanager and nodeIPManager from
Expand Down Expand Up @@ -83,7 +83,7 @@ func (b *bridgeConfiguration) getBridgePortConfigurations() ([]bridgeUDNConfigur
}

// addNetworkBridgeConfig adds the patchport and ctMark value for the provided netInfo into the bridge configuration cache
func (b *bridgeConfiguration) addNetworkBridgeConfig(nInfo util.NetInfo, masqCTMark uint, v4MasqIP, v6MasqIP *net.IPNet) {
func (b *bridgeConfiguration) addNetworkBridgeConfig(nInfo util.NetInfo, masqCTMark uint, v4MasqIPs, v6MasqIPs *udn.MasqueradeIPs) {
b.Lock()
defer b.Unlock()

Expand All @@ -95,8 +95,8 @@ func (b *bridgeConfiguration) addNetworkBridgeConfig(nInfo util.NetInfo, masqCTM
netConfig := &bridgeUDNConfiguration{
patchPort: patchPort,
masqCTMark: fmt.Sprintf("0x%x", masqCTMark),
v4MasqIP: v4MasqIP,
v6MasqIP: v6MasqIP,
v4MasqIPs: v4MasqIPs,
v6MasqIPs: v6MasqIPs,
}

b.netConfig[netName] = netConfig
Expand Down Expand Up @@ -149,8 +149,8 @@ type bridgeUDNConfiguration struct {
patchPort string
ofPortPatch string
masqCTMark string
v4MasqIP *net.IPNet
v6MasqIP *net.IPNet
v4MasqIPs *udn.MasqueradeIPs
v6MasqIPs *udn.MasqueradeIPs
}

func (netConfig *bridgeUDNConfiguration) setBridgeNetworkOfPortsInternal() error {
Expand Down Expand Up @@ -179,23 +179,22 @@ func NewUserDefinedNetworkGateway(netInfo util.NetInfo, networkID int, node *v1.
defaultNetworkGateway Gateway) (*UserDefinedNetworkGateway, error) {
// Generate a per network conntrack mark and masquerade IPs to be used for egress traffic.
var (
v4MasqIP *net.IPNet
v6MasqIP *net.IPNet
v4MasqIPs *udn.MasqueradeIPs
v6MasqIPs *udn.MasqueradeIPs
err error
)
masqCTMark := ctMarkUDNBase + uint(networkID)
if config.IPv4Mode {
v4MasqIPs, err := udn.AllocateV4MasqueradeIPs(networkID)
v4MasqIPs, err = udn.AllocateV4MasqueradeIPs(networkID)
if err != nil {
return nil, fmt.Errorf("failed to get v4 masquerade IP, network %s (%d): %v", netInfo.GetNetworkName(), networkID, err)
}
v4MasqIP = v4MasqIPs.GatewayRouter
}
if config.IPv6Mode {
v6MasqIPs, err := udn.AllocateV6MasqueradeIPs(networkID)
v6MasqIPs, err = udn.AllocateV6MasqueradeIPs(networkID)
if err != nil {
return nil, fmt.Errorf("failed to get v6 masquerade IP, network %s (%d): %v", netInfo.GetNetworkName(), networkID, err)
}
v6MasqIP = v6MasqIPs.GatewayRouter
}

gw, ok := defaultNetworkGateway.(*gateway)
Expand All @@ -211,8 +210,8 @@ func NewUserDefinedNetworkGateway(netInfo util.NetInfo, networkID int, node *v1.
kubeInterface: kubeInterface,
vrfManager: vrfManager,
masqCTMark: masqCTMark,
v4MasqIP: v4MasqIP,
v6MasqIP: v6MasqIP,
v4MasqIPs: v4MasqIPs,
v6MasqIPs: v6MasqIPs,
gateway: gw,
ruleManager: ruleManager,
}, nil
Expand Down Expand Up @@ -258,7 +257,7 @@ func (udng *UserDefinedNetworkGateway) AddNetwork() error {
return fmt.Errorf("could not set loose mode for reverse path filtering on management port %s: %v", mgmtPortName, err)
}
if udng.openflowManager != nil {
udng.openflowManager.addNetwork(udng.NetInfo, udng.masqCTMark, udng.v4MasqIP, udng.v6MasqIP)
udng.openflowManager.addNetwork(udng.NetInfo, udng.masqCTMark, udng.v4MasqIPs, udng.v6MasqIPs)

waiter := newStartupWaiterWithTimeout(waitForPatchPortTimeout)
readyFunc := func() (bool, error) {
Expand Down
110 changes: 106 additions & 4 deletions go-controller/pkg/node/gateway_udn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
utilnet "k8s.io/utils/net"
"k8s.io/utils/ptr"

"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
Expand Down Expand Up @@ -176,6 +177,66 @@ func setUpUDNOpenflowManagerFakeOVSCommands(fexec *ovntest.FakeExec) {
})
}

func checkDefaultSvcIsolationOVSFlows(flows []string, defaultConfig *bridgeUDNConfiguration, ofPortHost, bridgeMAC string, svcCIDR *net.IPNet) {
By(fmt.Sprintf("Checking default service isolation flows for %s", svcCIDR.String()))

var masqIP string
var protoPrefix string
if utilnet.IsIPv4CIDR(svcCIDR) {
protoPrefix = "ip"
masqIP = config.Gateway.MasqueradeIPs.V4HostMasqueradeIP.String()
} else {
protoPrefix = "ip6"
masqIP = config.Gateway.MasqueradeIPs.V6HostMasqueradeIP.String()
}

var nTable0Flows int
var nTable2Flows int
for _, flow := range flows {
if strings.Contains(flow, fmt.Sprintf("priority=500, in_port=%s, %s, %s_dst=%s, actions=ct(commit,zone=%d,nat(src=%s),table=2)",
ofPortHost, protoPrefix, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone,
masqIP)) {
nTable0Flows++
} else if strings.Contains(flow, fmt.Sprintf("priority=100, table=2, actions=set_field:%s->eth_dst,output:%s",
bridgeMAC, defaultConfig.ofPortPatch)) {
nTable2Flows++
}
}

Expect(nTable0Flows).To(Equal(1))
Expect(nTable2Flows).To(Equal(1))
}

func checkUDNSvcIsolationOVSFlows(flows []string, netConfig *bridgeUDNConfiguration, netName, ofPortHost, bridgeMAC string, svcCIDR *net.IPNet, expectedNFlows int) {
By(fmt.Sprintf("Checking UDN %s service isolation flows for %s; expected %d flows",
netName, svcCIDR.String(), expectedNFlows))

var mgmtMasqIP string
var protoPrefix string
if utilnet.IsIPv4CIDR(svcCIDR) {
mgmtMasqIP = netConfig.v4MasqIPs.ManagementPort.IP.String()
protoPrefix = "ip"
} else {
mgmtMasqIP = netConfig.v6MasqIPs.ManagementPort.IP.String()
protoPrefix = "ip6"
}

var nTable0Flows int
var nTable2Flows int
for _, flow := range flows {
if strings.Contains(flow, fmt.Sprintf("priority=550, in_port=%s, %s, %s_src=%s, %s_dst=%s, actions=ct(commit,zone=%d,table=2)",
ofPortHost, protoPrefix, protoPrefix, mgmtMasqIP, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone)) {
nTable0Flows++
} else if strings.Contains(flow, fmt.Sprintf("priority=200, table=2, %s, %s_src=%s, actions=set_field:%s->eth_dst,output:%s",
protoPrefix, protoPrefix, mgmtMasqIP, bridgeMAC, netConfig.ofPortPatch)) {
nTable2Flows++
}
}

Expect(nTable0Flows).To(Equal(expectedNFlows))
Expect(nTable2Flows).To(Equal(expectedNFlows))
}

var _ = Describe("UserDefinedNetworkGateway", func() {
var (
netName = "bluenet"
Expand Down Expand Up @@ -522,6 +583,7 @@ var _ = Describe("UserDefinedNetworkGateway", func() {
// FIXME: extract openflow manager func from the spawning of a go routine so it can be called directly below.
localGw.openflowManager.syncFlows()
flowMap := udnGateway.gateway.openflowManager.flowCache

Expect(len(flowMap["DEFAULT"])).To(Equal(45))
Expect(udnGateway.masqCTMark).To(Equal(udnGateway.masqCTMark))
var udnFlows int
Expand All @@ -539,20 +601,32 @@ var _ = Describe("UserDefinedNetworkGateway", func() {

Expect(udnGateway.AddNetwork()).To(Succeed())
flowMap = udnGateway.gateway.openflowManager.flowCache
Expect(len(flowMap["DEFAULT"])).To(Equal(59)) // 14 UDN Flows are added by default
Expect(len(flowMap["DEFAULT"])).To(Equal(62)) // 17 UDN Flows are added by default
Expect(len(udnGateway.openflowManager.defaultBridge.netConfig)).To(Equal(2)) // default network + UDN network
defaultUdnConfig := udnGateway.openflowManager.defaultBridge.netConfig["default"]
bridgeUdnConfig := udnGateway.openflowManager.defaultBridge.netConfig["bluenet"]
bridgeMAC := udnGateway.openflowManager.defaultBridge.macAddress.String()
ofPortHost := udnGateway.openflowManager.defaultBridge.ofPortHost
for _, flows := range flowMap {
for _, flow := range flows {
if strings.Contains(flow, fmt.Sprintf("0x%x", udnGateway.masqCTMark)) {
// UDN Flow
udnFlows++
} else if strings.Contains(flow, fmt.Sprintf("in_port=%s", udnGateway.openflowManager.defaultBridge.netConfig["bluenet"].ofPortPatch)) {
} else if strings.Contains(flow, fmt.Sprintf("in_port=%s", bridgeUdnConfig.ofPortPatch)) {
udnFlows++
}
}
}
Expect(udnFlows).To(Equal(14))

for _, svcCIDR := range config.Kubernetes.ServiceCIDRs {
// Check flows for default network service CIDR.
checkDefaultSvcIsolationOVSFlows(flowMap["DEFAULT"], defaultUdnConfig, ofPortHost, bridgeMAC, svcCIDR)

// Expect exactly one flow per UDN for tables 0 and 2 for service isolation.
checkUDNSvcIsolationOVSFlows(flowMap["DEFAULT"], bridgeUdnConfig, "bluenet", ofPortHost, bridgeMAC, svcCIDR, 1)
}

cnode := node.DeepCopy()
kubeMock.On("UpdateNodeStatus", cnode).Return(nil) // check if network key gets deleted from annotation
Expect(udnGateway.DelNetwork()).To(Succeed())
Expand All @@ -569,6 +643,14 @@ var _ = Describe("UserDefinedNetworkGateway", func() {
}
}
Expect(udnFlows).To(Equal(0))

for _, svcCIDR := range config.Kubernetes.ServiceCIDRs {
// Check flows for default network service CIDR.
checkDefaultSvcIsolationOVSFlows(flowMap["DEFAULT"], defaultUdnConfig, ofPortHost, bridgeMAC, svcCIDR)

// Expect no more flows per UDN for tables 0 and 2 for service isolation.
checkUDNSvcIsolationOVSFlows(flowMap["DEFAULT"], bridgeUdnConfig, "bluenet", ofPortHost, bridgeMAC, svcCIDR, 0)
}
return nil
})
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -711,20 +793,32 @@ var _ = Describe("UserDefinedNetworkGateway", func() {

Expect(udnGateway.AddNetwork()).To(Succeed())
flowMap = udnGateway.gateway.openflowManager.flowCache
Expect(len(flowMap["DEFAULT"])).To(Equal(59)) // 14 UDN Flows are added by default
Expect(len(flowMap["DEFAULT"])).To(Equal(62)) // 14 UDN Flows are added by default
Expect(len(udnGateway.openflowManager.defaultBridge.netConfig)).To(Equal(2)) // default network + UDN network
defaultUdnConfig := udnGateway.openflowManager.defaultBridge.netConfig["default"]
bridgeUdnConfig := udnGateway.openflowManager.defaultBridge.netConfig["bluenet"]
bridgeMAC := udnGateway.openflowManager.defaultBridge.macAddress.String()
ofPortHost := udnGateway.openflowManager.defaultBridge.ofPortHost
for _, flows := range flowMap {
for _, flow := range flows {
if strings.Contains(flow, fmt.Sprintf("0x%x", udnGateway.masqCTMark)) {
// UDN Flow
udnFlows++
} else if strings.Contains(flow, fmt.Sprintf("in_port=%s", udnGateway.openflowManager.defaultBridge.netConfig["bluenet"].ofPortPatch)) {
} else if strings.Contains(flow, fmt.Sprintf("in_port=%s", bridgeUdnConfig.ofPortPatch)) {
udnFlows++
}
}
}
Expect(udnFlows).To(Equal(14))

for _, svcCIDR := range config.Kubernetes.ServiceCIDRs {
// Check flows for default network service CIDR.
checkDefaultSvcIsolationOVSFlows(flowMap["DEFAULT"], defaultUdnConfig, ofPortHost, bridgeMAC, svcCIDR)

// Expect exactly one flow per UDN for tables 0 and 2 for service isolation.
checkUDNSvcIsolationOVSFlows(flowMap["DEFAULT"], bridgeUdnConfig, "bluenet", ofPortHost, bridgeMAC, svcCIDR, 1)
}

cnode := node.DeepCopy()
kubeMock.On("UpdateNodeStatus", cnode).Return(nil) // check if network key gets deleted from annotation
Expect(udnGateway.DelNetwork()).To(Succeed())
Expand All @@ -741,6 +835,14 @@ var _ = Describe("UserDefinedNetworkGateway", func() {
}
}
Expect(udnFlows).To(Equal(0))

for _, svcCIDR := range config.Kubernetes.ServiceCIDRs {
// Check flows for default network service CIDR.
checkDefaultSvcIsolationOVSFlows(flowMap["DEFAULT"], defaultUdnConfig, ofPortHost, bridgeMAC, svcCIDR)

// Expect no more flows per UDN for tables 0 and 2 for service isolation.
checkUDNSvcIsolationOVSFlows(flowMap["DEFAULT"], bridgeUdnConfig, "bluenet", ofPortHost, bridgeMAC, svcCIDR, 0)
}
return nil
})
Expect(err).NotTo(HaveOccurred())
Expand Down
7 changes: 4 additions & 3 deletions go-controller/pkg/node/openflow_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/generator/udn"
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"

"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
Expand Down Expand Up @@ -39,10 +40,10 @@ func (c *openflowManager) getExGwBridgePortConfigurations() ([]bridgeUDNConfigur
return c.externalGatewayBridge.getBridgePortConfigurations()
}

func (c *openflowManager) addNetwork(nInfo util.NetInfo, masqCTMark uint, v4MasqIP, v6MasqIP *net.IPNet) {
c.defaultBridge.addNetworkBridgeConfig(nInfo, masqCTMark, v4MasqIP, v6MasqIP)
func (c *openflowManager) addNetwork(nInfo util.NetInfo, masqCTMark uint, v4MasqIPs, v6MasqIPs *udn.MasqueradeIPs) {
c.defaultBridge.addNetworkBridgeConfig(nInfo, masqCTMark, v4MasqIPs, v6MasqIPs)
if c.externalGatewayBridge != nil {
c.externalGatewayBridge.addNetworkBridgeConfig(nInfo, masqCTMark, v4MasqIP, v6MasqIP)
c.externalGatewayBridge.addNetworkBridgeConfig(nInfo, masqCTMark, v4MasqIPs, v6MasqIPs)
}
}

Expand Down
Loading

0 comments on commit de54993

Please sign in to comment.