fix: Remove NLM_F_EXCL flag from Netlink Delete call (#2150) (#2157)

* fix: Remove unix.NLM_F_EXCL from Netlink Delete Route api call unix.NLM_F_EXCL is not expected to set in netlink delete route calls. It's no-op in older kernel and didnt return error. From kernel 5.19+, new flag NLM_F_BULK was defined with same value and serves a purpose in delete route call. This changes breaks azure cni and netlink calls fails in 5.19 kernel and onwards. The fix is to remove setting unix.NLM_F_EXCL in netlink delete route request. * fix: Remove unix.NLM_F_EXCL from Netlink Delete Route api call unix.NLM_F_EXCL is not expected to set in netlink delete route calls. It's no-op in older kernel and didnt return error. From kernel 5.19+, new flag NLM_F_BULK was defined with same value and serves a purpose in delete route call. This changes breaks azure cni and netlink calls fails in 5.19 kernel and onwards. The fix is to remove setting unix.NLM_F_EXCL in netlink delete route request. * Add unit tests for netlink add/delete address and add/delete routes Co-authored-by: tamilmani1989 <tamanoha@microsoft.com>
Azure · Aug 18, 2023 · e098d08 · e098d08
1 parent 879b644
commit e098d08
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 2 deletions.
diff --git a/netlink/ip_linux.go b/netlink/ip_linux.go
@@ -43,7 +43,7 @@ func (Netlink) setIPAddress(ifName string, ipAddress net.IP, ipNet *net.IPNet, a
 		flags = unix.NLM_F_CREATE | unix.NLM_F_EXCL | unix.NLM_F_ACK
 	} else {
 		msgType = unix.RTM_DELADDR
-		flags = unix.NLM_F_EXCL | unix.NLM_F_ACK
+		flags = unix.NLM_F_ACK
 	}
 
 	req := newRequest(msgType, flags)
@@ -225,7 +225,7 @@ func setIpRoute(route *Route, add bool) error {
 		flags = unix.NLM_F_CREATE | unix.NLM_F_EXCL | unix.NLM_F_ACK
 	} else {
 		msgType = unix.RTM_DELROUTE
-		flags = unix.NLM_F_EXCL | unix.NLM_F_ACK
+		flags = unix.NLM_F_ACK
 	}
 
 	req := newRequest(msgType, flags)

diff --git a/netlink/netlink_test.go b/netlink/netlink_test.go
@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 )
 
 const (
@@ -311,3 +312,66 @@ func TestAddRemoveStaticArp(t *testing.T) {
 		t.Errorf("DeleteLink failed: %+v", err)
 	}
 }
+
+func TestAddRemoveIPAddress(t *testing.T) {
+	_, err := addDummyInterface(ifName)
+	if err != nil {
+		t.Errorf("addDummyInterface failed: %v", err)
+	}
+
+	ip := net.ParseIP("192.168.0.4")
+	_, ipNet, _ := net.ParseCIDR("192.168.0.4/24")
+	nl := NewNetlink()
+
+	err = nl.setIPAddress(ifName, ip, ipNet, true)
+	if err != nil {
+		t.Errorf("ret val %v", err)
+	}
+
+	err = nl.setIPAddress(ifName, ip, ipNet, false)
+	if err != nil {
+		t.Errorf("ret val %v", err)
+	}
+
+	err = nl.DeleteLink(ifName)
+	if err != nil {
+		t.Errorf("DeleteLink failed: %+v", err)
+	}
+}
+
+func TestAddDeleteRoute(t *testing.T) {
+	_, err := addDummyInterface(ifName)
+	if err != nil {
+		t.Errorf("addDummyInterface failed: %v", err)
+	}
+
+	nl := NewNetlink()
+	err = nl.SetLinkState(ifName, true)
+	if err != nil {
+		t.Errorf("ret val %v", err)
+	}
+	_, dstIPNet, _ := net.ParseCIDR("192.168.0.4/24")
+	netif, _ := net.InterfaceByName(ifName)
+
+	route := Route{
+		Family:    unix.AF_INET,
+		Dst:       dstIPNet,
+		LinkIndex: netif.Index,
+		Scope:     RT_SCOPE_LINK,
+	}
+
+	err = setIpRoute(&route, true)
+	if err != nil {
+		t.Errorf("ret val %v", err)
+	}
+
+	err = setIpRoute(&route, false)
+	if err != nil {
+		t.Errorf("ret val %v", err)
+	}
+
+	err = nl.DeleteLink(ifName)
+	if err != nil {
+		t.Errorf("DeleteLink failed: %+v", err)
+	}
+}