From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id B9FF95A004F for ; Mon, 08 Jul 2024 23:27:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1720474055; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1Bch6QXG/+usSG5e6LDTa4IAFnMXIvU8DQ8DNMMpn/8=; b=UfokRZNTZtA6/ka9hzZXIbF1miStrzedFpH+4Gj2I71FcEb24qlU0daDhupwyO3Dipbdg3 ckHHZbJiv5FSNENwrsH3IHXGC2gges8p+b8RU5EFVawFw2cVUOwO15xW7tkhHMVAJ+L/Qg tWNfFOA/fnfGjZLGeBBN/pB/agKnkwk= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-357-EscgOEf3OsuopkXMz-x54g-1; Mon, 08 Jul 2024 17:27:34 -0400 X-MC-Unique: EscgOEf3OsuopkXMz-x54g-1 Received: by mail-qk1-f200.google.com with SMTP id af79cd13be357-79efed0e796so524637585a.1 for ; Mon, 08 Jul 2024 14:27:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720474053; x=1721078853; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=1Bch6QXG/+usSG5e6LDTa4IAFnMXIvU8DQ8DNMMpn/8=; b=KvKHJlHqdsJLqtVJ1QnHYZvGAmnBLb1Zm84F/vjmsilBw6mIxr+8TbehNvq8m+GKps HQw3JuXKXqKYhGIfRDdB0oIuEXFdtjanm6pNvmiV79aJLnGxeWuk3wXLPaqyGmMp/eS4 LtLxnCI8Fl4L+WqZumlWJiw1KTpUSYIOsFO/QBDgi80XMNLAKn/XG9a8a/e9QdNh3mzk kErOtDEVTsij5eRWgNY9KDCErFiC8W3HzeQZ1Qck22jS+ynjqP+wcAMJ+34e+/zwmeEF 0gBlbniaDoI6alVvZfkYXMVA9NTdcsg5b/z8RgfLz2KukaW0U9/9uxolkpPdUyihamoC 07Yg== X-Gm-Message-State: AOJu0YwpgRxw+YOuPXnPcB1LhXznKor4LhLzEoWnY5+KDhgln+O4Yz2A EUTtaLUpZKKtDkm67JhkVK3L/XW7MMX2NRN5trjr6Ymwlkuvy8oWL+bh0zwoFdSHHVRZ577npv/ quJq1ZoZFq+NZGeQ6EPufDfyuQalYhPadLOdbkaQW8jMTch1NCw== X-Received: by 2002:a05:620a:72b7:b0:79f:12e9:1e51 with SMTP id af79cd13be357-79f1b566597mr82657885a.5.1720474053631; Mon, 08 Jul 2024 14:27:33 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG5/G81mTLAK1CxSIMK82JfXGum3+aRhxHX9zZ9/HgAzkSmj99+h986vYR1ihZutJC2hnjtKA== X-Received: by 2002:a05:620a:72b7:b0:79f:12e9:1e51 with SMTP id af79cd13be357-79f1b566597mr82655785a.5.1720474053184; Mon, 08 Jul 2024 14:27:33 -0700 (PDT) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id af79cd13be357-79f18ff6762sm30117985a.10.2024.07.08.14.27.31 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Jul 2024 14:27:31 -0700 (PDT) Date: Mon, 8 Jul 2024 23:26:55 +0200 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v7 19/27] fwd: Update flow forwarding logic for UDP Message-ID: <20240708232655.76b43f59@elisabeth> In-Reply-To: <20240705020724.3447719-20-david@gibson.dropbear.id.au> References: <20240705020724.3447719-1-david@gibson.dropbear.id.au> <20240705020724.3447719-20-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 4AFG5M5AHYZ6XHN22O6CML2LUT4R7A3S X-Message-ID-Hash: 4AFG5M5AHYZ6XHN22O6CML2LUT4R7A3S X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, jmaloy@redhat.com X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri, 5 Jul 2024 12:07:16 +1000 David Gibson wrote: > Add logic to the fwd_nat_from_*() functions to forwarding UDP packets. The > logic here doesn't exactly match our current forwarding, since our current > forwarding has some very strange and buggy edge cases. Instead it's > attempting to replicate what appears to be the intended logic behind the > current forwarding. > > Signed-off-by: David Gibson > --- > fwd.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/fwd.c b/fwd.c > index 5731a536..4377de44 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -169,12 +169,15 @@ void fwd_scan_ports_init(struct ctx *c) > uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, > const struct flowside *ini, struct flowside *tgt) > { > - (void)proto; > - > tgt->eaddr = ini->faddr; > tgt->eport = ini->fport; > > - if (!c->no_map_gw) { > + if (proto == IPPROTO_UDP && tgt->eport == 53) { > + if (inany_equals4(&tgt->eaddr, &c->ip4.dns_match)) > + tgt->eaddr = inany_from_v4(c->ip4.dns_host); > + else if (inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) > + tgt->eaddr.a6 = c->ip6.dns_host; > + } else if (!c->no_map_gw) { There's a subtle difference here compared to the logic you dropped in 23/27 (udp_tap_handler()), which doesn't look correct to me. Earlier, with neither c->ip4.dns_match nor c->ip6.dns_match matching, we would let UDP traffic directed to port 53 be mapped to the host, if (!c->no_map_gw). That is, the logic was rather equivalent to this: if (proto == IPPROTO_UDP && tgt->eport == 53 && (inany_equals4(&tgt->eaddr, &c->ip4.dns_match) || inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) { if (inany_equals4(&tgt->eaddr, &c->ip4.dns_match)) tgt->eaddr = inany_from_v4(c->ip4.dns_host); else if (inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) tgt->eaddr.a6 = c->ip6.dns_host; } else if (!c->no_map_gw) { ... and I think we should maintain it, because if dns_match doesn't match, DNS traffic considerations shouldn't affect NAT decisions at all. > if (inany_equals4(&tgt->eaddr, &c->ip4.gw)) > tgt->eaddr = inany_loopback4; > else if (inany_equals6(&tgt->eaddr, &c->ip6.gw)) > @@ -191,6 +194,10 @@ uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, > > /* Let the kernel pick a host side source port */ > tgt->fport = 0; > + if (proto == IPPROTO_UDP) { > + /* But for UDP we preserve the source port */ > + tgt->fport = ini->eport; > + } > > return PIF_HOST; > } > @@ -232,9 +239,14 @@ uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto, > tgt->eport = ini->fport; > if (proto == IPPROTO_TCP) > tgt->eport += c->tcp.fwd_out.delta[tgt->eport]; > + else if (proto == IPPROTO_UDP) > + tgt->eport += c->udp.fwd_out.f.delta[tgt->eport]; > > /* Let the kernel pick a host side source port */ > tgt->fport = 0; > + if (proto == IPPROTO_UDP) > + /* But for UDP preserve the source port */ > + tgt->fport = ini->eport; > > return PIF_HOST; > } > @@ -256,20 +268,26 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, > tgt->eport = ini->fport; > if (proto == IPPROTO_TCP) > tgt->eport += c->tcp.fwd_in.delta[tgt->eport]; > + else if (proto == IPPROTO_UDP) > + tgt->eport += c->udp.fwd_in.f.delta[tgt->eport]; > > if (c->mode == MODE_PASTA && inany_is_loopback(&ini->eaddr) && > - proto == IPPROTO_TCP) { > + (proto == IPPROTO_TCP || proto == IPPROTO_UDP)) { > /* spliceable */ > > /* Preserve the specific loopback adddress used, but let the > * kernel pick a source port on the target side */ > tgt->faddr = ini->eaddr; > tgt->fport = 0; > + if (proto == IPPROTO_UDP) > + /* But for UDP preserve the source port */ > + tgt->fport = ini->eport; > > if (inany_v4(&ini->eaddr)) > tgt->eaddr = inany_loopback4; > else > tgt->eaddr = inany_loopback6; > + > return PIF_SPLICE; > } > -- Stefano