From 09bf2fb60b73949486090a9d6da39359bf73a5dc Mon Sep 17 00:00:00 2001 From: Boris Glimcher Date: Thu, 31 Aug 2023 23:24:50 +0300 Subject: [PATCH] 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 --- pkg/evpn/bridge.go | 7 +------ pkg/evpn/bridge_test.go | 17 ----------------- pkg/evpn/svi.go | 7 +------ pkg/evpn/svi_test.go | 19 ------------------- pkg/evpn/vrf.go | 7 +------ pkg/evpn/vrf_test.go | 24 ------------------------ 6 files changed, 3 insertions(+), 78 deletions(-) diff --git a/pkg/evpn/bridge.go b/pkg/evpn/bridge.go index d93a5b1b..6e14fcce 100644 --- 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- type vxlan id local 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- 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 index 37de5854..c2f12ec1 100644 --- 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 index 80a81da0..ce510b37 100644 --- 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 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 index d35f0366..aae45715 100644 --- 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 index 3a1b484f..77b6d8b4 100644 --- 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 index 49040913..5eb260a0 100644 --- 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}