In some environments, neighbor entries for non-cluster hosts remain stale forever, breaking connectivity in cases where their IP is reassigned to a new host with a different MAC address. This isn't a problem for hosts within the cluster; Cilium sets the managed and extern_learn flags for those neighbor entries, so the kernel actively refreshes them.

In our particular case, we have a set of VMs forming our Kubernetes cluster running a NodePort service and another VM in the same L2 domain talking to that service. This set of VMs is upgraded as a unit by spinning up new VMs, draining the old ones, and reassigning the IP addresses, although not necessarily in that order. At some point during this process, the MAC address assigned to the IP for that external VM changes. The network switches in our setup enable ARP suppression which, as I understand it, suppresses GARPs if the ARP suppression cache on the switch already has an entry for a given MAC address. This, combined with Cilium's current behavior leads to a situation where we we need to apply some workarounds to get things talking again. What we hope would occur is that cluster nodes would send an ARP request for any STALE neighbor entries in their ARP table, but currently this never happens.

fib_redirect() and others call bpf_fib_lookup() and pass its result along to fib_do_redirect(). Currently, fib_do_redirect() only uses bpf_redirect_neigh() if the FIB lookup was unsuccessful, returning BPF_FIB_LKUP_RET_NO_NEIGH. Otherwise, it "manually" populates the L2 header of the packet and uses bpf_redirect() to send it out through the host interface. However, bpf_fib_lookup() returns BPF_FIB_LKUP_RET_SUCCESS regardless of the state of the neighbor entry, and bpf_redirect() doesn't trigger a neighbor event in the kernel. The net result is that stale entries can remain stale which is problematic in setups like the one described above.

Always use bpf_redirect_neigh() if available to trigger refresh of stale entries and avoid corner cases like ours where invalid entries can can remain stale forever. Use BPF_FIB_LOOKUP_SKIP_NEIGH [1] in preceding calls to bpf_fib_lookup() to avoid duplicate work.

[1]: bpf: Add BPF_FIB_LOOKUP_SKIP_NEIGH for bpf_fib_lookup

Hey Jordan, just a question for clarification/better understanding:

In the kernel's BPF fib lookup helper, we test nud_state for NUD_VALID (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/core/filter.c#n6180):

	if (!neigh || !(READ_ONCE(neigh->nud_state) & NUD_VALID))
		return BPF_FIB_LKUP_RET_NO_NEIGH;

The NUD_VALID is defined as (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/net/neighbour.h#n39):

#define NUD_VALID	(NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE|NUD_PROBE|NUD_STALE|NUD_DELAY)

The kernel has a NEIGH_VAR_BASE_REACHABLE_TIME_MS which iiuc defines the period:

base_reachable_time (since Linux 2.2) Once a neighbor has been found, the entry is considered
to be valid for at least a random value between base_reachable_time/2 and 3*base_reachable_time/2.
An entry's validity will be extended if it receives positive feedback from higher level protocols.
Defaults to 30 seconds. This file is now obsolete in favor of base_reachable_time_ms.
base_reachable_time_ms (since Linux 2.6.12) As for base_reachable_time, but measures time in
milliseconds. Defaults to 30000 milliseconds.

Dumb q: Would it rather make sense to tweak base reachable time instead? Is that mostly for NUD_REACHABLE state or does it also relate to NUD_STALE somehow?

Are you saying with redirect_neigh() the NUD_STALE automatically triggers a new resolution to go back into NUD_REACHABLE?

Edit: found the state diagram:

Ok, so in the kernel the transition from NUD_REACHABLE -> NUD_STALE does trigger re-resolution:

/* Neighbour state is suspicious;
   disable fast path.

   Called with write_locked neigh.
 */
static void neigh_suspect(struct neighbour *neigh)
{
	neigh_dbg(2, "neigh %p is suspected\n", neigh);

	WRITE_ONCE(neigh->output, neigh->ops->output);
}

Without This Change

The neighbor entry for 192.168.4.255, a VM in the same L2 domain as my cluster nodes, remains STALE after making an HTTP request to a NodePort service hosted on that node.

jordan@crow:~$ ip neigh | grep 225
192.168.4.225 dev enp5s0 lladdr bc:24:11:93:7f:d4 STALE
...
jordan@crow:~$ ip neigh | grep 225
192.168.4.225 dev enp5s0 lladdr bc:24:11:93:7f:d4 STALE

With This Change

The HTTP request from 192.168.4.255 triggers a neighbor event on the cluster node, so the neighbor entry transitions from STALE to REACHABLE.

jordan@crow:~$ ip neigh | grep 192.168.4.225
192.168.4.225 dev enp5s0 lladdr bc:24:11:93:7f:d4 STALE
...
jordan@crow:~$ ip neigh | grep 192.168.4.225
192.168.4.225 dev enp5s0 lladdr bc:24:11:93:7f:d4 DELAY
...
jordan@crow:~$ ip neigh | grep 192.168.4.225
192.168.4.225 dev enp5s0 lladdr bc:24:11:93:7f:d4 REACHABLE

Is it correct to assume that the above entries are the backend IPs? If yes, would this PR which is in main already have helped? #37352

@ysksuzuki,

Thank you for the PR! I have two questions.

• Have you considered making this behavior, where bpf_redirect_neigh() is always used when available, opt-in instead of enabling it by default? Or have you already evaluated that the potential performance impact would be trivial?

Thank you for taking a look! In my view this should simply be the default behavior if there are no ill effects, as it works for a broader set of circumstances. I've run some iperf3 benchmarks at 100 Gbps and did not experience any noticeable changes in throughput or CPU utilization between this and the old approach. I will try to find the measurements somewhere and provide them here. I'm happy to run some additional benchmarks if you are not convinced though :).

• Does this issue occur only when the destination is a host outside the cluster? If so, would it be worth considering an optimization where bpf_redirect_neigh() is always used only when the destination is outside the cluster?

The issue I experienced does, yes. Outside the cluster but in the same L2 domain as the cluster nodes. I believe it may further complicate things in fib.h to need to differentiate between cluster and non-cluster IPs as we invoke fib_do_redirect(). If there is no performance concern I think we should aim for the simplest approach.

@borkmann,

Dumb q: Would it rather make sense to tweak base reachable time instead? Is that mostly for NUD_REACHABLE state or does it also relate to NUD_STALE somehow?

If my read of the docs and code is correct, I'm not sure tweaking this solves the underlying issue. Seems like it just takes longer for the entry to go stale? A long enough timeout may mask the issue I described in one environment, but as with any timeout value, what works in one environment may not work for another.

Are you saying with redirect_neigh() the NUD_STALE automatically triggers a new resolution to go back into NUD_REACHABLE?

Right, any subsequent calls to neigh_output() after neigh_suspect() happens (when an entry goes stale) triggers neigh_resolve_output() as opposed to the usual neigh_connected_output(). neigh_resolve_output() triggers neigh_event_send() before jumping into dev_queue_xmit().

static const struct neigh_ops arp_generic_ops = {
	.family =		AF_INET,
	.solicit =		arp_solicit,
	.error_report =		arp_error_report,
	.output =		neigh_resolve_output,
	.connected_output =	neigh_connected_output,
};

static const struct neigh_ops arp_hh_ops = {
	.family =		AF_INET,
	.solicit =		arp_solicit,
	.error_report =		arp_error_report,
	.output =		neigh_resolve_output,
	.connected_output =	neigh_resolve_output,
};

static inline int neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
{
	return neigh_event_send_probe(neigh, skb, true);
}

Is it correct to assume that the above entries are the backend IPs? If yes, would this PR which is in main already have helped? #37352

To clarify, 192.168.4.225 is the IP of another VM in the same L2 domain as my cluster nodes but not part of the cluster itself. The neighbor entry exists on my cluster node (crow) as 192.168.4.225 talks to a node port service hosted on crow. I'm not sure #37352 helps here, as it seems to be targeting neighbor entries for backends managed by Cilium itself.

All in all, bpf_redirect_neigh() keeps neighbor entries fresh in addition to doing the neigh table lookup. By and large, Cilium keeps most relevant neighbor entries fresh (managed extern_learn for cluster nodes, etc.), but there are some corner cases lurking like this one. I was actually surprised to learn that Cilium doesn't already just default to bpf_redirect_neigh() wherever possible, as it seems like the behavior more closely matches what you'd get when using the kernel's stack. This could be my own ignorance talking though.

Are there performance reasons for the current preference for bpf_redirect() in most cases? I was concerned at first there might be a performance hit switching to bpf_redirect_neigh() in all cases due to the duplicated neighbor lookup (at least without BPF_FIB_LOOKUP_SKIP_NEIGH) in bpf_fib_lookup() and some of the extra work it has to do compared to bpf_redirect(). I was unable to detect any significant differences with 100 Gbps iperf3 test which I'm happy to share here, although that doesn't mean they don't exist :). If indeed we are concerned about every ounce of performance and bpf_redirect_neigh() is noticeably slower or worse in some dimension, perhaps we can be a bit more intelligent about when we use bpf_redirect_neigh() as @ysksuzuki hinted at above; however, I wanted to explore the simple approach first.

To clarify, 192.168.4.225 is the IP of another VM in the same L2 domain as my cluster nodes but not part of the cluster itself. The neighbor entry exists on my cluster node (crow) as 192.168.4.225 talks to a node port service hosted on crow

@jrife Just to confirm, the issue occurs when the datapath handles the reply traffic from the backends at nodeport_rev_dnat_ingress_ipv[4,6], and it hits a stale ARP cache entry for the client 192.168.4.225?

We can rely on the managed and extern_learn neighbor entries for the backends in the forward path. (With #37352, the Cilium agent manages these entries even if the backends are external hosts.) However, we can't rely on them in the reply path, as the Cilium agent doesn't manage the client's neighbor entries.

@jrife Just to confirm, the issue occurs when the datapath handles the reply traffic from the backends at nodeport_rev_dnat_ingress_ipv[4,6], and it hits a stale ARP cache entry for the client 192.168.4.225?

This is correct.

Bumping this thread again, since I'm not sure what the verdict was here @ysksuzuki .

@borkmann @ysksuzuki Looks like this one went stale. I'm wondering what the resolution is here. Is there anything in particular you are concerned about or data you would like to see?

My immediate reaction to this was "ugh, this feels like a workaround for something that should be improved in the kernel. And it won't help bpf_xdp when encountering the same problem".

At the same time, once #38308 is merged I feel it's a good moment to take a fresh look at all our FIB-driven redirect handling. And see whether we can simplify the logic more, or extend it to also cover scenarios like this.

My immediate reaction to this was "ugh, this feels like a workaround for something that should be improved in the kernel. And it won't help bpf_xdp when encountering the same problem".

From my point of view, it's less a kernel issue and more that the behavior of bpf_redirect skips a step that the kernel would normally perform on the way out, namely engaging with the neighboring subsystem and triggering side-effects like neighbor resolution on stale entires. bpf_redirect_neigh does do this stuff, so all things being equal (including performance) I'm not sure why you wouldn't just prefer redirect_neigh here; it seems like you'd be less likely to hit weird edge cases like this. Any insight into why bpf_redirect is currently preferred here?

My immediate reaction to this was "ugh, this feels like a workaround for something that should be improved in the kernel. And it won't help bpf_xdp when encountering the same problem".

From my point of view, it's less a kernel issue and more that the behavior of bpf_redirect skips a step that the kernel would normally perform on the way out, namely engaging with the neighboring subsystem and triggering side-effects like neighbor resolution on stale entires. bpf_redirect_neigh does do this stuff, so all things being equal (including performance) I'm not sure why you wouldn't just prefer redirect_neigh here; it seems like you'd be less likely to hit weird edge cases like this. Any insight into why bpf_redirect is currently preferred here?

Yes, we should just stick to redirect_neigh and remove the corner cases. For XDP it's unfortunately not possible to resolve anything ad-hoc, so there we need to rely on managed neigh to keep entries fresh and have this corner case wrt staleness for non-managed neighbors.

At the same time, once #38308 is merged I feel it's a good moment to take a fresh look at all our FIB-driven redirect handling. And see whether we can simplify the logic more, or extend it to also cover scenarios like this.

Started here, would be cool to rebase once this lands and see how easy the code looks overall.

Started #39490, would be cool to rebase once this lands and see how easy the code looks overall.

Sure I can wait for this to merge. This PR probably needs a rebase in general, since it's a bit old.

ci-gateway-api failure looks like #40243. Retriggering.

/ci-ginkgo

/ci-gateway-api

ci-ginkgo is failing fairly consistently with

• Failure [173.568 seconds]
K8sDatapathServicesTest
/home/runner/work/cilium/cilium/test/ginkgo-ext/scopes.go:461
  Checks N/S loadbalancing
  /home/runner/work/cilium/cilium/test/ginkgo-ext/scopes.go:461
    With ClusterIP external access
    /home/runner/work/cilium/cilium/test/ginkgo-ext/scopes.go:461
      ClusterIP can be accessed when external access is enabled [It]
      /home/runner/work/cilium/cilium/test/ginkgo-ext/scopes.go:515

      cannot curl to service IP from host: time-> DNS: '0.000022()', Connect: '0.000000',Transfer '0.000000', total '0.000000'command terminated with exit code 7
      
      Expected command: kubectl exec -n kube-system log-gatherer-52wsk -- curl -k --path-as-is -s -D /dev/stderr --fail --connect-timeout 5 --max-time 20 [http://10.96.114.208:80/](http://10.96.114.208/) -w "time-> DNS: '%{time_namelookup}(%{remote_ip})', Connect: '%{time_connect}',Transfer '%{time_starttransfer}', total '%{time_total}'" 
      To succeed, but it failed:
      Exitcode: 7 
      Err: exit status 7
      Stdout:
       	 time-> DNS: '0.000022()', Connect: '0.000000',Transfer '0.000000', total '0.000000'
      Stderr:
       	 command terminated with exit code 7

At a glance, this seems unrelated to this PR, as this is the first time I'm seeing this and started happening after I rebased. Inspecting the most recent runs I see several runs with the same failure:

• https://github.com/cilium/cilium/actions/runs/16991274813
• https://github.com/cilium/cilium/actions/runs/16991468284
• https://github.com/cilium/cilium/actions/runs/16992137171
• https://github.com/cilium/cilium/actions/runs/16992791434
• https://github.com/cilium/cilium/actions/runs/16993917752
• https://github.com/cilium/cilium/actions/runs/16995052553

I'll do a bit more investigation and perhaps file a CI bug.

Edit: Another PR with the same failure: #41190

FWIW looking at the above links, if you browse to "Echo Workflow Dispatch Inputs" and expand the step inside, it's clear that these results are each from different PRs. The last link skipped that step, which implies the workflow was triggered directly (likely due to schedule) rather than via workflow_dispatch, so that likely means that it was also observed directly on a branch.

@joestringer Thanks. I just discovered the "Echo Workflow Dispatch Inputs" step after desperately searching for a way to correlate the run with its origin. I updated my description above to point to another example I found. Since this seems like a CI regression and not localized to this PR, I'll open an issue.

/test

/test

The ci-multi-pool failure seems to be an existing flake looking at a series of recent runs. Created #41283 with details.

/ci-multi-pool

Retriggering ci-eks. Looks like the node groups couldn't be created (#40836). Link

/ci-eks