From 37d78c9ef3944c1b060e3e8259b82fea3f8ec6bf Mon Sep 17 00:00:00 2001
From: David Gibson <david@gibson.dropbear.id.au>
Date: Wed, 26 Mar 2025 14:44:06 +1100
Subject: [PATCH] udp: Always hash socket facing flowsides
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 <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
udp_flow.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/udp_flow.c b/udp_flow.c
index c6b8630..7e80924 100644
--- a/udp_flow.c
+++ b/udp_flow.c
@@ -41,25 +41,23 @@ 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;
+ 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;
+ }
}
- 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_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);