From ebdd1306ad560c878097aaf41af249173fab8417 Mon Sep 17 00:00:00 2001 From: Vipul Singh Date: Tue, 22 Aug 2023 09:52:55 -0700 Subject: [PATCH] fix: Remove NLM_F_EXCL flag from Netlink Delete call (#2150) (#2158) * 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 --- netlink/ip_linux.go | 4 +-- netlink/netlink_test.go | 64 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/netlink/ip_linux.go b/netlink/ip_linux.go index 9da3819ade..5967e09f3d 100644 --- 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 index 2cf04c86f0..d5586f093a 100644 --- 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) + } +}