Skip to content

Commit

Permalink
Set NO_FLOOD to IPsec tunnel ports
Browse files Browse the repository at this point in the history
Set NO_FLOOD to IPsec tunnel ports to avoid ARP flooding.

Signed-off-by: Xu Liu <xliu2@vmware.com>
  • Loading branch information
xliuxu committed Dec 5, 2022
1 parent b977b1d commit 5953341
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 18 deletions.
1 change: 1 addition & 0 deletions cmd/antrea-agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ func run(o *Options) error {
k8sClient,
informerFactory,
ofClient,
ovsctl.NewClient(o.config.OVSBridge),
ovsBridgeClient,
routeClient,
ifaceStore,
Expand Down
14 changes: 9 additions & 5 deletions pkg/agent/controller/noderoute/node_route_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"antrea.io/antrea/pkg/agent/util"
"antrea.io/antrea/pkg/agent/wireguard"
"antrea.io/antrea/pkg/ovs/ovsconfig"
"antrea.io/antrea/pkg/ovs/ovsctl"
utilip "antrea.io/antrea/pkg/util/ip"
"antrea.io/antrea/pkg/util/k8s"
"antrea.io/antrea/pkg/util/runtime"
Expand All @@ -65,6 +66,7 @@ type Controller struct {
kubeClient clientset.Interface
ovsBridgeClient ovsconfig.OVSBridgeClient
ofClient openflow.Client
ovsCtlClient ovsctl.OVSCtlClient
routeClient route.Interface
interfaceStore interfacestore.InterfaceStore
networkConfig *config.NetworkConfig
Expand Down Expand Up @@ -92,6 +94,7 @@ func NewNodeRouteController(
kubeClient clientset.Interface,
informerFactory informers.SharedInformerFactory,
client openflow.Client,
ovsCtlClient ovsctl.OVSCtlClient,
ovsBridgeClient ovsconfig.OVSBridgeClient,
routeClient route.Interface,
interfaceStore interfacestore.InterfaceStore,
Expand All @@ -107,6 +110,7 @@ func NewNodeRouteController(
kubeClient: kubeClient,
ovsBridgeClient: ovsBridgeClient,
ofClient: client,
ovsCtlClient: ovsCtlClient,
routeClient: routeClient,
interfaceStore: interfaceStore,
networkConfig: networkConfig,
Expand Down Expand Up @@ -671,11 +675,6 @@ func (c *Controller) createIPSecTunnelPort(nodeName string, nodeIP net.IP) (int3
}
c.interfaceStore.DeleteInterface(interfaceConfig)
exists = false
} else {
if interfaceConfig.OFPort != 0 {
klog.V(2).InfoS("Found cached IPsec tunnel interface", "node", nodeName, "interface", interfaceConfig.InterfaceName, "port", interfaceConfig.OFPort)
return interfaceConfig.OFPort, nil
}
}
}
if !exists {
Expand Down Expand Up @@ -715,6 +714,11 @@ 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)
}

interfaceConfig.OFPort = ofPort
return ofPort, nil
}
Expand Down
13 changes: 12 additions & 1 deletion pkg/agent/controller/noderoute/node_route_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"antrea.io/antrea/pkg/agent/util"
"antrea.io/antrea/pkg/ovs/ovsconfig"
ovsconfigtest "antrea.io/antrea/pkg/ovs/ovsconfig/testing"
ovsctltest "antrea.io/antrea/pkg/ovs/ovsctl/testing"
utilip "antrea.io/antrea/pkg/util/ip"
)

Expand All @@ -58,6 +59,7 @@ type fakeController struct {
ovsClient *ovsconfigtest.MockOVSBridgeClient
routeClient *routetest.MockInterface
interfaceStore interfacestore.InterfaceStore
ovsCtlClient *ovsctltest.MockOVSCtlClient
}

type fakeIPsecCertificateManager struct{}
Expand All @@ -75,7 +77,9 @@ func newController(t *testing.T, networkConfig *config.NetworkConfig) (*fakeCont
routeClient := routetest.NewMockInterface(ctrl)
interfaceStore := interfacestore.NewInterfaceStore()
ipsecCertificateManager := &fakeIPsecCertificateManager{}
c := NewNodeRouteController(clientset, informerFactory, ofClient, ovsClient, routeClient, interfaceStore, networkConfig, &config.NodeConfig{GatewayConfig: &config.GatewayConfig{
ovsCtlClient := ovsctltest.NewMockOVSCtlClient(ctrl)

c := NewNodeRouteController(clientset, informerFactory, ofClient, ovsCtlClient, ovsClient, routeClient, interfaceStore, networkConfig, &config.NodeConfig{GatewayConfig: &config.GatewayConfig{
IPv4: nil,
MAC: gatewayMAC,
}}, nil, false, ipsecCertificateManager)
Expand All @@ -86,6 +90,7 @@ func newController(t *testing.T, networkConfig *config.NetworkConfig) (*fakeCont
ofClient: ofClient,
ovsClient: ovsClient,
routeClient: routeClient,
ovsCtlClient: ovsCtlClient,
interfaceStore: interfaceStore,
}, ctrl.Finish
}
Expand Down Expand Up @@ -339,6 +344,7 @@ func TestCreateIPSecTunnelPortPSK(t *testing.T) {

node1PortName := util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-1")
node2PortName := util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-2")
node3PortName := util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-3")
c.ovsClient.EXPECT().CreateTunnelPortExt(
node1PortName, ovsconfig.TunnelType("vxlan"), int32(0),
false, "", nodeIP1.String(), "", "changeme", nil,
Expand All @@ -348,7 +354,11 @@ func TestCreateIPSecTunnelPortPSK(t *testing.T) {
false, "", nodeIP2.String(), "", "changeme", nil,
map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-2"}).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)
c.ovsCtlClient.EXPECT().SetPortNoFlood(2)
c.ovsClient.EXPECT().GetOFPort(node3PortName, false).Return(int32(5), nil)
c.ovsCtlClient.EXPECT().SetPortNoFlood(5)
c.ovsClient.EXPECT().DeletePort("123").Times(1)

tests := []struct {
Expand Down Expand Up @@ -407,6 +417,7 @@ func TestCreateIPSecTunnelPortCert(t *testing.T) {
false, "", nodeIP1.String(), "xyz-k8s-0-1", "", nil,
map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-1"}).Times(1)
c.ovsClient.EXPECT().GetOFPort(node1PortName, false).Return(int32(1), nil)
c.ovsCtlClient.EXPECT().SetPortNoFlood(1)

tests := []struct {
name string
Expand Down
47 changes: 35 additions & 12 deletions test/e2e/ipsec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"
"regexp"
"strconv"
"strings"
"testing"

"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -74,47 +75,57 @@ func TestIPSec(t *testing.T) {
t.Run("testIPSecDeleteStaleTunnelPorts", func(t *testing.T) { testIPSecDeleteStaleTunnelPorts(t, data) })
}

func (data *TestData) readSecurityAssociationsStatus(nodeName string) (up int, connecting int, isCertAuth bool, err error) {
func (data *TestData) readOVSIPsecTunnelStatus(nodeName string) (string, error) {
antreaPodName, err := data.getAntreaPodOnNode(nodeName)
if err != nil {
return 0, 0, false, err
return "", err
}
cmd := strings.Split("ovs-appctl -t ovs-monitor-ipsec tunnels/show", " ")
stdout, stderr, err := data.RunCommandFromPod(antreaNamespace, antreaPodName, "antrea-ipsec", cmd)
return fmt.Sprintf("stdout:\n %sstderr:\n %s", stdout, stderr), err
}

func (data *TestData) readSecurityAssociationsStatus(nodeName string) (up int, connecting int, isCertAuth bool, stdout string, err error) {
antreaPodName, err := data.getAntreaPodOnNode(nodeName)
if err != nil {
return 0, 0, false, "", err
}
cmd := []string{"ipsec", "statusall"}
stdout, stderr, err := data.RunCommandFromPod(antreaNamespace, antreaPodName, "antrea-ipsec", cmd)
if err != nil {
return 0, 0, false, fmt.Errorf("error when running 'ipsec status' on '%s': %v - stdout: %s - stderr: %s", nodeName, err, stdout, stderr)
return 0, 0, false, "", fmt.Errorf("error when running 'ipsec status' on '%s': %v - stdout: %s - stderr: %s", nodeName, err, stdout, stderr)
}
re := regexp.MustCompile(`Security Associations \((\d+) up, (\d+) connecting\)`)
matches := re.FindStringSubmatch(stdout)
if len(matches) == 0 {
return 0, 0, false, fmt.Errorf("unexpected 'ipsec statusall' output: %s", stdout)
return 0, 0, false, "", fmt.Errorf("unexpected 'ipsec statusall' output: %s", stdout)
}
v, err := strconv.ParseUint(matches[1], 10, 32)
if err != nil {
return 0, 0, false, fmt.Errorf("error when retrieving 'up' SAs from 'ipsec statusall' output: %v", err)
return 0, 0, false, "", fmt.Errorf("error when retrieving 'up' SAs from 'ipsec statusall' output: %v", err)
}
up = int(v)
v, err = strconv.ParseUint(matches[2], 10, 32)
if err != nil {
return 0, 0, false, fmt.Errorf("error when retrieving 'connecting' SAs from 'ipsec statusall' output: %v", err)
return 0, 0, false, "", fmt.Errorf("error when retrieving 'connecting' SAs from 'ipsec statusall' output: %v", err)
}
connecting = int(v)

re = regexp.MustCompile(`uses ([a-z-]+) key authentication`)
match := re.FindStringSubmatch(stdout)
if len(match) == 0 {
return 0, 0, false, fmt.Errorf("failed to determine authentication method from 'ipsec statusall' output: %s", stdout)
return 0, 0, false, "", fmt.Errorf("failed to determine authentication method from 'ipsec statusall' output: %s", stdout)
}

if match[1] == "pre-shared" {
isCertAuth = false
} else if match[1] == "public" {
isCertAuth = true
} else {
return 0, 0, false, fmt.Errorf("unknown key authentication mode %q", match[1])
return 0, 0, false, "", fmt.Errorf("unknown key authentication mode %q", match[1])
}

return up, connecting, isCertAuth, nil
return up, connecting, isCertAuth, stdout, nil
}

// testIPSecTunnelConnectivity checks that Pod traffic across two Nodes over
Expand All @@ -129,17 +140,29 @@ func testIPSecTunnelConnectivity(t *testing.T, data *TestData, certAuth bool) {
}
podInfos, deletePods := createPodsOnDifferentNodes(t, data, data.testNamespace, tag)
defer deletePods()

debugTunnelStatus := func(msg string) {
tunnelStatus, err := data.readOVSIPsecTunnelStatus(nodeName(0))
if err != nil {
t.Errorf("Failed to read IPsec tunnel status: %s. Status:%s", err, tunnelStatus)
} else {
t.Logf("%s:\n%s", msg, tunnelStatus)
}
}
debugTunnelStatus("IPsec tunnel status before ping mesh")
defer debugTunnelStatus("IPsec tunnel status after ping mesh")

data.runPingMesh(t, podInfos[:2], agnhostContainerName)

// We know that testPodConnectivityDifferentNodes always creates a Pod on Node 0 for the
// inter-Node ping test.
nodeName := nodeName(0)
if up, _, isCertAuth, err := data.readSecurityAssociationsStatus(nodeName); err != nil {
if up, _, isCertAuth, stdout, err := data.readSecurityAssociationsStatus(nodeName); err != nil {
t.Errorf("Error when reading Security Associations: %v", err)
} else if up == 0 {
t.Errorf("Expected at least one 'up' Security Association, but got %d", up)
t.Errorf("Expected at least one 'up' Security Association, but got %d. ipsec status output:\n %s", up, stdout)
} else if isCertAuth != certAuth {
t.Errorf("Expected certificate authentication to be %t, got %t", certAuth, isCertAuth)
t.Errorf("Expected certificate authentication to be %t, got %t. ipsec status output:\n %s", certAuth, isCertAuth, stdout)
} else {
t.Logf("Found %d 'up' SecurityAssociation(s) for Node '%s', certificate auth: %t", up, nodeName, isCertAuth)
}
Expand Down

0 comments on commit 5953341

Please sign in to comment.