On Thu, Feb 22, 2024 at 06:46:02PM +0100, Stefano Brivio wrote:On Thu, 22 Feb 2024 10:21:09 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Not really, because "this host" has a different meaning to the sender than it does to us. For example, returning replies to DHCP broadcasts to localhost would absolutely not be correct. Of course, attempting to run a DHCP server within passt/pasta sounds problematic, but my point is that localhost is not a reasonable translation.If an incoming packet has a source address of 0.0.0.0 we translate that to the gateway address. This doesn't really make sense, because we have no way to do a reverse translation for reply packets.Well, we would translate that back to a loopback address, which is fine if we take 0.0.0.0 as "This host on this network".Actually, after my previous note based on RFC 6890, I went and had a look at RFC 1122, section 3.2.1.3, which also states that 0.0.0.0: MUST NOT be sent, except as a source address as part of an initialization procedure by which the host learns its own IP address. ...so I guess dropping it here is fine.I'm not dropping it: I'm leaving it untranslated.By the way, I added this originally as part of commit 6488c3e8489d ("tcp, udp: Replace loopback source address by gateway address") on the basis that 0.0.0.0 could be used in lieu of a loopback address, but sure, we shouldn't even get it from the kernel to start with.Again, that's true for certain API calls, but AFAICT it's not true on the wire. At least it seems to be that way in practice, although I haven't located an RFC to say that explicitly.-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibsonCertain UDP protocols do use an unspecified source address in some circumstances (e.g. DHCP). These generally either require no reply, a multicast reply, or provide a suitable reply address by other means. In none of those cases does translating it in passt/pasta make sense. The best we can really do here is just leave it as is. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- udp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/udp.c b/udp.c index a3961bfd..d2f8027c 100644 --- a/udp.c +++ b/udp.c @@ -599,7 +599,6 @@ static size_t udp_update_hdr4(const struct ctx *c, int n, in_port_t dstport, src_port == 53) { b->iph.saddr = c->ip4.dns_match.s_addr; } else if (IN4_IS_ADDR_LOOPBACK(&b->s_in.sin_addr) || - IN4_IS_ADDR_UNSPECIFIED(&b->s_in.sin_addr)|| IN4_ARE_ADDR_EQUAL(&b->s_in.sin_addr, &c->ip4.addr_seen)) { b->iph.saddr = c->ip4.gw.s_addr; udp_tap_map[V4][src_port].ts = now->tv_sec;