From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 474C65A0278 for ; Sun, 18 Feb 2024 22:00:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708290015; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bZRiRjLAIpw0uGbNQMYzpn47aGF2ZWchh/GKnvs5MLU=; b=bJYzeE5OS/OzqwrZ7ptTybf1eI1xScAcnnbtFLJyZa4HCDHtRBabEfh483g9hHWdMTZ3zs O/A7Uzdi7cnw4ctwIf9mRJy4+clw+pbBKMRVU3/R82ADy0nT2keJONKlK166PUn+bEdfGX HtG3s+dBLjA/0A3eLC/8kxZ8YC2l+8A= Received: from mail-ej1-f69.google.com (mail-ej1-f69.google.com [209.85.218.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-434-8kG54xgmMFyphC3yRbqiug-1; Sun, 18 Feb 2024 16:00:13 -0500 X-MC-Unique: 8kG54xgmMFyphC3yRbqiug-1 Received: by mail-ej1-f69.google.com with SMTP id a640c23a62f3a-a3e4cea0385so38074166b.2 for ; Sun, 18 Feb 2024 13:00:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708290012; x=1708894812; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=bZRiRjLAIpw0uGbNQMYzpn47aGF2ZWchh/GKnvs5MLU=; b=ccaoXzge5IYBBfWaoIp3NizDNRFdFZQr2kudcBPguS3G+0WEZ8zPyNBfNNXXMcpQ2e TjNK8SOVl6gH9cCiT0oeK9U1SW1uxXlPVA1b+AfMmqwEw/E/OVHNVNFP1WbMJnRgyx8p QDCqOcs5dkDHVkcsVZMBQt3NmDvy9af2WPIGcVdDwWrABV25as/Z6k8mlzC8CVXrvigv sRs0StPreWiUSD9t2aEJp6RIhDfYRvjmYhTxVti+J36/+7xDVXdfbiO4X8SPjLtgFfqu c/XiiHT2OZTXFaP2ud+5ksC88zmSwR9Jert2wJKs1YnOsZx9Xe3jujZFZ+xFDp4eT4t2 gOBw== X-Gm-Message-State: AOJu0YxQlGF5Y9+zM8blba961W6cjFmXAXMzRXSxNqLoC0KrSk3A1sou ZHSHntXHz+3V/j70iv1MkgsbzNCTeHVfvm++HpiWxllBrUX5Dc0U92GEsQnq7qI3B92xPfz385f VpIZo+UJ8yP3bYI3Ypi4xID10TIl59VhRza0ugdNtfdGWacslEQ== X-Received: by 2002:a17:906:c9c6:b0:a3e:272e:7b98 with SMTP id hk6-20020a170906c9c600b00a3e272e7b98mr2617137ejb.40.1708290011933; Sun, 18 Feb 2024 13:00:11 -0800 (PST) X-Google-Smtp-Source: AGHT+IFH4Dts0jX3CyTJGPVC4yO0dbSn47El0YU4Urmg1KSIQrc336y1yut4u/W+Ma5k8t9GRfB8+g== X-Received: by 2002:a17:906:c9c6:b0:a3e:272e:7b98 with SMTP id hk6-20020a170906c9c600b00a3e272e7b98mr2617134ejb.40.1708290011653; Sun, 18 Feb 2024 13:00:11 -0800 (PST) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id s8-20020a170906500800b00a3de4c7bf00sm2256342ejj.79.2024.02.18.13.00.10 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 18 Feb 2024 13:00:11 -0800 (PST) Date: Sun, 18 Feb 2024 21:59:37 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 09/22] tcp_splice: Simplify clean up logic Message-ID: <20240218215937.126b2839@elisabeth> In-Reply-To: <20240206011734.884138-10-david@gibson.dropbear.id.au> References: <20240206011734.884138-1-david@gibson.dropbear.id.au> <20240206011734.884138-10-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.36; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: LTKSAEZBNK3R5I6GTYQUH4SJCVSUZT53 X-Message-ID-Hash: LTKSAEZBNK3R5I6GTYQUH4SJCVSUZT53 X-MailFrom: sbrivio@redhat.com 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: passt-dev@passt.top 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: On Tue, 6 Feb 2024 12:17:21 +1100 David Gibson wrote: > 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 40ecb5d4..f0343eb5 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; Nit: a newline here would be nice. > conn_event(c, conn, SPLICE_CONNECT); > } else { > conn_event(c, conn, SPLICE_ESTABLISHED); > @@ -459,6 +451,9 @@ bool tcp_splice_conn_from_sock(const struct ctx *c, > conn->f.type = FLOW_TCP_SPLICE; > conn->flags = inany_v4(&aany) ? 0 : SPLICE_V6; > 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);