fix(bridge): avoid LinkByName netlink calls

[glimchb]

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]

[codecov]

Codecov Report

Merging #148 (09bf2fb) into main (c1f0da9) will decrease coverage by 0.33%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #148      +/-   ##
==========================================
- Coverage   75.00%   74.67%   -0.33%     
==========================================
  Files           5        5              
  Lines        1012      999      -13     
==========================================
- Hits          759      746      -13     
  Misses        218      218              
  Partials       35       35              

Files Changed	Coverage Δ
pkg/evpn/bridge.go	73.23% <100.00%> (-0.54%)	⬇️
pkg/evpn/svi.go	73.01% <100.00%> (-0.53%)	⬇️
pkg/evpn/vrf.go	75.58% <100.00%> (-0.38%)	⬇️

[mardim91]

Some comments

1. I do not see where the tenantbridgeName variable gets assigned (take its value).
2. I am not sure if it is the same thing creating a link object by hand compared to returning the link object by using the linkByName function. Maybe the returned object has more attributes that are needed during the subsequent netlink call (just a speculation).
3. I prefer the LinkByName as this way we are sure that object actually exists on the system and has not been deleted accidentally by a third party process.
4. I do not believe has a huge performance overhead. WDYT ?

[venkatmahalingam]

Some comments

1. I do not see where the tenantbridgeName variable gets assigned (take its value).
2. I am not sure if it is the same thing creating a link object by hand compared to returning the link object by using the linkByName function. Maybe the returned object has more attributes that are needed during the subsequent netlink call (just a speculation).
3. I prefer the LinkByName as this way we are sure that object actually exists on the system and has not been deleted accidentally by a third party process.
4. I do not believe has a huge performance overhead. WDYT ?

I thought querying a Linux kernel every time while we maintain a cache is unnecessary, if there is a possibility of third party process deletion, IMO, that's not a good design/coding practice and we should have a single source of truth.

[mardim91]

Some comments

1. I do not see where the tenantbridgeName variable gets assigned (take its value).
2. I am not sure if it is the same thing creating a link object by hand compared to returning the link object by using the linkByName function. Maybe the returned object has more attributes that are needed during the subsequent netlink call (just a speculation).
3. I prefer the LinkByName as this way we are sure that object actually exists on the system and has not been deleted accidentally by a third party process.
4. I do not believe has a huge performance overhead. WDYT ?

I thought querying a Linux kernel every time while we maintain a cache is unnecessary, if there is a possibility of third party process deletion, IMO, that's not a good design/coding practice and we should have a single source of truth.

Not really sure. To be honest I think is more proper to for me actually get the link object that is returned and use it for deletion than creating a new object with just the name. Also we do no know how the subsequent netlink calls will behave if we do no actually get the real object but we create one by hand. Maybe they will nee some attributed from the object that do no exist and that make them fail.

Also kubernetes CNIs use also the LinkByName approach in order to do further netlink operations check here