Skip to content

Commit

Permalink
Ensure that promote_secondaries is set on IPAssigner interfaces (#6898)
Browse files Browse the repository at this point in the history
The IPAssigner should ensure that the promote_secondaries sysctl
variable is always set when creating an interface. This ensures that
When the primary IP address is removed from the interface, a secondary
IP address is promoted, instead of removing all the corresponding
secondary IP addresses. Otherwise, the deletion of one IP address can
trigger the automatic removal of all other IP addresses in the same
subnet, if the deleted IP happens to be the primary (first one assigned
chronologically). For example, this can affect the Egress feature (when
EgressSeparateSubnet is used).

Fixes #6890

Signed-off-by: Antonin Bas <[email protected]>
  • Loading branch information
antoninbas authored Jan 6, 2025
1 parent c010958 commit 1ca6f6c
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 6 deletions.
17 changes: 15 additions & 2 deletions pkg/agent/ipassigner/ip_assigner_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ func getARPIgnoreForInterface(iface string) (int, error) {
return arpIgnore, nil
}
return arpIgnoreAll, nil

}

// ensureDummyDevice creates the dummy device if it doesn't exist.
Expand All @@ -306,7 +305,14 @@ func ensureDummyDevice(deviceName string) (netlink.Link, error) {
dummy := &netlink.Dummy{
LinkAttrs: netlink.LinkAttrs{Name: deviceName},
}
if err = netlink.LinkAdd(dummy); err != nil {
if err := netlink.LinkAdd(dummy); err != nil {
return nil, err
}
// When the primary IP address is removed from the interface, promote a corresponding secondary IP address
// instead of removing all the corresponding secondary IP addresses. Otherwise, the deletion of one IP address
// can trigger the automatic removal of all other IP addresses in the same subnet, if the deleted IP happens to
// be the primary (first one assigned chronologically).
if err := util.EnsurePromoteSecondariesOnInterface(deviceName); err != nil {
return nil, err
}
return dummy, nil
Expand Down Expand Up @@ -503,6 +509,13 @@ func (a *ipAssigner) getAssignee(subnetInfo *crdv1b1.SubnetInfo, createIfNotExis
if err := util.EnsureRPFilterOnInterface(name, 2); err != nil {
return nil, err
}
// When the primary IP address is removed from the interface, promote a corresponding secondary IP address
// instead of removing all the corresponding secondary IP addresses. Otherwise, the deletion of one IP address
// can trigger the automatic removal of all other IP addresses in the same subnet, if the deleted IP happens to
// be the primary (first one assigned chronologically).
if err := util.EnsurePromoteSecondariesOnInterface(name); err != nil {
return nil, err
}
as, err := a.addVLANAssignee(vlan, subnetInfo.VLAN)
if err != nil {
return nil, err
Expand Down
5 changes: 5 additions & 0 deletions pkg/agent/util/net_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,11 @@ func EnsureRPFilterOnInterface(ifaceName string, value int) error {
return sysctl.EnsureSysctlNetValue(path, value)
}

func EnsurePromoteSecondariesOnInterface(ifaceName string) error {
path := fmt.Sprintf("ipv4/conf/%s/promote_secondaries", ifaceName)
return sysctl.EnsureSysctlNetValue(path, 1)
}

func getRoutesOnInterface(linkIndex int) ([]interface{}, error) {
link, err := netlinkUtil.LinkByIndex(linkIndex)
if err != nil {
Expand Down
49 changes: 45 additions & 4 deletions test/integration/agent/ip_assigner_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,43 @@ func TestIPAssigner(t *testing.T) {
ip1VLAN30 := "10.10.30.10"
subnet20 := &crdv1b1.SubnetInfo{PrefixLength: 24, VLAN: 20}
subnet30 := &crdv1b1.SubnetInfo{PrefixLength: 24, VLAN: 30}
desiredIPs := map[string]*crdv1b1.SubnetInfo{ip1: nil, ip2: nil, ip3: nil, ip1VLAN20: subnet20, ip2VLAN20: subnet20, ip1VLAN30: subnet30}
// These IPs will be assigned to the correct interface, in the specified order.
ipsToAssign := []struct {
ip string
subnetInfo *crdv1b1.SubnetInfo
}{
{
ip: ip1,
},
{
ip: ip2,
},
{
ip: ip3,
},
// ip1VLAN20 and ip2VLAN20 are in the same subnet and will be assigned to the same
// interface (antrea-ext.20).
// ip1VLAN20 will be assigned first, which means ip1VLAN20 will be the "primary" IP,
// while ip2VLAN20 will be the "secondary" IP.
{
ip: ip1VLAN20,
subnetInfo: subnet20,
},
{
ip: ip2VLAN20,
subnetInfo: subnet20,
},
{
ip: ip1VLAN30,
subnetInfo: subnet30,
},
}

desiredIPs := make(map[string]*crdv1b1.SubnetInfo)
for _, assignment := range ipsToAssign {
ip, subnetInfo := assignment.ip, assignment.subnetInfo
desiredIPs[ip] = subnetInfo

for ip, subnetInfo := range desiredIPs {
_, errAssign := ipAssigner.AssignIP(ip, subnetInfo, false)
cmd := exec.Command("ip", "addr")
out, err := cmd.CombinedOutput()
Expand Down Expand Up @@ -88,7 +122,14 @@ func TestIPAssigner(t *testing.T) {
assert.Equal(t, map[string]*crdv1b1.SubnetInfo{}, newIPAssigner.AssignedIPs(), "Assigned IPs don't match")

ip4 := "2021:124:6020:1006:250:56ff:fea7:36c4"
newDesiredIPs := map[string]*crdv1b1.SubnetInfo{ip1: nil, ip2: nil, ip4: nil, ip1VLAN20: subnet20}
// ip1VLAN20 is omitted, so it will be removed from the antrea-ext.20 interface. Because it
// is the primary IP address, secondary IPs (in this case ip2VLAN20) in the same subnet will
// be automatically removed when the primary is removed, unless the promote_secondaries
// sysctl variable has been set to 1 on the interface, which should be the case.
// By removing ip1VLAN20 (primary), we can therefore validate that IPAssigner is setting
// promote_secondaries correctly on the interface, as otherwise ip2VLAN20 will be removed
// automatically.
newDesiredIPs := map[string]*crdv1b1.SubnetInfo{ip1: nil, ip2: nil, ip4: nil, ip2VLAN20: subnet20}
err = newIPAssigner.InitIPs(newDesiredIPs)
require.NoError(t, err, "InitIPs failed")
assert.Equal(t, newDesiredIPs, newIPAssigner.AssignedIPs(), "Assigned IPs don't match")
Expand All @@ -98,7 +139,7 @@ func TestIPAssigner(t *testing.T) {
assert.Equal(t, sets.New[string](fmt.Sprintf("%s/32", ip1), fmt.Sprintf("%s/32", ip2), fmt.Sprintf("%s/128", ip4)), actualIPs, "Actual IPs don't match")
actualIPs, err = listIPAddresses(vlan20Device)
require.NoError(t, err, "Failed to list IP addresses")
assert.Equal(t, sets.New[string](fmt.Sprintf("%s/%d", ip1VLAN20, subnet20.PrefixLength)), actualIPs, "Actual IPs don't match")
assert.Equal(t, sets.New[string](fmt.Sprintf("%s/%d", ip2VLAN20, subnet20.PrefixLength)), actualIPs, "Actual IPs don't match")
_, err = netlink.LinkByName("antrea-ext.30")
require.Error(t, err, "VLAN 30 device should be deleted but was not")

Expand Down

0 comments on commit 1ca6f6c

Please sign in to comment.