From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id E0E7D5A0272 for ; Wed, 20 Dec 2023 08:09:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1703056152; bh=SHMh3lCya87d+2A6iD7kaRnMI8PpgmnZh6wmA36WihQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LWsqqpl114NQM6UkGg4joCf/jgS6gz4TyHQ8NIsxFAdG79D3Wd1SsJPu8JqMCuyJN qTq8t1jqAUhJY3YFDbwaOQOXMYpMb+xcjru5IsZyUvncWLcm5C96x2X6L0v6G1DZzo n8ZW5a0rTb6SWFQLQ43N7qwrmKl/ix6LY1Kg49PorGUhRVLGthh6PfJFBWuEKm/rdv iRIq7dbzzMCz8szHnXrupERj+7uQDKErXq3Is2FzV/H3SHXCIhL9DyT0ye0AE6dmD/ PQ4qXsdhc45Ia93zcqLVQWL9ewWHQeZwX9fdzlpDvk3s1HiSlLHMGh4HiMI3t9z+EV QK7CYGFydeI7A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Sw4Rr383xz4xM2; Wed, 20 Dec 2023 18:09:12 +1100 (AEDT) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH v2 12/13] flow: Enforce that freeing of closed flows must happen in deferred handlers Date: Wed, 20 Dec 2023 18:09:07 +1100 Message-ID: <20231220070908.2506277-13-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20231220070908.2506277-1-david@gibson.dropbear.id.au> References: <20231220070908.2506277-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: KOFEYT7YM2H5CT5WOVUZTILPH72LLVUH X-Message-ID-Hash: KOFEYT7YM2H5CT5WOVUZTILPH72LLVUH 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: Currently, flows are only evern finally freed (and the table compacted) from the deferred handlers. Some future ways we want to optimise managing the flow table will rely on this, so enforce it: rather than having the TCP code directly call flow_table_compact(), add a boolean return value to the per-flow deferred handlers. If true, this indicates that the flow code itself should free the flow. This forces all freeing of flows to occur during the flow code's scan of the table in flow_defer_handler() which opens possibilities for future optimisations. Signed-off-by: David Gibson --- flow.c | 13 +++++++++---- flow.h | 1 - tcp.c | 9 +++++---- tcp_conn.h | 4 ++-- tcp_splice.c | 9 +++++---- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/flow.c b/flow.c index 3713fac..f298bd0 100644 --- a/flow.c +++ b/flow.c @@ -72,7 +72,7 @@ union flow *flow_alloc_commit(const union flow *pflow, enum flow_type type) * @c: Execution context * @hole: Pointer to recently closed flow */ -void flow_table_compact(const struct ctx *c, union flow *hole) +static void flow_table_compact(const struct ctx *c, union flow *hole) { union flow *from; @@ -122,18 +122,23 @@ void flow_defer_handler(const struct ctx *c, const struct timespec *now) } for (flow = flowtab + flow_count - 1; flow >= flowtab; flow--) { + bool closed = false; + switch (flow->f.type) { case FLOW_TCP: - tcp_flow_defer(c, flow); + closed = tcp_flow_defer(flow); break; case FLOW_TCP_SPLICE: - tcp_splice_flow_defer(c, flow); - if (timer) + closed = tcp_splice_flow_defer(flow); + if (!closed && timer) tcp_splice_timer(c, flow); break; default: /* Assume other flow types don't need any handling */ ; } + + if (closed) + flow_table_compact(c, flow); } } diff --git a/flow.h b/flow.h index 44058bf..8064f0e 100644 --- a/flow.h +++ b/flow.h @@ -68,7 +68,6 @@ static inline bool flow_sidx_eq(flow_sidx_t a, flow_sidx_t b) union flow; -void flow_table_compact(const struct ctx *c, union flow *hole); void flow_defer_handler(const struct ctx *c, const struct timespec *now); void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...) diff --git a/tcp.c b/tcp.c index 77fe2d1..0071654 100644 --- a/tcp.c +++ b/tcp.c @@ -1303,21 +1303,22 @@ static struct tcp_tap_conn *tcp_hash_lookup(const struct ctx *c, /** * tcp_flow_defer() - Deferred per-flow handling (clean up closed connections) - * @c: Execution context * @flow: Flow table entry for this connection + * + * Return: true if the flow is ready to free, false otherwise */ -void tcp_flow_defer(const struct ctx *c, union flow *flow) +bool tcp_flow_defer(union flow *flow) { const struct tcp_tap_conn *conn = &flow->tcp; if (flow->tcp.events != CLOSED) - return; + return false; close(conn->sock); if (conn->timer != -1) close(conn->timer); - flow_table_compact(c, flow); + return true; } static void tcp_rst_do(struct ctx *c, struct tcp_tap_conn *conn); diff --git a/tcp_conn.h b/tcp_conn.h index 825155a..636224e 100644 --- a/tcp_conn.h +++ b/tcp_conn.h @@ -158,8 +158,8 @@ extern int init_sock_pool6 [TCP_SOCK_POOL_SIZE]; void tcp_tap_conn_update(const struct ctx *c, struct tcp_tap_conn *old, struct tcp_tap_conn *new); void tcp_splice_conn_update(const struct ctx *c, struct tcp_splice_conn *new); -void tcp_flow_defer(const struct ctx *c, union flow *flow); -void tcp_splice_flow_defer(const struct ctx *c, union flow *flow); +bool tcp_flow_defer(union flow *flow); +bool tcp_splice_flow_defer(union flow *flow); void tcp_splice_timer(const struct ctx *c, union flow *flow); int tcp_conn_pool_sock(int pool[]); int tcp_conn_new_sock(const struct ctx *c, sa_family_t af); diff --git a/tcp_splice.c b/tcp_splice.c index 869ab4b..571228f 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -244,16 +244,17 @@ void tcp_splice_conn_update(const struct ctx *c, struct tcp_splice_conn *new) /** * tcp_splice_flow_defer() - Deferred per-flow handling (clean up closed) - * @c: Execution context * @flow: Flow table entry for this connection + * + * Return: true if the flow is ready to free, false otherwise */ -void tcp_splice_flow_defer(const struct ctx *c, union flow *flow) +bool tcp_splice_flow_defer(union flow *flow) { struct tcp_splice_conn *conn = &flow->tcp_splice; unsigned side; if (!(flow->tcp_splice.flags & CLOSING)) - return; + return false; for (side = 0; side < SIDES; side++) { if (conn->events & SPLICE_ESTABLISHED) { @@ -277,7 +278,7 @@ void tcp_splice_flow_defer(const struct ctx *c, union flow *flow) conn->flags = 0; flow_dbg(conn, "CLOSED"); - flow_table_compact(c, flow); + return true; } /** -- 2.43.0