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 #123 Signed-off-by: Boris Glimcher <[email protected]>
opiproject · Aug 31, 2023 · ccfbeca · ccfbeca
1 parent 1a2b0e5
commit ccfbeca
Show file tree

Hide file tree

Showing 5 changed files with 5 additions and 48 deletions.
diff --git a/pkg/evpn/bridge.go b/pkg/evpn/bridge.go
@@ -60,12 +60,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)
 		// TODO: remove hard-coded 167772162 == "10.0.0.2"
@@ -79,6 +73,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
@@ -117,14 +117,6 @@ func Test_CreateLogicalBridge(t *testing.T) {
 			"",
 			true,
 		},
-		"failed LinkByName call": {
-			testLogicalBridgeID,
-			&testLogicalBridge,
-			nil,
-			codes.NotFound,
-			"unable to find key br-tenant",
-			false,
-		},
 		"failed LinkAdd call": {
 			testLogicalBridgeID,
 			&testLogicalBridge,
@@ -189,24 +181,19 @@ func Test_CreateLogicalBridge(t *testing.T) {
 			}
 
 			// TODO: refactor this mocking
-			if strings.Contains(name, "failed LinkByName") {
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(nil, errors.New(tt.errMsg)).Once()
-			} else if strings.Contains(name, "failed LinkAdd") {
+			if strings.Contains(name, "failed LinkAdd") {
 				// myip := net.ParseIP("10.0.0.2")
 				myip := make(net.IP, 4)
 				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(tt.errMsg)).Once()
 			} else if strings.Contains(name, "failed LinkSetMaster") {
 				myip := make(net.IP, 4)
 				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(nil).Once()
 				mockNetlink.EXPECT().LinkSetMaster(vxlan, bridge).Return(errors.New(tt.errMsg)).Once()
 			} else if strings.Contains(name, "failed LinkSetUp") {
@@ -215,7 +202,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(tt.errMsg)).Once()
@@ -225,7 +211,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
@@ -78,12 +78,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
@@ -186,14 +186,6 @@ func Test_CreateSvi(t *testing.T) {
 			fmt.Sprintf("unable to find key %v", "unknown-vrf-id"),
 			false,
 		},
-		"failed LinkByName call": {
-			testSviID,
-			&testSvi,
-			nil,
-			codes.NotFound,
-			"unable to find key br-tenant",
-			false,
-		},
 		"failed BridgeVlanAdd call": {
 			testSviID,
 			&testSvi,
@@ -252,25 +244,20 @@ func Test_CreateSvi(t *testing.T) {
 			opi.Bridges[testLogicalBridgeName] = &testLogicalBridge
 
 			// TODO: refactor this mocking
-			if strings.Contains(name, "failed LinkByName") {
-				mockNetlink.EXPECT().LinkByName(tenantbridgeName).Return(nil, errors.New(tt.errMsg)).Once()
-			} else if strings.Contains(name, "failed BridgeVlanAdd") {
+			if strings.Contains(name, "failed BridgeVlanAdd") {
 				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(tt.errMsg)).Once()
 			} else if strings.Contains(name, "failed LinkAdd") {
 				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)}
 				mockNetlink.EXPECT().LinkAdd(vlandev).Return(errors.New(tt.errMsg)).Once()
 			} else if strings.Contains(name, "failed LinkSetHardwareAddr") {
 				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
@@ -193,13 +193,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)