Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

UDN: Ensure pod2service isolation in local gw mode. #4705

Merged
merged 1 commit into from
Sep 20, 2024
Merged

UDN: Ensure pod2service isolation in local gw mode. #4705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 59 additions & 14 deletions go-controller/pkg/node/gateway_shared_intf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,9 @@ func (npw *nodePortWatcher) updateServiceFlowCache(service *kapi.Service, netInf

// Add flows for default network services that are accessible from UDN networks
if util.IsNetworkSegmentationSupportEnabled() {
// The flow added below has a higher priority than the default network service flow:
// priority=500,ip,in_port=LOCAL,nw_dst=10.96.0.0/16 actions=ct(commit,table=2,zone=64001,nat(src=169.254.0.2))
// This ordering ensures that there is no SNAT for UDN originated traffic.
// The flow added below has a higher priority than the per UDN service flow:
// priority=200, table=2, ip, ip_src=169.254.0.<UDN>, actions=set_field:<bridge-mac>->eth_dst,output:<UDN-patch-port>
// This ordering ensures that traffic to UDN allowed default services goes to the the default patch port.

if util.IsUDNEnabledService(ktypes.NamespacedName{Namespace: service.Namespace, Name: service.Name}.String()) {
key = strings.Join([]string{"UDNAllowedSVC", service.Namespace, service.Name}, "_")
Expand All @@ -295,10 +295,13 @@ func (npw *nodePortWatcher) updateServiceFlowCache(service *kapi.Service, netInf
ipPrefix = "ipv6"
masqueradeSubnet = config.Gateway.V6MasqueradeSubnet
}
// table 0, user-defined network host -> OVN towards default cluster network services
npw.ofm.updateFlowCacheEntry(key, []string{fmt.Sprintf("cookie=%s, priority=600, in_port=%s, %s, %s_src=%s, %s_dst=%s,"+
"actions=ct(commit,zone=%d,table=2)",
defaultOpenFlowCookie, npw.ofm.defaultBridge.ofPortHost, ipPrefix, ipPrefix, masqueradeSubnet, ipPrefix, service.Spec.ClusterIP, config.Default.HostMasqConntrackZone)})
// table 2, user-defined network host -> OVN towards default cluster network services
defaultNetConfig := npw.ofm.defaultBridge.netConfig[types.DefaultNetworkName]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity: What was wrong with the previous way of accessing the ofport: npw.ofm.defaultBridge.ofPortHost ? If this is duplicate data stored in the netconfig should we remove the ofPortHost field from the bridgeConfiguration?

Copy link
Contributor Author

@dceara dceara Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm nothing? But this flow is changing: instead of matching of inport=ofPortHost in table 0 it now is matching on ip_src=masq_subnet with action=set_mac; output to ofPortUDNPatch in table 2.

The job of the flow I'm changing is now done by the new one I'm adding in table 0 (for the whole svc CIDR, not only for select services like kapi) here:
https://github.com/ovn-org/ovn-kubernetes/pull/4705/files#diff-d3aa58d9b58a0a09264f072df46ab01d0501eb508c4656411ae2dc1ac68fb3c4R1329

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this helps explain it better. On top of the change @kyrtapz added to allow access to the kapi, I do:

a. In table 0: I add a priority 550 flow matching on ip_src=masqSubnet that skips SNAT (covers all UDNs).
b. In table 2: I add a flow per UDN to send traffic to the patch port-x if src-IP=UDN-masq-IP-X, priority 200
c. I moved the flow from table 0 that used to skip SNAT for packets going to kapi to table 2 (it's now covered by "a"), priority 300 and change it to match on ip_src=masqSubnet && ip_dst == kapi and change it to send to the default network patch port (higher priority than "b").


npw.ofm.updateFlowCacheEntry(key, []string{fmt.Sprintf("cookie=%s, priority=300, table=2, %s, %s_src=%s, %s_dst=%s, "+
"actions=set_field:%s->eth_dst,output:%s",
defaultOpenFlowCookie, ipPrefix, ipPrefix, masqueradeSubnet, ipPrefix, service.Spec.ClusterIP,
npw.ofm.getDefaultBridgeMAC().String(), defaultNetConfig.ofPortPatch)})
}
}
return utilerrors.Join(errors...)
Expand Down Expand Up @@ -1316,11 +1319,23 @@ func flowsForDefaultBridge(bridge *bridgeConfiguration, extraIPs []net.IP) ([]st
masqSubnet = config.Gateway.V6MasqueradeSubnet
}

// table 0, Host -> OVN towards SVC, SNAT to special IP
// table 0, Host (default network) -> OVN towards SVC, SNAT to special IP.
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, priority=500, in_port=%s, %s, %s_dst=%s,"+
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))
defaultOpenFlowCookie, ofPortHost, protoPrefix, protoPrefix,
svcCIDR, config.Default.HostMasqConntrackZone, masqIP))

if util.IsNetworkSegmentationSupportEnabled() {
// table 0, Host (UDNs) -> OVN towards SVC, SNAT to special IP.
// For packets originating from UDN, commit without NATing, those
// have already been SNATed to the masq IP of the UDN.
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,
masqSubnet, protoPrefix, svcCIDR, config.Default.HostMasqConntrackZone))
}

masqDst := masqIP
if util.IsNetworkSegmentationSupportEnabled() {
Expand Down Expand Up @@ -1420,8 +1435,38 @@ func flowsForDefaultBridge(bridge *bridgeConfiguration, extraIPs []net.IP) ([]st

// table 2, dispatch from Host -> OVN
dftFlows = append(dftFlows,
fmt.Sprintf("cookie=%s, table=2, "+
"actions=set_field:%s->eth_dst,output:%s", defaultOpenFlowCookie, bridgeMacAddress, defaultNetConfig.ofPortPatch))
fmt.Sprintf("cookie=%s, priority=100, table=2, "+
"actions=set_field:%s->eth_dst,output:%s", defaultOpenFlowCookie,
bridgeMacAddress, defaultNetConfig.ofPortPatch))

// table 2, priority 200, dispatch from UDN -> Host -> OVN. These packets have
// already been SNATed to the UDN's masq IP.
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,
Expand Down Expand Up @@ -1522,7 +1567,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 @@ -1597,7 +1642,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
View file
Open in desktop
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
Loading
Loading