Src addr code review (Was: Source IP incorrect on multi homed systems)

[Daniel Gröber]

Hi,

I though it might be useful to do some quick and dirty code review instead
of speculating wildly to figure out where these source IP selection
problems could be coming from ;)

>From previous code deep dives I know the udp_tunnel_xmit_skb function is
where tunnel packets get handed off to the kernel. So in
net/wireguard/socket.c:send4 we have:

	udp_tunnel_xmit_skb(rt, sock, skb, fl.saddr, fl.daddr, ds,
			    ip4_dst_hoplimit(&rt->dst), 0, fl.fl4_sport,
			    fl.fl4_dport, false, false);

Where fl.saddr is the source address that's supposedly wrong (sometimes? I
guess?) Where does that come from?

Let's look at the code (heavily culled):

	struct flowi4 fl = {
		.saddr = endpoint->src4.s_addr,
	};
	if (cache)
		rt = dst_cache_get_ip4(cache, &fl.saddr);
	if (!rt) {
		if (unlikely(!inet_confirm_addr(sock_net(sock), NULL, 0,
						fl.saddr, RT_SCOPE_HOST)))
			fl.saddr = 0;
		if (unlikely(endpoint->src_if4 && ((IS_ERR(rt) &&
			     PTR_ERR(rt) == -EINVAL) || (!IS_ERR(rt) &&
			     rt->dst.dev->ifindex != endpoint->src_if4))))
			fl.saddr = 0;

Well it's initialized from endpoint->src4.s_addr, overwritten with zero in
some cases, which I believe lets the kernel do it's regular source addr
selection, and populated from something called dst_cache at some callsites.

@Nico could it perhaps simply be that you're hitting one of these zero'ing
cases and that's why it's using regular kernel src addr selection instead
of the cached endpoint src4 address?

The first case !inet_confirm_addr(..., RT_SCOPE_HOST) ought to confirm that
the saddr is actually still a local address. Makes sens if the address we
remembered was removed from the interface we can't use it anymore.

The second case looks like it's checking if the (sometimes cached) src_if4
interface index is still what the route we're about to use points to.

If neither of those seem likely we can keep reading :)

--Daniel