Ensure OVS port is valid before setting NO_FLOOD

pkg/agent/agent.go

@@ -379,17 +379,20 @@ func (i *Initializer) initInterfaceStore() error {
 }
 
 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)
+			if intf.OFPort < 0 {
+				klog.InfoS("Skipped setting no-flood for port due to invalid ofPort", "port", intf.InterfaceName, "ofport", intf.OFPort)
+				continue
+			}
+			if err := i.ovsCtlClient.SetPortNoFlood(int(intf.OFPort)); err != nil {
+				return fmt.Errorf("failed to set no-flood for port %s: %w", intf.InterfaceName, err)
 			}
-			klog.InfoS("Set port no-flood successfully", "PortName", intf.InterfaceName)
+			klog.InfoS("Set no-flood for port", "port", intf.InterfaceName)
 		}
 	}
 	return nil

pkg/agent/agent_linux.go

@@ -348,8 +348,7 @@ func (i *Initializer) prepareL7NetworkPolicyInterfaces() error {
 			return err
 		}
 
-		itf := interfacestore.NewTrafficControlInterface(portName)
-		itf.OVSPortConfig = &interfacestore.OVSPortConfig{PortUUID: portUUID, OFPort: ofPort}
+		itf := interfacestore.NewTrafficControlInterface(portName, &interfacestore.OVSPortConfig{PortUUID: portUUID, OFPort: ofPort})
 		i.ifaceStore.AddInterface(itf)
 	}
 

pkg/agent/agent_test.go

@@ -37,6 +37,7 @@ import (
 	"antrea.io/antrea/pkg/agent/types"
 	"antrea.io/antrea/pkg/ovs/ovsconfig"
 	ovsconfigtest "antrea.io/antrea/pkg/ovs/ovsconfig/testing"
+	ovsctltest "antrea.io/antrea/pkg/ovs/ovsctl/testing"
 	"antrea.io/antrea/pkg/util/env"
 	"antrea.io/antrea/pkg/util/ip"
 	"antrea.io/antrea/pkg/util/runtime"
@@ -647,3 +648,70 @@ func mockConfigureLinkAddress(returnedErr error) func() {
 		configureLinkAddresses = originalConfigureLinkAddresses
 	}
 }
+
+func TestRestorePortConfigs(t *testing.T) {
+	tests := []struct {
+		name                string
+		existingInterfaces  []*interfacestore.InterfaceConfig
+		expectedOVSCtlCalls func(client *ovsctltest.MockOVSCtlClientMockRecorder)
+		expectedErr         string
+	}{
+		{
+			name: "success",
+			existingInterfaces: []*interfacestore.InterfaceConfig{
+				interfacestore.NewIPSecTunnelInterface("antrea-ipsec1",
+					ovsconfig.GeneveTunnel,
+					"node1",
+					net.ParseIP("1.1.1.1"),
+					"abcdefg",
+					"node1",
+					&interfacestore.OVSPortConfig{OFPort: 11, PortUUID: "uuid1"}),
+				interfacestore.NewTunnelInterface(defaultTunInterfaceName,
+					ovsconfig.GeneveTunnel,
+					0,
+					net.ParseIP("1.1.1.10"),
+					true,
+					&interfacestore.OVSPortConfig{OFPort: 12}),
+				interfacestore.NewTrafficControlInterface("antrea-tap1",
+					&interfacestore.OVSPortConfig{OFPort: 13, PortUUID: "uuid3"}),
+				interfacestore.NewTrafficControlInterface("antrea-tap2",
+					&interfacestore.OVSPortConfig{OFPort: -1, PortUUID: "uuid3"}),
+			},
+			expectedOVSCtlCalls: func(client *ovsctltest.MockOVSCtlClientMockRecorder) {
+				client.SetPortNoFlood(11).Return(nil)
+				client.SetPortNoFlood(13).Return(nil)
+			},
+		},
+		{
+			name: "fail",
+			existingInterfaces: []*interfacestore.InterfaceConfig{
+				interfacestore.NewTrafficControlInterface("antrea-tap1",
+					&interfacestore.OVSPortConfig{OFPort: 10, PortUUID: "uuid3"}),
+			},
+			expectedOVSCtlCalls: func(client *ovsctltest.MockOVSCtlClientMockRecorder) {
+				client.SetPortNoFlood(10).Return(fmt.Errorf("server unavailable"))
+			},
+			expectedErr: "failed to set no-flood for port antrea-tap1: server unavailable",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			controller := mock.NewController(t)
+			defer controller.Finish()
+			mockOVSCtlClient := ovsctltest.NewMockOVSCtlClient(controller)
+			ifaceStore := interfacestore.NewInterfaceStore()
+			initializer := &Initializer{
+				ifaceStore:   ifaceStore,
+				ovsCtlClient: mockOVSCtlClient,
+			}
+			ifaceStore.Initialize(tt.existingInterfaces)
+			tt.expectedOVSCtlCalls(mockOVSCtlClient.EXPECT())
+			err := initializer.restorePortConfigs()
+			if tt.expectedErr == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.ErrorContains(t, err, tt.expectedErr)
+			}
+		})
+	}
+}

pkg/agent/controller/trafficcontrol/controller.go

@@ -762,8 +762,7 @@ func (c *Controller) getOrCreateTrafficControlPort(port *v1alpha2.TrafficControl
 			return 0, err
 		}
 	}
-	itf := interfacestore.NewTrafficControlInterface(portName)
-	itf.OVSPortConfig = &interfacestore.OVSPortConfig{PortUUID: portUUID, OFPort: ofPort}
+	itf := interfacestore.NewTrafficControlInterface(portName, &interfacestore.OVSPortConfig{PortUUID: portUUID, OFPort: ofPort})
 	c.interfaceStore.AddInterface(itf)
 	// Create binding for the newly created port.
 	c.portToTCBindings[portName] = &portToTCBinding{

pkg/agent/interfacestore/interface_cache_test.go

@@ -40,7 +40,6 @@ func TestNewInterfaceStore(t *testing.T) {
 	t.Run("testGatewayInterface", testGatewayInterface)
 	t.Run("testTunnelInterface", testTunnelInterface)
 	t.Run("testUplinkInterface", testUplinkInterface)
-	t.Run("testTrafficControlInterface", testTrafficControlInterface)
 	t.Run("testExternalEntityInterface", testEntityInterface)
 }
 
@@ -164,16 +163,6 @@ func testUplinkInterface(t *testing.T) {
 	testGeneralInterface(t, uplinkInterface, UplinkInterface)
 }
 
-func testTrafficControlInterface(t *testing.T) {
-	tcInterface := NewTrafficControlInterface("tc0")
-	tcInterface.IPs = []net.IP{hostIP}
-	tcInterface.OVSPortConfig = &OVSPortConfig{
-		OFPort:   17,
-		PortUUID: "1234567890",
-	}
-	testGeneralInterface(t, tcInterface, TrafficControlInterface)
-}
-
 func testEntityInterface(t *testing.T) {
 	store := NewInterfaceStore()
 	portConfig := &OVSPortConfig{OFPort: 18, PortUUID: "123456789"}

pkg/agent/interfacestore/types.go

@@ -175,8 +175,8 @@ func NewUplinkInterface(uplinkName string) *InterfaceConfig {
 	return uplinkConfig
 }
 
-func NewTrafficControlInterface(interfaceName string) *InterfaceConfig {
-	trafficControlConfig := &InterfaceConfig{InterfaceName: interfaceName, Type: TrafficControlInterface}
+func NewTrafficControlInterface(interfaceName string, ovsPortConfig *OVSPortConfig) *InterfaceConfig {
+	trafficControlConfig := &InterfaceConfig{InterfaceName: interfaceName, Type: TrafficControlInterface, OVSPortConfig: ovsPortConfig}
 	return trafficControlConfig
 }