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=202502 header.b=gkW71H1A; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id F0A4A5A0271 for ; Wed, 26 Mar 2025 04:44:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1742960649; bh=Shw5Oi7j1QlF8ih/9Ubw0ioAcXj6B3wSkS0atfVhu80=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gkW71H1ARUArIISHHX9x8c55pBCN7DC/ExBfAzNibVXXVwm4v2EjzUTX5T+ApSpG7 rGSZ3Ka4jHA4Ql+7abbJ5OZvdthRZMwrxoSvMALqqootRaQdyeZFuOZby/P/fgeV8w +0/++x0RhjWtMfogOvN8jiPTAJLzYnor+9EB3qaSLbJDWeALykkP1c+Zel+I5aVkIt 7rj3VM6/xYWv1KIggrpnBtVKXDI1ZXbj9iAUR7DVDRF5ZJ6uhsLkugUasbSucrfHgc clPF+aafFW5t76Qe7WMSL8gNXCdshVtGHlcNGv2v4G/bV82dqdxY/1M4qvvOM87eSu Cxyp5Y+yLHRWA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZMt215jLJz4xCy; Wed, 26 Mar 2025 14:44:09 +1100 (AEDT) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH v2 6/7] udp: Always hash socket facing flowsides Date: Wed, 26 Mar 2025 14:44:06 +1100 Message-ID: <20250326034407.2240846-7-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250326034407.2240846-1-david@gibson.dropbear.id.au> References: <20250326034407.2240846-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: ZSSRGQ7PCJUBFXBUBXORZID46F2KKYC3 X-Message-ID-Hash: ZSSRGQ7PCJUBFXBUBXORZID46F2KKYC3 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: 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: For UDP packets from the tap interface (like TCP) we use a hash table to look up which flow they belong to. Unlike TCP, we sometimes also create a hash table entry for the socket side of UDP flows. We need that when we receive a UDP packet from a "listening" socket which isn't specific to a single flow. At present we only do this for the initiating side of flows, which re-use the listening socket. For the target side we use a connected "reply" socket specific to the single flow. We have in mind changes that maye introduce some edge cases were we could receive UDP packets on a non flow specific socket more often. To allow for those changes - and slightly simplifying things in the meantime - always put both sides of a UDP flow - tap or socket - in the hash table. It's not that costly, and means we always have the option of falling back to a hash lookup. Signed-off-by: David Gibson --- udp_flow.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/udp_flow.c b/udp_flow.c index c6b8630a..7e809244 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -41,24 +41,22 @@ struct udp_flow *udp_at_sidx(flow_sidx_t sidx) */ void udp_flow_close(const struct ctx *c, struct udp_flow *uflow) { + unsigned sidei; + if (uflow->closed) return; /* Nothing to do */ - if (uflow->s[INISIDE] >= 0) { - /* The listening socket needs to stay in epoll */ - close(uflow->s[INISIDE]); - uflow->s[INISIDE] = -1; - } - - if (uflow->s[TGTSIDE] >= 0) { - /* But the flow specific one needs to be removed */ - epoll_del(c, uflow->s[TGTSIDE]); - close(uflow->s[TGTSIDE]); - uflow->s[TGTSIDE] = -1; + flow_foreach_sidei(sidei) { + flow_hash_remove(c, FLOW_SIDX(uflow, sidei)); + if (uflow->s[sidei] >= 0) { + /* The listening socket needs to stay in epoll, but the + * flow specific one needs to be removed */ + if (sidei == TGTSIDE) + epoll_del(c, uflow->s[sidei]); + close(uflow->s[sidei]); + uflow->s[sidei] = -1; + } } - flow_hash_remove(c, FLOW_SIDX(uflow, INISIDE)); - if (!pif_is_socket(uflow->f.pif[TGTSIDE])) - flow_hash_remove(c, FLOW_SIDX(uflow, TGTSIDE)); uflow->closed = true; } @@ -77,6 +75,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, { struct udp_flow *uflow = NULL; const struct flowside *tgt; + unsigned sidei; uint8_t tgtpif; if (!(tgt = flow_target(c, flow, IPPROTO_UDP))) @@ -143,14 +142,14 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, } } - flow_hash_insert(c, FLOW_SIDX(uflow, INISIDE)); - - /* If the target side is a socket, it will be a reply socket that knows - * its own flowside. But if it's tap, then we need to look it up by - * hash. + /* Tap sides always need to be looked up by hash. Socket sides don't + * always, but sometimes do (receiving packets on a socket not specific + * to one flow). Unconditionally hash both sides so all our bases are + * covered */ - if (!pif_is_socket(tgtpif)) - flow_hash_insert(c, FLOW_SIDX(uflow, TGTSIDE)); + flow_foreach_sidei(sidei) + flow_hash_insert(c, FLOW_SIDX(uflow, sidei)); + FLOW_ACTIVATE(uflow); return FLOW_SIDX(uflow, TGTSIDE); -- 2.49.0