Hi Nico, That issue was closed by myself, but the patch didn't get applied cause the issue was came from wireguard itself, and the maintener told me that I should send my patch to wireguard upstream (but I just gave up for sending it to wireguard team).
Nico Schottelius <nico.schottel...@ungleich.ch> 于2023年2月20日周一 18:41写道: > > > Hello 曹煜, > > on github it seems your patch was applied / the issue was closed - is > that the correct current status? > > Best regards, > > Nico > > 曹煜 <cao8...@gmail.com> writes: > > > Hi all, > > I've hacked that source code myself months ago, and it works well on > > my use case (I have 4 dual stack pppoe wan set on my openwrt router, > > and seted a wireguard sever on it), my hack will pickup the dst_addr > > from incoming handshake packet in kernel sk_buff, and then use that > > addr as src_addr to reply. > > I'm not good at source code, and I know that my hack may be ugly, but > > it works, hope this patch can help: > > https://github.com/openwrt/packages/issues/9538#issuecomment-1150592803 > > > > Daniel Gröber <d...@darkboxed.org> 于2023年2月20日周一 06:42写道: > >> > >> 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 > >> > >> > >> > > > -- > Sustainable and modern Infrastructures by ungleich.ch