From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202410 header.b=XRIZNCeO; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 806CA5A061C for ; Thu, 05 Dec 2024 05:26:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202410; t=1733372760; bh=ZvbKVzd4fbCJEc+KzRlitGD3P5eZFI0f8DU853VnI5E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XRIZNCeOAkRdFcYNm7G8WhYdM3MlHHMJKz2oet9Xi2+1x7Xx6APuJiGM6IcJfWlWn VHFJnIH5xvim/VtqOeC2cJ0i8LNcPJ+TvNPb59JIZ2g31eEuL40KkuVTS2WwN4PKx/ 2J1+hyVTI/mSRJyjVPNI4qIuYs2jRCVGf3XGmaE6DoMS88HzZ7BxjXXqqwXelLzXca YrzM5M7pR0N9L4v2rRDvKVnTUM3qk4x++Nso9Vc1cvyFBYGOSUzLavObzy9wNNEMbn bk9dXmNsYkRdYIW3LygVG6bovbABFEQ1DrRzZaT36n7jt/+yPhrCvfLibadIxy6HZR yb3FpQTVKJj0Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Y3hCX2HBfz4x6J; Thu, 5 Dec 2024 15:26:00 +1100 (AEDT) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH 2/2] flow: Remove over-zealous sanity checks in flow_sidx_hash() Date: Thu, 5 Dec 2024 15:26:02 +1100 Message-ID: <20241205042602.701755-3-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.47.0 In-Reply-To: <20241205042602.701755-1-david@gibson.dropbear.id.au> References: <20241205042602.701755-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: R3DUFQ2LLRKHP7EDEFZH2F5HKW5LSWNR X-Message-ID-Hash: R3DUFQ2LLRKHP7EDEFZH2F5HKW5LSWNR X-MailFrom: dgibson@gandalf.ozlabs.org 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: steffhip@gmail.com, David Gibson 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: In flow_sidx_hash() we verify that the flow we're hashing doesn't have an unspecified endpoint address, or zero for either port. The hash table only works if we're looking for exact matches of address and port, and this is attempting to catch any cases where we might have left address or port unpopulated or filled with a wildcard. This doesn't really work though, because there are cases where unspecified addresses or zero ports are correct: * We already use unspecified addresses for our address in cases where we don't know the specific local address for that side, and exclude the obvious extra check on side->oaddr for that reason. * Zero port numbers aren't strictly forbidden over the wire. We forbid them for TCP & UDP because they can't safely be handled on the socket side. However for ICMP a zero id, which goes in the port field is valid. * Possible future flow types (for example, for multicast protocols) might legitimately have an unspecified address. Although it makes them easier to miss, these sorts of sanity checks really have to be done at the protocol / flow type layer, and we already do so. Remove the checks in flow_sidx_hash() other than checking that the pif is specified. Link: https://bugs.passt.top/show_bug.cgi?id=105 Signed-off-by: David Gibson --- flow.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/flow.c b/flow.c index 1ea112b..ee1221b 100644 --- a/flow.c +++ b/flow.c @@ -597,12 +597,7 @@ static uint64_t flow_sidx_hash(const struct ctx *c, flow_sidx_t sidx) const struct flowside *side = &f->side[sidx.sidei]; uint8_t pif = f->pif[sidx.sidei]; - /* For the hash table to work, entries must have complete endpoint - * information, and at least a forwarding port. - */ - ASSERT(pif != PIF_NONE && !inany_is_unspecified(&side->eaddr) && - side->eport != 0 && side->oport != 0); - + ASSERT(pif != PIF_NONE); return flow_hash(c, FLOW_PROTO(f), pif, side); } -- 2.47.0