public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: steffhip@gmail.com, David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 2/2] flow: Remove over-zealous sanity checks in flow_sidx_hash()
Date: Thu,  5 Dec 2024 15:26:02 +1100	[thread overview]
Message-ID: <20241205042602.701755-3-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20241205042602.701755-1-david@gibson.dropbear.id.au>

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 <david@gibson.dropbear.id.au>
---
 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);
 }
 
-- 
@@ -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


  parent reply	other threads:[~2024-12-05  4:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-05  4:26 [PATCH 0/2] Fix handling of ICMP flows with 0 id David Gibson
2024-12-05  4:26 ` [PATCH 1/2] udp: Improve detail of UDP endpoint sanity checking David Gibson
2024-12-05  4:26 ` David Gibson [this message]
2024-12-05 21:46 ` [PATCH 0/2] Fix handling of ICMP flows with 0 id Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241205042602.701755-3-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    --cc=steffhip@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).