fix(bridge): avoid LinkByName netlink calls

Main reason is perfoamnce. If we have the relevant info in the cache/DB, we could just use it instead of netlink get calls. Fixes opiproject#123 Signed-off-by: Boris Glimcher <[email protected]>
glimchb · Sep 13, 2023 · 09bf2fb · 09bf2fb
1 parent c1f0da9
commit 09bf2fb
Show file tree

Hide file tree

Showing 6 changed files with 3 additions and 78 deletions.
diff --git a/pkg/evpn/bridge.go b/pkg/evpn/bridge.go
@@ -67,12 +67,6 @@ func (s *Server) CreateLogicalBridge(_ context.Context, in *pb.CreateLogicalBrid
 	}
 	// create vxlan only if VNI is not empty
 	if in.LogicalBridge.Spec.Vni != nil {
-		bridge, err := s.nLink.LinkByName(tenantbridgeName)
-		if err != nil {
-			err := status.Errorf(codes.NotFound, "unable to find key %s", tenantbridgeName)
-			log.Printf("error: %v", err)
-			return nil, err
-		}
 		// Example: ip link add vxlan-<LB-vlan-id> type vxlan id <LB-vni> local <vtep-ip> dstport 4789 nolearning proxy
 		myip := make(net.IP, 4)
 		binary.BigEndian.PutUint32(myip, in.LogicalBridge.Spec.VtepIpPrefix.Addr.GetV4Addr())
@@ -85,6 +79,7 @@ func (s *Server) CreateLogicalBridge(_ context.Context, in *pb.CreateLogicalBrid
 			return nil, err
 		}
 		// Example: ip link set vxlan-<LB-vlan-id> master br-tenant addrgenmode none
+		bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
 		if err := s.nLink.LinkSetMaster(vxlan, bridge); err != nil {
 			fmt.Printf("Failed to add Vxlan to bridge: %v", err)
 			return nil, err

diff --git a/pkg/evpn/bridge_test.go b/pkg/evpn/bridge_test.go
@@ -139,17 +139,6 @@ func Test_CreateLogicalBridge(t *testing.T) {
 			errMsg:  "",
 			exist:   true,
 		},
-		"failed LinkByName call": {
-			id:      testLogicalBridgeID,
-			in:      &testLogicalBridge,
-			out:     nil,
-			errCode: codes.NotFound,
-			errMsg:  fmt.Sprintf("unable to find key %v", tenantbridgeName),
-			exist:   false,
-			on: func(mockNetlink *mocks.Netlink, errMsg string) {
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(nil, errors.New(errMsg)).Once()
-			},
-		},
 		"failed LinkAdd call": {
 			id:      testLogicalBridgeID,
 			in:      &testLogicalBridge,
@@ -163,8 +152,6 @@ func Test_CreateLogicalBridge(t *testing.T) {
 				binary.BigEndian.PutUint32(myip, 167772162)
 				vxlanName := fmt.Sprintf("vni%d", *testLogicalBridge.Spec.Vni)
 				vxlan := &netlink.Vxlan{LinkAttrs: netlink.LinkAttrs{Name: vxlanName}, VxlanId: int(*testLogicalBridge.Spec.Vni), Port: 4789, Learning: false, SrcAddr: myip}
-				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkAdd(vxlan).Return(errors.New(errMsg)).Once()
 			},
 		},
@@ -181,7 +168,6 @@ func Test_CreateLogicalBridge(t *testing.T) {
 				vxlanName := fmt.Sprintf("vni%d", *testLogicalBridge.Spec.Vni)
 				vxlan := &netlink.Vxlan{LinkAttrs: netlink.LinkAttrs{Name: vxlanName}, VxlanId: int(*testLogicalBridge.Spec.Vni), Port: 4789, Learning: false, SrcAddr: myip}
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkAdd(vxlan).Return(nil).Once()
 				mockNetlink.EXPECT().LinkSetMaster(vxlan, bridge).Return(errors.New(errMsg)).Once()
 			},
@@ -199,7 +185,6 @@ func Test_CreateLogicalBridge(t *testing.T) {
 				vxlanName := fmt.Sprintf("vni%d", *testLogicalBridge.Spec.Vni)
 				vxlan := &netlink.Vxlan{LinkAttrs: netlink.LinkAttrs{Name: vxlanName}, VxlanId: int(*testLogicalBridge.Spec.Vni), Port: 4789, Learning: false, SrcAddr: myip}
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkAdd(vxlan).Return(nil).Once()
 				mockNetlink.EXPECT().LinkSetMaster(vxlan, bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkSetUp(vxlan).Return(errors.New(errMsg)).Once()
@@ -218,7 +203,6 @@ func Test_CreateLogicalBridge(t *testing.T) {
 				vxlanName := fmt.Sprintf("vni%d", *testLogicalBridge.Spec.Vni)
 				vxlan := &netlink.Vxlan{LinkAttrs: netlink.LinkAttrs{Name: vxlanName}, VxlanId: int(*testLogicalBridge.Spec.Vni), Port: 4789, Learning: false, SrcAddr: myip}
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkAdd(vxlan).Return(nil).Once()
 				mockNetlink.EXPECT().LinkSetMaster(vxlan, bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkSetUp(vxlan).Return(nil).Once()
@@ -239,7 +223,6 @@ func Test_CreateLogicalBridge(t *testing.T) {
 				vxlanName := fmt.Sprintf("vni%d", *testLogicalBridge.Spec.Vni)
 				vxlan := &netlink.Vxlan{LinkAttrs: netlink.LinkAttrs{Name: vxlanName}, VxlanId: int(*testLogicalBridge.Spec.Vni), Port: 4789, Learning: false, SrcAddr: myip}
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkAdd(vxlan).Return(nil).Once()
 				mockNetlink.EXPECT().LinkSetMaster(vxlan, bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkSetUp(vxlan).Return(nil).Once()

diff --git a/pkg/evpn/svi.go b/pkg/evpn/svi.go
@@ -85,12 +85,7 @@ func (s *Server) CreateSvi(_ context.Context, in *pb.CreateSviRequest) (*pb.Svi,
 		return nil, err
 	}
 	// not found, so create a new one
-	bridge, err := s.nLink.LinkByName(tenantbridgeName)
-	if err != nil {
-		err := status.Errorf(codes.NotFound, "unable to find key %s", tenantbridgeName)
-		log.Printf("error: %v", err)
-		return nil, err
-	}
+	bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
 	vid := uint16(bridgeObject.Spec.VlanId)
 	// Example: bridge vlan add dev br-tenant vid <vlan-id> self
 	if err := s.nLink.BridgeVlanAdd(bridge, vid, false, false, true, false); err != nil {

diff --git a/pkg/evpn/svi_test.go b/pkg/evpn/svi_test.go
@@ -204,17 +204,6 @@ func Test_CreateSvi(t *testing.T) {
 			exist:   false,
 			on:      nil,
 		},
-		"failed bridge LinkByName call": {
-			id:      testSviID,
-			in:      &testSvi,
-			out:     nil,
-			errCode: codes.NotFound,
-			errMsg:  fmt.Sprintf("unable to find key %v", tenantbridgeName),
-			exist:   false,
-			on: func(mockNetlink *mocks.Netlink, errMsg string) {
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(nil, errors.New(errMsg)).Once()
-			},
-		},
 		"failed BridgeVlanAdd call": {
 			id:      testSviID,
 			in:      &testSvi,
@@ -225,7 +214,6 @@ func Test_CreateSvi(t *testing.T) {
 			on: func(mockNetlink *mocks.Netlink, errMsg string) {
 				vid := uint16(testLogicalBridge.Spec.VlanId)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().BridgeVlanAdd(bridge, vid, false, false, true, false).Return(errors.New(errMsg)).Once()
 			},
 		},
@@ -239,7 +227,6 @@ func Test_CreateSvi(t *testing.T) {
 			on: func(mockNetlink *mocks.Netlink, errMsg string) {
 				vid := uint16(testLogicalBridge.Spec.VlanId)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().BridgeVlanAdd(bridge, vid, false, false, true, false).Return(nil).Once()
 				vlanName := fmt.Sprintf("vlan%d", vid)
 				vlandev := &netlink.Vlan{LinkAttrs: netlink.LinkAttrs{Name: vlanName, ParentIndex: bridge.Attrs().Index}, VlanId: int(vid)}
@@ -256,7 +243,6 @@ func Test_CreateSvi(t *testing.T) {
 			on: func(mockNetlink *mocks.Netlink, errMsg string) {
 				vid := uint16(testLogicalBridge.Spec.VlanId)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().BridgeVlanAdd(bridge, vid, false, false, true, false).Return(nil).Once()
 				vlanName := fmt.Sprintf("vlan%d", vid)
 				vlandev := &netlink.Vlan{LinkAttrs: netlink.LinkAttrs{Name: vlanName, ParentIndex: bridge.Attrs().Index}, VlanId: int(vid)}
@@ -275,7 +261,6 @@ func Test_CreateSvi(t *testing.T) {
 			on: func(mockNetlink *mocks.Netlink, errMsg string) {
 				vid := uint16(testLogicalBridge.Spec.VlanId)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().BridgeVlanAdd(bridge, vid, false, false, true, false).Return(nil).Once()
 				vlanName := fmt.Sprintf("vlan%d", vid)
 				vlandev := &netlink.Vlan{LinkAttrs: netlink.LinkAttrs{Name: vlanName, ParentIndex: bridge.Attrs().Index}, VlanId: int(vid)}
@@ -297,7 +282,6 @@ func Test_CreateSvi(t *testing.T) {
 			on: func(mockNetlink *mocks.Netlink, errMsg string) {
 				vid := uint16(testLogicalBridge.Spec.VlanId)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().BridgeVlanAdd(bridge, vid, false, false, true, false).Return(nil).Once()
 				vlanName := fmt.Sprintf("vlan%d", vid)
 				vlandev := &netlink.Vlan{LinkAttrs: netlink.LinkAttrs{Name: vlanName, ParentIndex: bridge.Attrs().Index}, VlanId: int(vid)}
@@ -320,7 +304,6 @@ func Test_CreateSvi(t *testing.T) {
 			on: func(mockNetlink *mocks.Netlink, errMsg string) {
 				vid := uint16(testLogicalBridge.Spec.VlanId)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().BridgeVlanAdd(bridge, vid, false, false, true, false).Return(nil).Once()
 				vlanName := fmt.Sprintf("vlan%d", vid)
 				vlandev := &netlink.Vlan{LinkAttrs: netlink.LinkAttrs{Name: vlanName, ParentIndex: bridge.Attrs().Index}, VlanId: int(vid)}
@@ -345,7 +328,6 @@ func Test_CreateSvi(t *testing.T) {
 			on: func(mockNetlink *mocks.Netlink, errMsg string) {
 				vid := uint16(testLogicalBridge.Spec.VlanId)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().BridgeVlanAdd(bridge, vid, false, false, true, false).Return(nil).Once()
 				vlanName := fmt.Sprintf("vlan%d", vid)
 				vlandev := &netlink.Vlan{LinkAttrs: netlink.LinkAttrs{Name: vlanName, ParentIndex: bridge.Attrs().Index}, VlanId: int(vid)}
@@ -371,7 +353,6 @@ func Test_CreateSvi(t *testing.T) {
 			on: func(mockNetlink *mocks.Netlink, errMsg string) {
 				vid := uint16(testLogicalBridge.Spec.VlanId)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: tenantbridgeName}}
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().BridgeVlanAdd(bridge, vid, false, false, true, false).Return(nil).Once()
 				vlanName := fmt.Sprintf("vlan%d", vid)
 				vlandev := &netlink.Vlan{LinkAttrs: netlink.LinkAttrs{Name: vlanName, ParentIndex: bridge.Attrs().Index}, VlanId: int(vid)}

diff --git a/pkg/evpn/vrf.go b/pkg/evpn/vrf.go
@@ -200,13 +200,8 @@ func (s *Server) DeleteVrf(_ context.Context, in *pb.DeleteVrfRequest) (*emptypb
 		}
 		// use netlink to find BRIDGE device
 		bridgeName := fmt.Sprintf("br%d", *obj.Spec.Vni)
-		bridgedev, err := s.nLink.LinkByName(bridgeName)
+		bridgedev := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}
 		log.Printf("Deleting BRIDGE %v", bridgedev)
-		if err != nil {
-			err := status.Errorf(codes.NotFound, "unable to find key %s", bridgeName)
-			log.Printf("error: %v", err)
-			return nil, err
-		}
 		// bring link down
 		if err := s.nLink.LinkSetDown(bridgedev); err != nil {
 			fmt.Printf("Failed to up link: %v", err)

diff --git a/pkg/evpn/vrf_test.go b/pkg/evpn/vrf_test.go
@@ -450,24 +450,6 @@ func Test_DeleteVrf(t *testing.T) {
 				mockNetlink.EXPECT().LinkDel(vxlan).Return(errors.New(errMsg)).Once()
 			},
 		},
-		"failed bridge LinkByName call": {
-			in:      testVrfID,
-			out:     &emptypb.Empty{},
-			errCode: codes.NotFound,
-			errMsg:  fmt.Sprintf("unable to find key %v", "br1000"),
-			missing: false,
-			on: func(mockNetlink *mocks.Netlink, errMsg string) {
-				myip := make(net.IP, 4)
-				binary.BigEndian.PutUint32(myip, 167772162)
-				vxlanName := fmt.Sprintf("vni%d", *testVrf.Spec.Vni)
-				vxlan := &netlink.Vxlan{LinkAttrs: netlink.LinkAttrs{Name: vxlanName}, VxlanId: int(*testVrf.Spec.Vni), Port: 4789, Learning: false, SrcAddr: myip}
-				mockNetlink.EXPECT().LinkByName(vxlanName).Return(vxlan, nil).Once()
-				mockNetlink.EXPECT().LinkSetDown(vxlan).Return(nil).Once()
-				mockNetlink.EXPECT().LinkDel(vxlan).Return(nil).Once()
-				bridgeName := fmt.Sprintf("br%d", *testVrf.Spec.Vni)
-				mockNetlink.EXPECT().LinkByName(bridgeName).Return(nil, errors.New(errMsg)).Once()
-			},
-		},
 		"failed bridge LinkSetDown call": {
 			in:      testVrfID,
 			out:     &emptypb.Empty{},
@@ -484,7 +466,6 @@ func Test_DeleteVrf(t *testing.T) {
 				mockNetlink.EXPECT().LinkDel(vxlan).Return(nil).Once()
 				bridgeName := fmt.Sprintf("br%d", *testVrf.Spec.Vni)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}
-				mockNetlink.EXPECT().LinkByName(bridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkSetDown(bridge).Return(errors.New(errMsg)).Once()
 			},
 		},
@@ -504,7 +485,6 @@ func Test_DeleteVrf(t *testing.T) {
 				mockNetlink.EXPECT().LinkDel(vxlan).Return(nil).Once()
 				bridgeName := fmt.Sprintf("br%d", *testVrf.Spec.Vni)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}
-				mockNetlink.EXPECT().LinkByName(bridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkSetDown(bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkDel(bridge).Return(errors.New(errMsg)).Once()
 			},
@@ -525,7 +505,6 @@ func Test_DeleteVrf(t *testing.T) {
 				mockNetlink.EXPECT().LinkDel(vxlan).Return(nil).Once()
 				bridgeName := fmt.Sprintf("br%d", *testVrf.Spec.Vni)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}
-				mockNetlink.EXPECT().LinkByName(bridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkSetDown(bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkDel(bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkByName(testVrfID).Return(nil, errors.New(errMsg)).Once()
@@ -547,7 +526,6 @@ func Test_DeleteVrf(t *testing.T) {
 				mockNetlink.EXPECT().LinkDel(vxlan).Return(nil).Once()
 				bridgeName := fmt.Sprintf("br%d", *testVrf.Spec.Vni)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}
-				mockNetlink.EXPECT().LinkByName(bridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkSetDown(bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkDel(bridge).Return(nil).Once()
 				vrf := &netlink.Vrf{LinkAttrs: netlink.LinkAttrs{Name: testVrfID}, Table: 1001}
@@ -571,7 +549,6 @@ func Test_DeleteVrf(t *testing.T) {
 				mockNetlink.EXPECT().LinkDel(vxlan).Return(nil).Once()
 				bridgeName := fmt.Sprintf("br%d", *testVrf.Spec.Vni)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}
-				mockNetlink.EXPECT().LinkByName(bridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkSetDown(bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkDel(bridge).Return(nil).Once()
 				vrf := &netlink.Vrf{LinkAttrs: netlink.LinkAttrs{Name: testVrfID}, Table: 1001}
@@ -596,7 +573,6 @@ func Test_DeleteVrf(t *testing.T) {
 				mockNetlink.EXPECT().LinkDel(vxlan).Return(nil).Once()
 				bridgeName := fmt.Sprintf("br%d", *testVrf.Spec.Vni)
 				bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}
-				mockNetlink.EXPECT().LinkByName(bridgeName).Return(bridge, nil).Once()
 				mockNetlink.EXPECT().LinkSetDown(bridge).Return(nil).Once()
 				mockNetlink.EXPECT().LinkDel(bridge).Return(nil).Once()
 				vrf := &netlink.Vrf{LinkAttrs: netlink.LinkAttrs{Name: testVrfID}, Table: 1001}