sbr: Add options to configure static gateway IP and preserve default routes

[dagrayvid]

This PR aims to solve two problems I faced using the source-based router CNI plugin for distributed LLM inference with vLLM / llm-d.

1. Option to configure a statically defined gateway IP to set or override the gateway if not passed in from the CNI config chain.

In my environment (IBM Cloud), the DHCP server my secondary high-speed networks does not provide the gateway IP, so I needed a way to override / statically define the gateway IPs to ensure that the custom routing tables included the default route to the gateway.

2. Option to "preserve default routes" in the main routing table, so that packets without a source IP are still routed correctly. For example in my environment after enabling SBR, ping -I 10.0.0.6 10.1.0.7 would work, but ping 10.1.0.7. With "preserveDefaultRoutes": true, both work.

Tested in my environment with this CNI config:

{
  "cniVersion": "0.3.1",
  "name": "dhcp-host-device-port-1",
  "plugins": [
    {
      "type": "host-device",
      "device": "enp163s0",
      "isRdma": true,
      "ipam": {
        "type": "dhcp"
      }
    },
    {
      "type": "sbr-custom",
      "gateway": "10.0.0.1",
      "preserveDefaultRoutes": true
    }
  ]
}

In a Pod, the result of the statically defined gateway:

bash-5.1# ip route show table 100
default via 10.0.0.1 dev net1
10.0.0.0/16 dev net1 proto kernel scope link src 10.0.0.5

and the result of preserve default route:

bash-5.1# ip route show table 254
default via 10.130.0.1 dev eth0
10.0.0.0/16 dev net1 scope link src 10.0.0.5
10.1.0.0/16 dev net2 scope link src 10.1.0.6
10.2.0.0/16 dev net3 scope link src 10.2.0.6
...

[squeed]

Looks nice!
Would there ever be a case where we wanted just source-hinted routes without the table?

[dagrayvid]

Thanks for the review @squeed!

Would there ever be a case where we wanted just source-hinted routes without the table?

I'm not sure I fully understand the question. If a user wanted only source-hinted routes without custom tables, I don't think they'd need SBR at all - the kernel already creates these automatically when IPs are assigned.

In my case at least, without SBR on my interface at 10.5.0.5, the source-hinted route is already in the main table:

$ ip route show
...
10.5.0.0/16 dev net6 proto kernel scope link src 10.5.0.5

The SBR plugin (without this added preserveDefaultRoutes option) deletes these after copying them to the custom tables:

plugins/plugins/meta/sbr/main.go

Lines 396 to 402 in 0d9f9b1

	for _, route := range routes {
	log.Printf("Deleting route %s from table %d", route.String(), route.Table)
	err := netlink.RouteDel(&route)
	if err != nil {
	return fmt.Errorf("Failed to delete route: %v", err)
	}
	}

Does that make sense? Please let me know if I've misunderstood your question.

[squeed]

🤦 of course. I don't know what I was thinking.

[karampok]

Some comments from my side

• 5 commits seem too many imo, maybe 2 (one for static gateway on for preserveDefaultRoutes) or even 1 (like atomic commits)
• test look a bit thin, like only ipv4 not unhappy path

(Note: I am not maintainer of the repo)

[karampok]

PreserveDefaultRoutes: is not so clear to me.

The "defaultRoutes" suffix makes think that in the main table I see a default route, which is not true, I only should see the subnet via eth0 src x.y.z.k, is that correct?

thx

[dagrayvid]

I think this is a good point. I renamed this option to AddSourceHints. Do you think that is more clear?

[squeed]

Can you please squash your commits? And a v6 test would be good.

[dagrayvid]

Thanks @squeed @karampok for the review. I renamed PreserveDefaultRoutes to AddSourceHints and added tests for ipv6 and dual stack. Also squashed the commits. PTAL

[karampok]

one nit comment, but lgtm in general

[dagrayvid]

/retest

[dagrayvid]

Just following up on this PR
@squeed