Skip to content

Commit

Permalink
fix(bridge): avoid LinkByName netlink calls
Browse files Browse the repository at this point in the history 
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 <Boris.Glimcher@emc.com>
  • Loading branch information
@glimchb
glimchb committed Aug 31, 2023
1 parent 1a2b0e5 commit ccfbeca
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 48 deletions.
7 changes: 1 addition & 6 deletions pkg/evpn/bridge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
17 changes: 1 addition & 16 deletions pkg/evpn/bridge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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") {
Expand All @@ -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()
Expand All @@ -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()
Expand Down
7 changes: 1 addition & 6 deletions pkg/evpn/svi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
15 changes: 1 addition & 14 deletions pkg/evpn/svi_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)}
Expand Down
7 changes: 1 addition & 6 deletions pkg/evpn/vrf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}}

Check warning on line 196 in pkg/evpn/vrf.go

View check run for this annotation

Codecov / codecov/patch

pkg/evpn/vrf.go#L196 

Added line #L196 was not covered by tests
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)
Expand Down

0 comments on commit ccfbeca

Please sign in to comment.