From eb53b64a0b860281322999a297e485d969ceb81b Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Wed, 8 Jan 2025 11:22:37 -0800 Subject: [PATCH] Ensure correct sysctl values if VLAN iface exists in IPAssigner (#6900) If the VLAN interface already exixts (e.g., in case of an Agent restart), we should still ensure that the necessary systcl variables are set correctly. Signed-off-by: Antonin Bas --- pkg/agent/ipassigner/ip_assigner_linux.go | 19 +++++++-------- .../agent/ip_assigner_linux_test.go | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/pkg/agent/ipassigner/ip_assigner_linux.go b/pkg/agent/ipassigner/ip_assigner_linux.go index 6850cbe38f4..d09dbfcdef4 100644 --- a/pkg/agent/ipassigner/ip_assigner_linux.go +++ b/pkg/agent/ipassigner/ip_assigner_linux.go @@ -508,6 +508,15 @@ func (a *ipAssigner) getAssignee(subnetInfo *crdv1b1.SubnetInfo, createIfNotExis return nil, fmt.Errorf("error creating VLAN sub-interface for VLAN %d", subnetInfo.VLAN) } } + as, err := a.addVLANAssignee(vlan, subnetInfo.VLAN) + if err != nil { + return nil, err + } + return as, nil +} + +func (a *ipAssigner) addVLANAssignee(link netlink.Link, vlan int32) (*assignee, error) { + name := link.Attrs().Name // Loose mode is needed because incoming traffic received on the interface is expected to be received on the parent // external interface when looking up the main table. To make it look up the custom table, we will need to restore // the mark on the reply traffic and turn on src_valid_mark on this interface, which is more complicated. @@ -521,18 +530,10 @@ func (a *ipAssigner) getAssignee(subnetInfo *crdv1b1.SubnetInfo, createIfNotExis if err := util.EnsurePromoteSecondariesOnInterface(name); err != nil { return nil, err } - as, err := a.addVLANAssignee(vlan, subnetInfo.VLAN) - if err != nil { - return nil, err - } - return as, nil -} - -func (a *ipAssigner) addVLANAssignee(link netlink.Link, vlan int32) (*assignee, error) { if err := netlink.LinkSetUp(link); err != nil { return nil, fmt.Errorf("error setting up interface %v", link) } - iface, err := net.InterfaceByName(link.Attrs().Name) + iface, err := net.InterfaceByName(name) if err != nil { return nil, err } diff --git a/test/integration/agent/ip_assigner_linux_test.go b/test/integration/agent/ip_assigner_linux_test.go index 803cd4acfe5..39f4a1920b8 100644 --- a/test/integration/agent/ip_assigner_linux_test.go +++ b/test/integration/agent/ip_assigner_linux_test.go @@ -25,11 +25,29 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "antrea.io/antrea/pkg/agent/ipassigner" + "antrea.io/antrea/pkg/agent/util/sysctl" crdv1b1 "antrea.io/antrea/pkg/apis/crd/v1beta1" ) const dummyDeviceName = "antrea-dummy0" +func checkSysctl(t *testing.T, path string, expected int) { + t.Helper() + v, err := sysctl.GetSysctlNet(path) + require.NoError(t, err) + assert.Equalf(t, expected, v, "Wrong value for %s", path) +} + +func checkRPFilterOnInterface(t *testing.T, ifaceName string, expected int) { + t.Helper() + checkSysctl(t, fmt.Sprintf("ipv4/conf/%s/rp_filter", ifaceName), expected) +} + +func checkPromoteSecondariesOnInterface(t *testing.T, ifaceName string, expected int) { + t.Helper() + checkSysctl(t, fmt.Sprintf("ipv4/conf/%s/promote_secondaries", ifaceName), expected) +} + func TestIPAssigner(t *testing.T) { nodeLinkName := nodeIntf.Name require.NotNil(t, nodeLinkName, "Get Node link failed") @@ -40,6 +58,7 @@ func TestIPAssigner(t *testing.T) { dummyDevice, err := netlink.LinkByName(dummyDeviceName) require.NoError(t, err, "Failed to find the dummy device") defer netlink.LinkDel(dummyDevice) + checkPromoteSecondariesOnInterface(t, dummyDeviceName, 1) _, err = ipAssigner.AssignIP("x", nil, false) assert.Error(t, err, "Assigning an invalid IP should fail") @@ -103,9 +122,13 @@ func TestIPAssigner(t *testing.T) { vlan20Device, err := netlink.LinkByName("antrea-ext.20") require.NoError(t, err, "Failed to find the VLAN 20 device") defer netlink.LinkDel(vlan20Device) + checkRPFilterOnInterface(t, "antrea-ext.20", 2) + checkPromoteSecondariesOnInterface(t, "antrea-ext.20", 1) vlan30Device, err := netlink.LinkByName("antrea-ext.30") require.NoError(t, err, "Failed to find the VLAN 30 device") defer netlink.LinkDel(vlan30Device) + checkRPFilterOnInterface(t, "antrea-ext.30", 2) + checkPromoteSecondariesOnInterface(t, "antrea-ext.30", 1) actualIPs, err := listIPAddresses(dummyDevice) require.NoError(t, err, "Failed to list IP addresses")