Skip to content

feat: convert execs to ip to netlink calls#1697

Merged
aauren merged 17 commits intomasterfrom
migrate_to_netlink_library
Jun 29, 2025
Merged

feat: convert execs to ip to netlink calls#1697
aauren merged 17 commits intomasterfrom
migrate_to_netlink_library

Conversation

@aauren
Copy link
Copy Markdown
Collaborator

@aauren aauren commented Jul 5, 2024 •
edited
Loading

Not making direct exec calls to user binary interfaces has long been a principle of kube-router. When kube-router was first coded, the netlink library was missing significant features that forced us to exec out. However, now netlink seems to have most of the functionality that we need.

This converts all of the places where we can use netlink to use the netlink functionality.

The current state of this PR is untested and still needs to undergo significant testing:

  • Ensure IPv4 routes are getting populated correctly
  • Ensure IPv4 source routing is being added to custom table
  • Ensure IPv6 routes are getting populated correctly
  • Ensure IPv6 source routing is being added to custom table
  • Ensure IPv4 Service VIPs get added to the dummy interface
  • Ensure IPv6 Service VIPs get added to the dummy interface
  • Ensure DSR works
  • Ensure ipip encapsulation works
  • Ensure fou encapsulation works

@aauren aauren force-pushed the migrate_to_netlink_library branch from 38733d4 to 7a58331 Compare July 5, 2024 20:36
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 4, 2024

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Sep 4, 2024
@aauren aauren added override-stale Don't allow automatic management of stale issues / PRs and removed Stale labels Sep 8, 2024
@aauren aauren mentioned this pull request Oct 1, 2024
Merged
@aauren aauren force-pushed the migrate_to_netlink_library branch 2 times, most recently from 4657546 to cc734e5 Compare May 6, 2025 22:14
@aauren aauren mentioned this pull request May 10, 2025
Closed
@aauren aauren force-pushed the migrate_to_netlink_library branch from 3a4526c to 08f07c7 Compare May 10, 2025 21:00
@aauren aauren force-pushed the migrate_to_netlink_library branch from 08f07c7 to b0246ef Compare June 20, 2025 21:41
aauren added 16 commits June 28, 2025 12:27
@aauren
feat: convert execs to ip to netlink calls
7f7b59d 
Not making direct exec calls to user binary interfaces has long been a
principle of kube-router. When kube-router was first coded, the netlink
library was missing significant features that forced us to exec out.
However, now netlink seems to have most of the functionality that we
need.

This converts all of the places where we can use netlink to use the
netlink functionality.
@aauren
fix(linux_routing.go): choose first rt_tables file
11e7d8f 
The rt_tables list is already in an ordered form in terms of priority.
Once one is found, it should be considered the optimal one and stop
looking for additional tables.
@aauren
fix(linux_networking.go): filter routes to be deleted by table
7afa017 
Previously we were accidentally deleting all routes that were found,
this mimics the previous functionality better by only deleting external
IPs that were found in the externalIPRouteTable that are no longer in
the activeExternalIPs map.

Also improves logging around any routes that are deleted as this is
likely of interest to all kube-router administrators.
@aauren
fix(linux_networking.go): add scope to local routes
8627bfc 
In order for a local route to be valid it needs to have the scope set to
host. When we were executing ip commands iproute2 just did this for us
to make the command accurate. Now that we're communicating with the
netlink socket, we need to do this conversion for ourselves.

Without this we get an error that says "invalid argument" from the
netlink subsystem. But if the route isn't local, then most of the
routing logic for services doesn't work correctly because it acts upon
external traffic as well as local traffic which isn't correct.
@aauren
fix(ip rule): use NewRule() for all rule creations
069d80e 
It has proven to be tricky to insert new rules without calling the
designated NewRule() function from the netlink library. Usually attempts
will fail with an operation not supported message.

This improves the reliability of rule insertion.
@aauren
fact: add ip utils library & add unit testing
9a31dbf 
Consolidate IP utility functions into a new file and add proper unit
testing. Additionally consolidate logic and references to default route
subnets.
@aauren
fix: add proper nil rule src handling
7ca733b 
When ip rules are evaluated in the netlink library, default routes for
src and dst are equated to nil. This makes it difficult to evaluate
them and requires additional handling in order for them.

I filed an issue upstream so that this could potentially get fixed:
vishvananda/netlink#1080 however if it doesn't
get resolved, this should allow us to move forward.
@aauren
fact(utils): factor out single subnet ip logic
899dc4d 
Removes repeated logic of calculating IP address subnets for single
subnet hosts and consolidates it in one place.
@aauren
fix(NSC): pass fwmark to traffic director as an int
34db638 
It used to be when we were using iproute2's CLI we needed to have the
fwmark as a hex number so we were passing it as a string in that format.

However, now that we use the netlink library directly, we already have
the fwmark in the condition that we need it. So instead of doing all of
these string <-> int conversions, lets just keep this simpler.
@aauren
fix(ipAddrDel): check for routes before trying to delete
5d28ec3 
Instead of deleting and just hoping for the best, this change makes it
so that we check first whether or not a route exists. This helps to
reduce needless warnings that the user receives and is just all around
more accurate.
@aauren
feat(NSC): add some additional debugging to traffic director
47761a1
@aauren
fix(ipAddrDel): check to see if IP exists on interface before delete
28d696b 
Rather than yolo'ing a delete of the IP on the interface, check to see
if it exists and save the user some warning in their logs.
@aauren
fix(DSR): setup source routing for all external IPs
2fe9815 
Previously, kube-router was only considering externalIPs when setting up
source routing policy, notably absent was consideration of LoadBalancer
IPs which are equally important for getting right with DSR.

This appears to have been a long-standing use-case that was never
correctly considered since when kube-router added a LoadBalancer
controller.
@aauren
feat(linux_tunnels): convert to use netlink library
9ec74e4
@aauren
fact(linux_tunnels): split SetupOverlayTunnel
7bce8a2 
Over time this function has grown to be way too large and difficult to
read. This refactor splits out this function into smaller chunks and
makes it easier to follow what's going on.
@aauren
feat(linux_networking): add some additional logging
f0244b5
@aauren aauren force-pushed the migrate_to_netlink_library branch from b0246ef to f0244b5 Compare June 28, 2025 17:27
@aauren
Copy link
Copy Markdown
Collaborator Author

aauren commented Jun 28, 2025

This PR is almost ready to go. There is just one small problem with IPv4 DSR service routing from a worker node in the cluster to an LB IP when the destination get's loadbalanced to another node in the cluster. During this scenario the service request times out instead of being fulfilled.

This doesn't seem to affect IPv6 traffic, or non-DSR enabled services, or IPv4 / DSR services when the traffic policy is local. Brief testing shows that this scenario also works ok with the current stable kube-router build.

@aauren
feat(DSR): disable routing DSR traffic via kube-bridge
102f1a7 
This was originally added in PR #210, but it appears to cause more
problems in my testing scenarios than it solves. When this is enabled,
it makes it so that services cannot be routed to from kube workers to
DSR enabled services when routed to other nodes in the cluster.
@aauren aauren marked this pull request as ready for review June 29, 2025 22:41
@aauren
Copy link
Copy Markdown
Collaborator Author

aauren commented Jun 29, 2025

There is a minor outstanding case where there is some problems with DSR traffic (see #1870) however, this was true whether netlink was used or iproute2 user-space tooling. So I think that we're fine leaving that issue alone for now.

@aauren aauren merged commit 7006205 into master Jun 29, 2025
7 checks passed
@aauren aauren deleted the migrate_to_netlink_library branch June 29, 2025 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

override-stale Don't allow automatic management of stale issues / PRs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@aauren