netlink walk calls may return EINTR (regression)

[saj]

Downstream users of the CNI plugins have been observing occasional interrupted system call failures. The simple loopback plugin is also affected.

Calls like netlink.AddrList may return EINTR as of vishvananda/netlink v1.2.1.
vishvananda/netlink@aa4f20d
Prior to this commit, the (possibly incomplete) results from a walk would be returned with a nil error.

I think this upstream commit was incorporated here in d924f05, which was shipped as v1.6.0.

https://www.kernel.org/doc/html/next/userspace-api/netlink/intro.html#dump-consistency

Dump consistency

Some of the data structures kernel uses for storing objects make it hard to provide an atomic snapshot of all the objects in a dump (without impacting the fast-paths updating them).

Kernel may set the NLM_F_DUMP_INTR flag on any message in a dump (including the NLMSG_DONE message) if the dump was interrupted and may be inconsistent (e.g. missing objects). User space should retry the dump if it sees the flag set.

More context can be found in vishvananda/netlink#1018, which later added netlink.ErrDumpInterrupted. This commit is yet to be released, though it is available on vishvananda/netlink trunk.

AIUI:

• if you require a consistent result from a walk, and can tolerate a potentially unbounded wait, these netlink calls should be retried
• otherwise, disregard the error
  if errors.Is(err, unix.EINTR) on vishvananda/netlink v1.3.0, or
  if errors.Is(err, netlink.ErrDumpInterrupted) on vishvananda/netlink trunk

ty

[thompson-shaun]

We also seem to be running into this issue in moby/buildkit ref: moby/buildkit#5533

[thaJeztah]

Related? cc @robmry

• Retry on EINTR from netlink dump calls moby/moby#48407
• networking: investigate EINTR regression after updating github.com/vishvananda/netlink to v1.3.0 moby/moby#48400

[robmry]

Yes, it's the same issue - the description captures it.

In moby we ended up trying up to (an arbitrary) five times, then using the results anyway - on the basis that we'd probably be better off than we were before vishvananda/netlink v1.2.1, because a retry is likely to succeed, and the worst case is the same as it was. That's in:

• After retries, use netlink data even if the dump is still interrupted moby/moby#48598

[BenTheElder]

We're seeing the loopback failure causing flakes in the Kubernetes project's CI occasionally (pod sandbox fails like Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "8ea1710a0876be5a53157e6a3dc1bdde0bf694d7e003886ed4bba6b62c2e5e9b": plugin type="loopback" failed (add): interrupted system call)
https://storage.googleapis.com/k8s-triage/index.html?pr=1&text=interrupted%20system%20call cc @aojea

[aojea]

@squeed this is a source of instability on the ecosystem, it seems #1121 (comment) is the best option to solve it kubernetes/kubernetes#129562 , using a wrapper https://github.com/moby/moby/blob/a0a0bbae6c82f60ccbcb7d1a5283905dd882638f/internal/nlwrap/nlwrap_linux.go with retries

It seems cilium also adopted this approach of having a wrapper of the netlink library with retries https://github.com/cilium/cilium/blob/bb70d6f3313c097a5755bd0deeef55e551b4d6ba/pkg/datapath/linux/safenetlink/netlink_linux.go#L19

[aroradaman]

cc @humblec @yashsingh74

[aojea]

cc: @adrianmoisey

[adrianmoisey]

I'll take a look at adding a wrapper here

[MikeZappa87]

Does it make sense to do the retry logic in the netlink library? That would have a greater impact vs the whack a mole approach.

[BenTheElder]

vishvananda/netlink#1018