From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id E1E335A0272 for ; Mon, 29 Jan 2024 05:36:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1706502959; bh=Qb7RlU4/uGvkFy7VfGMum8/DOdslDOzsq6rFYDxuHzc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SdpvSvdIBRe4bIC/To6x4/3QoucZTtb1mdQjuzBX0VuWug3Tp7DIMXJ7GU0y9Lj6Z rFtETbsyxBQz+tBA3ogZYwAAI2ZJIarSk2I0lpmOgO6KWO852ni+ZnNz2y99j1t/+r igi6oTQyOLmpFXv+uhwaCoPUUUqtm+hhm3RzXHQjitaktMoTkakS/VvWdkXLR2eWTP 6QoARg8MiSNZEw12FLrs2w1NXpQYrfcvyURTHBjC1tLlxwtSwtI3YH30AKXc5kYyu+ G3B4VuZAEM7oAXmx4LaGk6BUwu9Kd6UpGmgoMmwEmrOyb6fjkP2xPpOKegKcRW8vfW Hqj9d6d3WXHzA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TNb8b5Y8gz4x2K; Mon, 29 Jan 2024 15:35:59 +1100 (AEDT) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH 04/16] tcp_splice: Simplify clean up logic Date: Mon, 29 Jan 2024 15:35:45 +1100 Message-ID: <20240129043557.823451-5-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240129043557.823451-1-david@gibson.dropbear.id.au> References: <20240129043557.823451-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: ROWQJDM4D5MEHVTGR65DA72APDY3Z6MA X-Message-ID-Hash: ROWQJDM4D5MEHVTGR65DA72APDY3Z6MA 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 tcp_splice_flow_defer() contains specific logic to determine if we're far enough initialised that we need to close pipes and/or sockets. This is potentially fragile if we change something about the order in which we do things. We can simplify this by initialising the pipe and socket fields to -1 very early, then close()ing them if and only if they're non negative. This lets us remove a special case cleanup if our connect() fails. This will already trigger a CLOSING event, and the socket fd in question is populated in the connection structure. Thus we can let the new cleanup logic handle it rather than requiring an explicit close(). Signed-off-by: David Gibson --- tcp_splice.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/tcp_splice.c b/tcp_splice.c index b8d64eba..76258e89 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -246,16 +246,14 @@ bool tcp_splice_flow_defer(union flow *flow) return false; for (side = 0; side < SIDES; side++) { - if (conn->events & SPLICE_ESTABLISHED) { - /* Flushing might need to block: don't recycle them. */ - if (conn->pipe[side][0] != -1) { - close(conn->pipe[side][0]); - close(conn->pipe[side][1]); - conn->pipe[side][0] = conn->pipe[side][1] = -1; - } + /* Flushing might need to block: don't recycle them. */ + if (conn->pipe[side][0] >= 0) { + close(conn->pipe[side][0]); + close(conn->pipe[side][1]); + conn->pipe[side][0] = conn->pipe[side][1] = -1; } - if (side == 0 || conn->events & SPLICE_CONNECT) { + if (conn->s[side] >= 0) { close(conn->s[side]); conn->s[side] = -1; } @@ -284,8 +282,6 @@ static int tcp_splice_connect_finish(const struct ctx *c, int i = 0; for (side = 0; side < SIDES; side++) { - conn->pipe[side][0] = conn->pipe[side][1] = -1; - for (; i < TCP_SPLICE_PIPE_POOL_SIZE; i++) { if (splice_pipe_pool[i][0] >= 0) { SWAP(conn->pipe[side][0], @@ -361,12 +357,8 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn, } if (connect(conn->s[1], sa, sl)) { - if (errno != EINPROGRESS) { - int ret = -errno; - - close(sock_conn); - return ret; - } + if (errno != EINPROGRESS) + return -errno; conn_event(c, conn, SPLICE_CONNECT); } else { conn_event(c, conn, SPLICE_ESTABLISHED); @@ -468,6 +460,9 @@ bool tcp_splice_conn_from_sock(const struct ctx *c, conn->f.type = FLOW_TCP_SPLICE; conn->s[0] = s; + conn->s[1] = -1; + conn->pipe[0][0] = conn->pipe[0][1] = -1; + conn->pipe[1][0] = conn->pipe[1][1] = -1; if (tcp_splice_new(c, conn, ref.port, ref.pif)) conn_flag(c, conn, CLOSING); -- 2.43.0