Skip to content

Commit 5a5e3b6

Browse files
npinaevanpinaeva
committed
[bridgeconfig] move flow generation locking into methods.
The locking logic is slightly changed, because now bridge is only locked during flow generation and not for the whole openflow_manager update duration. Also only one bridge is now locked at a time. Signed-off-by: Nadia Pinaeva <[email protected]>
1 parent 28f9c1e commit 5a5e3b6

File tree

2 files changed

+65
-57
lines changed

2 files changed

+65
-57
lines changed

‎go-controller/pkg/node/bridgeconfig/bridgeflows.go‎

Lines changed: 59 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,35 @@ import (
1414
"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
1515
)
1616

17-
func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]string, error) {
17+
func (b *BridgeConfiguration) DefaultBridgeFlows(hostSubnets []*net.IPNet, extraIPs []net.IP) ([]string, error) {
18+
b.Mutex.Lock()
19+
defer b.Mutex.Unlock()
20+
dftFlows, err := b.flowsForDefaultBridge(extraIPs)
21+
if err != nil {
22+
return nil, err
23+
}
24+
dftCommonFlows, err := b.commonFlows(hostSubnets)
25+
if err != nil {
26+
return nil, err
27+
}
28+
return append(dftFlows, dftCommonFlows...), nil
29+
}
30+
31+
func (b *BridgeConfiguration) ExternalBridgeFlows(hostSubnets []*net.IPNet) ([]string, error) {
32+
b.Mutex.Lock()
33+
defer b.Mutex.Unlock()
34+
return b.commonFlows(hostSubnets)
35+
}
36+
37+
// must be called with bridge.mutex held
38+
func (b *BridgeConfiguration) flowsForDefaultBridge(extraIPs []net.IP) ([]string, error) {
1839
// CAUTION: when adding new flows where the in_port is ofPortPatch and the out_port is ofPortPhys, ensure
1940
// that dl_src is included in match criteria!
2041

21-
ofPortPhys := bridge.OfPortPhys
22-
bridgeMacAddress := bridge.MacAddress.String()
23-
ofPortHost := bridge.OfPortHost
24-
bridgeIPs := bridge.Ips
42+
ofPortPhys := b.OfPortPhys
43+
bridgeMacAddress := b.MacAddress.String()
44+
ofPortHost := b.OfPortHost
45+
bridgeIPs := b.Ips
2546

2647
var dftFlows []string
2748
// 14 bytes of overhead for ethernet header (does not include VLAN)
@@ -58,7 +79,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
5879
if err != nil {
5980
return nil, fmt.Errorf("unable to determine IPv4 physical IP of host: %v", err)
6081
}
61-
for _, netConfig := range bridge.PatchedNetConfigs() {
82+
for _, netConfig := range b.PatchedNetConfigs() {
6283
// table 0, SVC Hairpin from OVN destined to local host, DNAT and go to table 4
6384
dftFlows = append(dftFlows,
6485
fmt.Sprintf("cookie=%s, priority=500, in_port=%s, ip, ip_dst=%s, ip_src=%s,"+
@@ -82,7 +103,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
82103
continue
83104
}
84105

85-
for _, netConfig := range bridge.PatchedNetConfigs() {
106+
for _, netConfig := range b.PatchedNetConfigs() {
86107
dftFlows = append(dftFlows,
87108
fmt.Sprintf("cookie=%s, priority=500, in_port=%s, ip, ip_dst=%s, ip_src=%s,"+
88109
"actions=ct(commit,zone=%d,table=4)",
@@ -121,7 +142,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
121142
return nil, fmt.Errorf("unable to determine IPv6 physical IP of host: %v", err)
122143
}
123144
// table 0, SVC Hairpin from OVN destined to local host, DNAT to host, send to table 4
124-
for _, netConfig := range bridge.PatchedNetConfigs() {
145+
for _, netConfig := range b.PatchedNetConfigs() {
125146
dftFlows = append(dftFlows,
126147
fmt.Sprintf("cookie=%s, priority=500, in_port=%s, ipv6, ipv6_dst=%s, ipv6_src=%s,"+
127148
"actions=ct(commit,zone=%d,nat(dst=%s),table=4)",
@@ -144,7 +165,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
144165
continue
145166
}
146167

147-
for _, netConfig := range bridge.PatchedNetConfigs() {
168+
for _, netConfig := range b.PatchedNetConfigs() {
148169
dftFlows = append(dftFlows,
149170
fmt.Sprintf("cookie=%s, priority=500, in_port=%s, ipv6, ipv6_dst=%s, ipv6_src=%s,"+
150171
"actions=ct(commit,zone=%d,table=4)",
@@ -195,7 +216,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
195216
// we match on the UDNPodSubnet itself and we also don't SNAT to 169.254.0.2
196217
// sample flow: cookie=0xdeff105, duration=1472.742s, table=0, n_packets=9, n_bytes=666, priority=550
197218
// ip,in_port=LOCAL,nw_src=103.103.0.0/16,nw_dst=10.96.0.0/16 actions=ct(commit,table=2,zone=64001)
198-
for _, netConfig := range bridge.PatchedNetConfigs() {
219+
for _, netConfig := range b.PatchedNetConfigs() {
199220
if netConfig.IsDefaultNetwork() {
200221
continue
201222
}
@@ -228,7 +249,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
228249
// In UDN match on the whole masquerade subnet to handle replies from UDN enabled services
229250
masqDst = masqSubnet
230251
}
231-
for _, netConfig := range bridge.PatchedNetConfigs() {
252+
for _, netConfig := range b.PatchedNetConfigs() {
232253
// table 0, Reply hairpin traffic to host, coming from OVN, unSNAT
233254
dftFlows = append(dftFlows,
234255
fmt.Sprintf("cookie=%s, priority=500, in_port=%s, %s, %s_src=%s, %s_dst=%s,"+
@@ -251,7 +272,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
251272
dftFlows = append(dftFlows, reassemblyFlows...)
252273
}
253274
if ofPortPhys != "" {
254-
for _, netConfig := range bridge.PatchedNetConfigs() {
275+
for _, netConfig := range b.PatchedNetConfigs() {
255276
var actions string
256277
if config.Gateway.Mode != config.GatewayModeLocal || config.Gateway.DisablePacketMTUCheck {
257278
actions = fmt.Sprintf("output:%s", netConfig.OfPortPatch)
@@ -319,7 +340,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
319340
nodetypes.DefaultOpenFlowCookie, match_vlan, bridgeMacAddress, strip_vlan, ofPortHost))
320341
}
321342

322-
defaultNetConfig := bridge.NetConfig[types.DefaultNetworkName]
343+
defaultNetConfig := b.NetConfig[types.DefaultNetworkName]
323344

324345
// table 2, dispatch from Host -> OVN
325346
dftFlows = append(dftFlows,
@@ -330,7 +351,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
330351
// table 2, priority 200, dispatch from UDN -> Host -> OVN. These packets have
331352
// already been SNATed to the UDN's masq IP or have been marked with the UDN's packet mark.
332353
if config.IPv4Mode {
333-
for _, netConfig := range bridge.PatchedNetConfigs() {
354+
for _, netConfig := range b.PatchedNetConfigs() {
334355
if netConfig.IsDefaultNetwork() {
335356
continue
336357
}
@@ -368,7 +389,7 @@ func FlowsForDefaultBridge(bridge *BridgeConfiguration, extraIPs []net.IP) ([]st
368389
}
369390

370391
if config.IPv6Mode {
371-
for _, netConfig := range bridge.PatchedNetConfigs() {
392+
for _, netConfig := range b.PatchedNetConfigs() {
372393
if netConfig.IsDefaultNetwork() {
373394
continue
374395
}
@@ -472,13 +493,14 @@ func generateIPFragmentReassemblyFlow(ofPortPhys string) []string {
472493
return flows
473494
}
474495

475-
func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]string, error) {
496+
// must be called with bridge.mutex held
497+
func (b *BridgeConfiguration) commonFlows(hostSubnets []*net.IPNet) ([]string, error) {
476498
// CAUTION: when adding new flows where the in_port is ofPortPatch and the out_port is ofPortPhys, ensure
477499
// that dl_src is included in match criteria!
478-
ofPortPhys := bridge.OfPortPhys
479-
bridgeMacAddress := bridge.MacAddress.String()
480-
ofPortHost := bridge.OfPortHost
481-
bridgeIPs := bridge.Ips
500+
ofPortPhys := b.OfPortPhys
501+
bridgeMacAddress := b.MacAddress.String()
502+
ofPortHost := b.OfPortHost
503+
bridgeIPs := b.Ips
482504

483505
var dftFlows []string
484506

@@ -494,7 +516,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
494516
if ofPortPhys != "" {
495517
// table 0, we check to see if this dest mac is the shared mac, if so flood to all ports
496518
actions := ""
497-
for _, netConfig := range bridge.PatchedNetConfigs() {
519+
for _, netConfig := range b.PatchedNetConfigs() {
498520
actions += "output:" + netConfig.OfPortPatch + ","
499521
}
500522

@@ -506,7 +528,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
506528

507529
// table 0, check packets coming from OVN have the correct mac address. Low priority flows that are a catch all
508530
// for non-IP packets that would normally be forwarded with NORMAL action (table 0, priority 0 flow).
509-
for _, netConfig := range bridge.PatchedNetConfigs() {
531+
for _, netConfig := range b.PatchedNetConfigs() {
510532
dftFlows = append(dftFlows,
511533
fmt.Sprintf("cookie=%s, priority=10, table=0, in_port=%s, dl_src=%s, actions=output:NORMAL",
512534
nodetypes.DefaultOpenFlowCookie, netConfig.OfPortPatch, bridgeMacAddress))
@@ -521,7 +543,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
521543
return nil, fmt.Errorf("unable to determine IPv4 physical IP of host: %v", err)
522544
}
523545
if ofPortPhys != "" {
524-
for _, netConfig := range bridge.PatchedNetConfigs() {
546+
for _, netConfig := range b.PatchedNetConfigs() {
525547
// table0, packets coming from egressIP pods that have mark 1008 on them
526548
// will be SNAT-ed a final time into nodeIP to maintain consistency in traffic even if the GR
527549
// SNATs these into egressIP prior to reaching external bridge.
@@ -536,9 +558,9 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
536558
// table 0, packets coming from egressIP pods only from user defined networks. If an egressIP is assigned to
537559
// this node, then all networks get a flow even if no pods on that network were selected for by this egressIP.
538560
if util.IsNetworkSegmentationSupportEnabled() && config.OVNKubernetesFeature.EnableInterconnect &&
539-
config.Gateway.Mode != config.GatewayModeDisabled && bridge.EipMarkIPs != nil {
561+
config.Gateway.Mode != config.GatewayModeDisabled && b.EipMarkIPs != nil {
540562
if netConfig.MasqCTMark != nodetypes.CtMarkOVN {
541-
for mark, eip := range bridge.EipMarkIPs.GetIPv4() {
563+
for mark, eip := range b.EipMarkIPs.GetIPv4() {
542564
dftFlows = append(dftFlows,
543565
fmt.Sprintf("cookie=%s, priority=105, in_port=%s, dl_src=%s, ip, pkt_mark=%d, "+
544566
"actions=ct(commit, zone=%d, nat(src=%s), exec(set_field:%s->ct_mark)), output:%s",
@@ -580,7 +602,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
580602
nodetypes.DefaultOpenFlowCookie, ofPortHost, config.Default.ConntrackZone, nodetypes.CtMarkHost, mod_vlan_id, ofPortPhys))
581603
}
582604
if config.Gateway.Mode == config.GatewayModeLocal {
583-
for _, netConfig := range bridge.PatchedNetConfigs() {
605+
for _, netConfig := range b.PatchedNetConfigs() {
584606
// table 0, any packet coming from OVN send to host in LGW mode, host will take care of sending it outside if needed.
585607
// exceptions are traffic for egressIP and egressGW features and ICMP related traffic which will hit the priority 100 flow instead of this.
586608
dftFlows = append(dftFlows,
@@ -620,7 +642,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
620642
return nil, fmt.Errorf("unable to determine IPv6 physical IP of host: %v", err)
621643
}
622644
if ofPortPhys != "" {
623-
for _, netConfig := range bridge.PatchedNetConfigs() {
645+
for _, netConfig := range b.PatchedNetConfigs() {
624646
// table0, packets coming from egressIP pods that have mark 1008 on them
625647
// will be DNAT-ed a final time into nodeIP to maintain consistency in traffic even if the GR
626648
// DNATs these into egressIP prior to reaching external bridge.
@@ -635,9 +657,9 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
635657
// table 0, packets coming from egressIP pods only from user defined networks. If an egressIP is assigned to
636658
// this node, then all networks get a flow even if no pods on that network were selected for by this egressIP.
637659
if util.IsNetworkSegmentationSupportEnabled() && config.OVNKubernetesFeature.EnableInterconnect &&
638-
config.Gateway.Mode != config.GatewayModeDisabled && bridge.EipMarkIPs != nil {
660+
config.Gateway.Mode != config.GatewayModeDisabled && b.EipMarkIPs != nil {
639661
if netConfig.MasqCTMark != nodetypes.CtMarkOVN {
640-
for mark, eip := range bridge.EipMarkIPs.GetIPv6() {
662+
for mark, eip := range b.EipMarkIPs.GetIPv6() {
641663
dftFlows = append(dftFlows,
642664
fmt.Sprintf("cookie=%s, priority=105, in_port=%s, dl_src=%s, ipv6, pkt_mark=%d, "+
643665
"actions=ct(commit, zone=%d, nat(src=%s), exec(set_field:%s->ct_mark)), output:%s",
@@ -679,7 +701,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
679701

680702
}
681703
if config.Gateway.Mode == config.GatewayModeLocal {
682-
for _, netConfig := range bridge.PatchedNetConfigs() {
704+
for _, netConfig := range b.PatchedNetConfigs() {
683705
// table 0, any packet coming from OVN send to host in LGW mode, host will take care of sending it outside if needed.
684706
// exceptions are traffic for egressIP and egressGW features and ICMP related traffic which will hit the priority 100 flow instead of this.
685707
dftFlows = append(dftFlows,
@@ -714,7 +736,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
714736
// Due to the fact that ovn-controllers on different nodes apply the changes independently,
715737
// there is a chance that the pod traffic will reach the egress node before it configures the SNAT flows.
716738
// Drop pod traffic that is not SNATed, excluding local pods(required for ICNIv2)
717-
defaultNetConfig := bridge.NetConfig[types.DefaultNetworkName]
739+
defaultNetConfig := b.NetConfig[types.DefaultNetworkName]
718740
if config.OVNKubernetesFeature.EnableEgressIP {
719741
for _, clusterEntry := range config.Default.ClusterSubnets {
720742
cidr := clusterEntry.CIDR
@@ -739,7 +761,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
739761
}
740762

741763
if ofPortPhys != "" {
742-
for _, netConfig := range bridge.PatchedNetConfigs() {
764+
for _, netConfig := range b.PatchedNetConfigs() {
743765
isNetworkAdvertised := netConfig.Advertised.Load()
744766
// disableSNATMultipleGWs only applies to default network
745767
disableSNATMultipleGWs := netConfig.IsDefaultNetwork() && config.Gateway.DisableSNATMultipleGWs
@@ -817,7 +839,7 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
817839
"actions=output:%s", nodetypes.DefaultOpenFlowCookie, ofPortHost))
818840

819841
// Send UDN destined traffic to right patch port
820-
for _, netConfig := range bridge.PatchedNetConfigs() {
842+
for _, netConfig := range b.PatchedNetConfigs() {
821843
if netConfig.MasqCTMark != nodetypes.CtMarkOVN {
822844
dftFlows = append(dftFlows,
823845
fmt.Sprintf("cookie=%s, priority=5, table=11, ct_mark=%s, "+
@@ -838,13 +860,15 @@ func CommonFlows(hostSubnets []*net.IPNet, bridge *BridgeConfiguration) ([]strin
838860
return dftFlows, nil
839861
}
840862

841-
func PmtudDropFlows(bridge *BridgeConfiguration, ipAddrs []string) []string {
863+
func (b *BridgeConfiguration) PMTUDDropFlows(ipAddrs []string) []string {
864+
b.Mutex.Lock()
865+
defer b.Mutex.Unlock()
842866
var flows []string
843867
if config.Gateway.Mode != config.GatewayModeShared {
844868
return nil
845869
}
846870
for _, addr := range ipAddrs {
847-
for _, netConfig := range bridge.PatchedNetConfigs() {
871+
for _, netConfig := range b.PatchedNetConfigs() {
848872
flows = append(flows,
849873
nodeutil.GenerateICMPFragmentationFlow(addr, nodetypes.OutputPortDrop, netConfig.OfPortPatch, nodetypes.PmtudOpenFlowCookie, 700))
850874
}

‎go-controller/pkg/node/openflow_manager.go‎

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -207,52 +207,36 @@ func (c *openflowManager) Run(stopChan <-chan struct{}, doneWg *sync.WaitGroup)
207207
}
208208

209209
func (c *openflowManager) updateBridgePMTUDFlowCache(key string, ipAddrs []string) {
210-
// protect defaultBridge config from being updated by gw.nodeIPManager
211-
c.defaultBridge.Mutex.Lock()
212-
defer c.defaultBridge.Mutex.Unlock()
213-
214-
dftFlows := bridgeconfig.PmtudDropFlows(c.defaultBridge, ipAddrs)
210+
dftFlows := c.defaultBridge.PMTUDDropFlows(ipAddrs)
215211
c.updateFlowCacheEntry(key, dftFlows)
216212
if c.externalGatewayBridge != nil {
217-
c.externalGatewayBridge.Mutex.Lock()
218-
defer c.externalGatewayBridge.Mutex.Unlock()
219-
exGWBridgeDftFlows := bridgeconfig.PmtudDropFlows(c.externalGatewayBridge, ipAddrs)
213+
exGWBridgeDftFlows := c.externalGatewayBridge.PMTUDDropFlows(ipAddrs)
220214
c.updateExBridgeFlowCacheEntry(key, exGWBridgeDftFlows)
221215
}
222216
}
223217

224218
// updateBridgeFlowCache generates the "static" per-bridge flows
225219
// note: this is shared between shared and local gateway modes
226220
func (c *openflowManager) updateBridgeFlowCache(hostIPs []net.IP, hostSubnets []*net.IPNet) error {
227-
// protect defaultBridge config from being updated by gw.nodeIPManager
228-
c.defaultBridge.Mutex.Lock()
229-
defer c.defaultBridge.Mutex.Unlock()
230-
231221
// CAUTION: when adding new flows where the in_port is ofPortPatch and the out_port is ofPortPhys, ensure
232222
// that dl_src is included in match criteria!
233223

234-
dftFlows, err := bridgeconfig.FlowsForDefaultBridge(c.defaultBridge, hostIPs)
224+
dftFlows, err := c.defaultBridge.DefaultBridgeFlows(hostSubnets, hostIPs)
235225
if err != nil {
236226
return err
237227
}
238-
dftCommonFlows, err := bridgeconfig.CommonFlows(hostSubnets, c.defaultBridge)
239-
if err != nil {
240-
return err
241-
}
242-
dftFlows = append(dftFlows, dftCommonFlows...)
243228

244229
c.updateFlowCacheEntry("NORMAL", []string{fmt.Sprintf("table=0,priority=0,actions=%s\n", util.NormalAction)})
245230
c.updateFlowCacheEntry("DEFAULT", dftFlows)
246231

247232
// we consume ex gw bridge flows only if that is enabled
248233
if c.externalGatewayBridge != nil {
249-
c.externalGatewayBridge.Mutex.Lock()
250-
defer c.externalGatewayBridge.Mutex.Unlock()
251-
c.updateExBridgeFlowCacheEntry("NORMAL", []string{fmt.Sprintf("table=0,priority=0,actions=%s\n", util.NormalAction)})
252-
exGWBridgeDftFlows, err := bridgeconfig.CommonFlows(hostSubnets, c.externalGatewayBridge)
234+
exGWBridgeDftFlows, err := c.externalGatewayBridge.ExternalBridgeFlows(hostSubnets)
253235
if err != nil {
254236
return err
255237
}
238+
239+
c.updateExBridgeFlowCacheEntry("NORMAL", []string{fmt.Sprintf("table=0,priority=0,actions=%s\n", util.NormalAction)})
256240
c.updateExBridgeFlowCacheEntry("DEFAULT", exGWBridgeDftFlows)
257241
}
258242
return nil

0 commit comments

Comments
 (0)