From a82b522c9e8713da3d779d07266f02983041c89e Mon Sep 17 00:00:00 2001 From: Xu Liu Date: Wed, 22 Feb 2023 18:12:45 +0800 Subject: [PATCH] Restore NO_FLOOD to OVS ports after reconnecting the OVS bridge The NO_FLOOD configuration is lost when the OVS daemon is restarted. Currently, the only way to recover this configuration is by restarting the agent. This pull request adds logic to recover the configuration when receiving OVS reconnection events. Signed-off-by: Xu Liu --- pkg/agent/agent.go | 41 ++++++++++++++++--- .../noderoute/node_route_controller.go | 9 +++- .../noderoute/node_route_controller_test.go | 18 +++++--- pkg/agent/interfacestore/interface_cache.go | 13 +++++- .../interfacestore/interface_cache_test.go | 10 +++-- .../testing/mock_interfacestore.go | 14 +++++++ pkg/agent/interfacestore/types.go | 6 ++- 7 files changed, 90 insertions(+), 21 deletions(-) diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index d23059cdc8c..3ef5eec4b58 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -279,7 +279,6 @@ func (i *Initializer) initInterfaceStore() error { return intf } ifaceList := make([]*interfacestore.InterfaceConfig, 0, len(ovsPorts)) - ovsCtlClient := ovsctl.NewClient(i.ovsBridge) for index := range ovsPorts { port := &ovsPorts[index] ovsPort := &interfacestore.OVSPortConfig{ @@ -297,6 +296,8 @@ func (i *Initializer) initInterfaceStore() error { case interfacestore.AntreaUplink: intf = parseUplinkInterfaceFunc(port, ovsPort) case interfacestore.AntreaTunnel: + fallthrough + case interfacestore.AntreaIPsecTunnel: intf = parseTunnelInterfaceFunc(port, ovsPort) case interfacestore.AntreaHost: if port.Name == i.ovsBridge { @@ -314,9 +315,6 @@ func (i *Initializer) initInterfaceStore() error { intf = cniserver.ParseOVSPortInterfaceConfig(port, ovsPort, true) case interfacestore.AntreaTrafficControl: intf = trafficcontrol.ParseTrafficControlInterfaceConfig(port, ovsPort) - if err := ovsCtlClient.SetPortNoFlood(int(ovsPort.OFPort)); err != nil { - klog.ErrorS(err, "Failed to set port with no-flood config", "PortName", port.Name) - } default: klog.InfoS("Unknown Antrea interface type", "type", interfaceType) } @@ -340,7 +338,11 @@ func (i *Initializer) initInterfaceStore() error { fallthrough case port.IFType == ovsconfig.STTTunnel: intf = parseTunnelInterfaceFunc(port, ovsPort) - antreaIFType = interfacestore.AntreaTunnel + if intf.Type == interfacestore.IPSecTunnelInterface { + antreaIFType = interfacestore.AntreaIPsecTunnel + } else { + antreaIFType = interfacestore.AntreaTunnel + } case port.Name == i.ovsBridge: intf = nil antreaIFType = interfacestore.AntreaHost @@ -368,6 +370,23 @@ func (i *Initializer) initInterfaceStore() error { return nil } +func (i *Initializer) restorePortConfigs() error { + ovsCtlClient := ovsctl.NewClient(i.ovsBridge) + interfaces := i.ifaceStore.ListInterfaces() + for _, intf := range interfaces { + switch intf.Type { + case interfacestore.IPSecTunnelInterface: + fallthrough + case interfacestore.TrafficControlInterface: + if err := ovsCtlClient.SetPortNoFlood(int(intf.OFPort)); err != nil { + return fmt.Errorf("failed to set port %s with no-flood: %w", intf.InterfaceName, err) + } + klog.InfoS("Set port no-flood successfully", "PortName", intf.InterfaceName) + } + } + return nil +} + // Initialize sets up agent initial configurations. func (i *Initializer) Initialize() error { klog.Info("Setting up node network") @@ -386,6 +405,10 @@ func (i *Initializer) Initialize() error { return err } + if err := i.restorePortConfigs(); err != nil { + return err + } + // initializeWireGuard must be executed after setupOVSBridge as it requires gateway addresses on the OVS bridge. if i.networkConfig.TrafficEncryptionMode == config.TrafficEncryptionModeWireGuard { if err := i.initializeWireGuard(); err != nil { @@ -553,11 +576,17 @@ func (i *Initializer) initOpenFlowPipeline() error { i.ofClient.ReplayFlows() klog.Info("Flow replay completed") + klog.InfoS("Restoring OF port configs to OVS bridge") + if err := i.restorePortConfigs(); err != nil { + klog.ErrorS(err, "Failed to restore OF port configs") + } else { + klog.InfoS("Port configs restoration completed") + } // ofClient and ovsBridgeClient have their own mechanisms to restore connections with OVS, and it could // happen that ovsBridgeClient's connection is not ready when ofClient completes flow replay. We retry it // with a timeout that is longer time than ovsBridgeClient's maximum connecting retry interval (8 seconds) // to ensure the flag can be removed successfully. - err := wait.PollImmediate(200*time.Millisecond, 10*time.Second, func() (done bool, err error) { + err = wait.PollImmediate(200*time.Millisecond, 10*time.Second, func() (done bool, err error) { if err := i.FlowRestoreComplete(); err != nil { return false, nil } diff --git a/pkg/agent/controller/noderoute/node_route_controller.go b/pkg/agent/controller/noderoute/node_route_controller.go index dd2a6939b23..0bfcd801bcd 100644 --- a/pkg/agent/controller/noderoute/node_route_controller.go +++ b/pkg/agent/controller/noderoute/node_route_controller.go @@ -227,7 +227,7 @@ func (c *Controller) removeStaleTunnelPorts() error { // will not include it in the set. desiredInterfaces := make(map[string]bool) // knownInterfaces is the list of interfaces currently in the local cache. - knownInterfaces := c.interfaceStore.GetInterfaceKeysByType(interfacestore.TunnelInterface) + knownInterfaces := c.interfaceStore.GetInterfaceKeysByType(interfacestore.IPSecTunnelInterface) if c.networkConfig.TrafficEncryptionMode == config.TrafficEncryptionModeIPSec { for _, node := range nodes { @@ -656,8 +656,12 @@ func (c *Controller) createIPSecTunnelPort(nodeName string, nodeIP net.IP) (int3 exists = false } } + if !exists { - ovsExternalIDs := map[string]interface{}{ovsExternalIDNodeName: nodeName} + ovsExternalIDs := map[string]interface{}{ + ovsExternalIDNodeName: nodeName, + interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaIPsecTunnel, + } portUUID, err := c.ovsBridgeClient.CreateTunnelPortExt( portName, c.networkConfig.TunnelType, @@ -693,6 +697,7 @@ func (c *Controller) createIPSecTunnelPort(nodeName string, nodeIP net.IP) (int3 // Let NodeRouteController retry at errors. return 0, fmt.Errorf("failed to get of_port of IPsec tunnel port for Node %s", nodeName) } + // Set the port with no-flood to reject ARP flood packets. if err := c.ovsCtlClient.SetPortNoFlood(int(ofPort)); err != nil { return 0, fmt.Errorf("failed to set port %s with no-flood config: %w", portName, err) diff --git a/pkg/agent/controller/noderoute/node_route_controller_test.go b/pkg/agent/controller/noderoute/node_route_controller_test.go index dbfe9a995fa..a324dd43695 100644 --- a/pkg/agent/controller/noderoute/node_route_controller_test.go +++ b/pkg/agent/controller/noderoute/node_route_controller_test.go @@ -259,7 +259,7 @@ func setup(t *testing.T, ifaces []*interfacestore.InterfaceConfig, authenticatio func TestRemoveStaleTunnelPorts(t *testing.T) { c, closeFn := setup(t, []*interfacestore.InterfaceConfig{ { - Type: interfacestore.TunnelInterface, + Type: interfacestore.IPSecTunnelInterface, InterfaceName: util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-1"), TunnelInterfaceConfig: &interfacestore.TunnelInterfaceConfig{ NodeName: "xyz-k8s-0-1", @@ -307,7 +307,7 @@ func TestRemoveStaleTunnelPorts(t *testing.T) { func TestCreateIPSecTunnelPortPSK(t *testing.T) { c, closeFn := setup(t, []*interfacestore.InterfaceConfig{ { - Type: interfacestore.TunnelInterface, + Type: interfacestore.IPSecTunnelInterface, InterfaceName: "mismatchedname", TunnelInterfaceConfig: &interfacestore.TunnelInterfaceConfig{ NodeName: "xyz-k8s-0-2", @@ -320,7 +320,7 @@ func TestCreateIPSecTunnelPortPSK(t *testing.T) { }, }, { - Type: interfacestore.TunnelInterface, + Type: interfacestore.IPSecTunnelInterface, InterfaceName: util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-3"), TunnelInterfaceConfig: &interfacestore.TunnelInterfaceConfig{ NodeName: "xyz-k8s-0-3", @@ -348,11 +348,15 @@ func TestCreateIPSecTunnelPortPSK(t *testing.T) { c.ovsClient.EXPECT().CreateTunnelPortExt( node1PortName, ovsconfig.TunnelType("vxlan"), int32(0), false, "", nodeIP1.String(), "", "changeme", nil, - map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-1"}).Times(1) + map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-1", + interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaIPsecTunnel, + }).Times(1) c.ovsClient.EXPECT().CreateTunnelPortExt( node2PortName, ovsconfig.TunnelType("vxlan"), int32(0), false, "", nodeIP2.String(), "", "changeme", nil, - map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-2"}).Times(1) + map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-2", + interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaIPsecTunnel, + }).Times(1) c.ovsClient.EXPECT().GetOFPort(node1PortName, false).Return(int32(1), nil) c.ovsCtlClient.EXPECT().SetPortNoFlood(1) c.ovsClient.EXPECT().GetOFPort(node2PortName, false).Return(int32(2), nil) @@ -415,7 +419,9 @@ func TestCreateIPSecTunnelPortCert(t *testing.T) { c.ovsClient.EXPECT().CreateTunnelPortExt( node1PortName, ovsconfig.TunnelType("vxlan"), int32(0), false, "", nodeIP1.String(), "xyz-k8s-0-1", "", nil, - map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-1"}).Times(1) + map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-1", + interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaIPsecTunnel, + }).Times(1) c.ovsClient.EXPECT().GetOFPort(node1PortName, false).Return(int32(1), nil) c.ovsCtlClient.EXPECT().SetPortNoFlood(1) diff --git a/pkg/agent/interfacestore/interface_cache.go b/pkg/agent/interfacestore/interface_cache.go index 1d35378ae1c..479593452ab 100644 --- a/pkg/agent/interfacestore/interface_cache.go +++ b/pkg/agent/interfacestore/interface_cache.go @@ -86,8 +86,8 @@ func getInterfaceKey(obj interface{}) (string, error) { var key string if interfaceConfig.Type == ContainerInterface { key = util.GenerateContainerInterfaceKey(interfaceConfig.ContainerID) - } else if interfaceConfig.Type == TunnelInterface && interfaceConfig.NodeName != "" { - // Tunnel interface for a Node. + } else if interfaceConfig.Type == IPSecTunnelInterface { + // IPsec tunnel interface for a Node. key = util.GenerateNodeTunnelInterfaceKey(interfaceConfig.NodeName) } else { // Use the interface name as the key by default. @@ -123,6 +123,15 @@ func (c *interfaceCache) GetInterface(interfaceKey string) (*InterfaceConfig, bo return iface.(*InterfaceConfig), found } +// ListInterfacesByType lists all interfaces from local cache. +func (c *interfaceCache) ListInterfaces() []*InterfaceConfig { + interfaceConfigs := make([]*InterfaceConfig, 0) + for _, iface := range c.cache.List() { + interfaceConfigs = append(interfaceConfigs, iface.(*InterfaceConfig)) + } + return interfaceConfigs +} + // GetInterfaceByName retrieves interface from local cache given the interface // name. func (c *interfaceCache) GetInterfaceByName(interfaceName string) (*InterfaceConfig, bool) { diff --git a/pkg/agent/interfacestore/interface_cache_test.go b/pkg/agent/interfacestore/interface_cache_test.go index de41c9a597e..1596959fc72 100644 --- a/pkg/agent/interfacestore/interface_cache_test.go +++ b/pkg/agent/interfacestore/interface_cache_test.go @@ -140,14 +140,16 @@ func testTunnelInterface(t *testing.T) { assert.False(t, exists) ifaceNames := store.GetInterfaceKeysByType(TunnelInterface) - assert.Equal(t, 2, len(ifaceNames)) + assert.Equal(t, 1, len(ifaceNames)) + ipsecIfaceNames := store.GetInterfaceKeysByType(IPSecTunnelInterface) + assert.Equal(t, 1, len(ipsecIfaceNames)) store.DeleteInterface(ipsecTunnelInterface) - assert.Equal(t, 1, len(store.GetInterfaceKeysByType(TunnelInterface))) + assert.Equal(t, 0, len(store.GetInterfaceKeysByType(IPSecTunnelInterface))) _, exists = store.GetInterfaceByName(ipsecTunnelInterface.InterfaceName) assert.False(t, exists) store.AddInterface(ipsecTunnelInterface) - ifaceNames = store.GetInterfaceKeysByType(TunnelInterface) - assert.Equal(t, 2, len(ifaceNames)) + ifaceNames = store.GetInterfaceKeysByType(IPSecTunnelInterface) + assert.Equal(t, 1, len(ifaceNames)) _, exists = store.GetInterfaceByName(ipsecTunnelInterface.InterfaceName) assert.True(t, exists) } diff --git a/pkg/agent/interfacestore/testing/mock_interfacestore.go b/pkg/agent/interfacestore/testing/mock_interfacestore.go index 3e8aa50fbc5..70519cf5256 100644 --- a/pkg/agent/interfacestore/testing/mock_interfacestore.go +++ b/pkg/agent/interfacestore/testing/mock_interfacestore.go @@ -257,3 +257,17 @@ func (mr *MockInterfaceStoreMockRecorder) Len() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Len", reflect.TypeOf((*MockInterfaceStore)(nil).Len)) } + +// ListInterfaces mocks base method +func (m *MockInterfaceStore) ListInterfaces() []*interfacestore.InterfaceConfig { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListInterfaces") + ret0, _ := ret[0].([]*interfacestore.InterfaceConfig) + return ret0 +} + +// ListInterfaces indicates an expected call of ListInterfaces +func (mr *MockInterfaceStoreMockRecorder) ListInterfaces() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListInterfaces", reflect.TypeOf((*MockInterfaceStore)(nil).ListInterfaces)) +} diff --git a/pkg/agent/interfacestore/types.go b/pkg/agent/interfacestore/types.go index b0e03a31b35..0387cf1426d 100644 --- a/pkg/agent/interfacestore/types.go +++ b/pkg/agent/interfacestore/types.go @@ -35,6 +35,8 @@ const ( TrafficControlInterface // ExternalEntityInterface is used to mark current interface is for ExternalEntity Endpoint ExternalEntityInterface + // IPSecTunnelInterface is used to mark current interface is for IPSec tunnel port + IPSecTunnelInterface AntreaInterfaceTypeKey = "antrea-type" AntreaGateway = "gateway" @@ -43,6 +45,7 @@ const ( AntreaUplink = "uplink" AntreaHost = "host" AntreaTrafficControl = "traffic-control" + AntreaIPsecTunnel = "ipsec-tunnel" AntreaUnset = "" ) @@ -108,6 +111,7 @@ type InterfaceConfig struct { type InterfaceStore interface { Initialize(interfaces []*InterfaceConfig) AddInterface(interfaceConfig *InterfaceConfig) + ListInterfaces() []*InterfaceConfig DeleteInterface(interfaceConfig *InterfaceConfig) GetInterface(interfaceKey string) (*InterfaceConfig, bool) GetInterfaceByName(interfaceName string) (*InterfaceConfig, bool) @@ -162,7 +166,7 @@ func NewTunnelInterface(tunnelName string, tunnelType ovsconfig.TunnelType, dest // Node. func NewIPSecTunnelInterface(interfaceName string, tunnelType ovsconfig.TunnelType, nodeName string, nodeIP net.IP, psk, remoteName string, ovsPortConfig *OVSPortConfig) *InterfaceConfig { tunnelConfig := &TunnelInterfaceConfig{Type: tunnelType, NodeName: nodeName, RemoteIP: nodeIP, PSK: psk, RemoteName: remoteName} - return &InterfaceConfig{InterfaceName: interfaceName, Type: TunnelInterface, TunnelInterfaceConfig: tunnelConfig, OVSPortConfig: ovsPortConfig} + return &InterfaceConfig{InterfaceName: interfaceName, Type: IPSecTunnelInterface, TunnelInterfaceConfig: tunnelConfig, OVSPortConfig: ovsPortConfig} } // NewUplinkInterface creates InterfaceConfig for the uplink interface.