Add multi-path support to netlink client#185
Conversation
Resolves: #16 Signed-off-by: Maximilian Moehl <maximilian@moehl.eu>
| err := c.installRoute(dest, hop, table) | ||
| next := append(slices.Clone(current), hop) | ||
|
|
||
| // Ensure deterministic order to make comparing route tables across machines |
There was a problem hiding this comment.
a nuance point: I didn't understand the comment at the beginning, but realised that it is about using ip command during debugging. maybe saying '...printing and comparing routing tables across routers...'.
| delete(c.routes, key) | ||
| } else { | ||
| if err := c.replaceRoute(dest.Prefix, next, table); err != nil { | ||
| return fmt.Errorf("cannot install replacement route for %s: %v", dest, err) |
There was a problem hiding this comment.
Errors should be wrapped via %w. I will add a fixup commit.
| } | ||
|
|
||
| err := c.installRoute(dest, hop, table) | ||
| next := append(slices.Clone(current), hop) |
There was a problem hiding this comment.
The semantics changed slightly here I think. Where previously if there were some hops we added the hop, we might now not call it if our route cache (not necessarily the kernel's state) has the hop already.
By always writing via Replace (add or update), we could ensure that this will always converge to the desired state, even if the kernel's table changes.
I will add a fixup for this, but not sure if this is a relevant issue.
Wrap errors instead of just taking their message Signed-off-by: Alexander Lais <alexander.lais@sap.com>
Ensure that RouteReplace is always run, as it will add or update. This keeps the kernel's routing table in sync with our cache. Retains the old behavior of adding the route unconditionally and ensuring it exists. That matters for refill paths like MetalBond.AddRoutesForVni after peer reconnect. Signed-off-by: Alexander Lais <alexander.lais@sap.com>
peanball
left a comment
peanball
left a comment
There was a problem hiding this comment.
Do with the fixup commits as you'd like, either accept or rewrite in your name. Let me know if this style of review proposals is good at all or ```suggestion``` would have been better...
3 participants
Resolves: #16