incus icon indicating copy to clipboard operation
incus copied to clipboard

Use netlink directly instead of iproute2

Open gwenya opened this issue 8 months ago • 3 comments

We are currently calling the ip command for configuring addresses, routes and neighbour objects. This is unnecessary additional overhead and requires parsing the output in some cases, like here: https://github.com/lxc/incus/blob/faa488098f0d4ae41c3480b7e75473b51368a336/internal/server/ip/neigh.go#L61-L87

By using netlink directly we can avoid that. We already have a dependency on github.com/vishvananda/netlink, so we could use that without adding any extra dependencies.

If there is interest in this I will make a PR.

gwenya avatar Apr 23 '25 11:04 gwenya

Absolutely, it was actually the whole goal of the ip to separate all that logic out of random spots in the codebase ahead of moving them to direct netlink calls, so I'd definitely be happy to see us use netlink directly.

stgraber avatar Apr 23 '25 16:04 stgraber

@stgraber would you prefer one big PR for everything or smaller ones?

I'm also going to rework the API a bit, I can keep that separate as well to make code review easier.

Currently we're using strings for almost everything, which have to be parsed again to pass them to netlink, and I think there's something weird with the way the tc things are modeled, I'm gonna have to figure out what any of this even does.

gwenya avatar Apr 24 '25 22:04 gwenya

I'm fine either way. We could convert it bit by bit if that's easier or do it in one chunk if that's easier because of inter-dependencies.

Probably just try to split things at least on the commit front, basically one commit to convert to netlink, one commit to update whatever used it, or something like that.

stgraber avatar Apr 24 '25 22:04 stgraber