public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v2 13/22] tcp_splice: More specific variable names in new splice path
Date: Tue,  6 Feb 2024 12:17:25 +1100	[thread overview]
Message-ID: <20240206011734.884138-14-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240206011734.884138-1-david@gibson.dropbear.id.au>

In tcp_splice_conn_from_sock(), the 'port' variable stores the source port
of the connection on the originating side.  In tcp_splice_new(), called
directly from it, the 'port' parameter gives the _destination_ port of the
originating connection and is then updated to the destination port of the
connection on the other side.

Similarly, in tcp_splice_conn_from_sock(), 's' is the fd of the accetped
socket (on side 0), whereas in tcp_splice_new(), 's' is the fd of the
connecting socket (side 1).

I, for one, find having the same variable name with different meanings in
such close proximity in the flow of control pretty confusing.  Alter the
names for greater specificity and clarity.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp_splice.c | 40 ++++++++++++++++++++--------------------
 tcp_splice.h |  2 +-
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/tcp_splice.c b/tcp_splice.c
index 609f3242..d1263f21 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -378,29 +378,29 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn,
  * Return: return code from connect()
  */
 static int tcp_splice_new(const struct ctx *c, struct tcp_splice_conn *conn,
-			  in_port_t port, uint8_t pif)
+			  in_port_t dstport, uint8_t pif)
 {
 	sa_family_t af = CONN_V6(conn) ? AF_INET6 : AF_INET;
-	int s = -1;
+	int s1;
 
 	if (pif == PIF_SPLICE) {
-		port += c->tcp.fwd_out.delta[port];
+		dstport += c->tcp.fwd_out.delta[dstport];
 
-		s = tcp_conn_sock(c, af);
+		s1 = tcp_conn_sock(c, af);
 	} else {
 		ASSERT(pif == PIF_HOST);
 
-		port += c->tcp.fwd_in.delta[port];
+		dstport += c->tcp.fwd_in.delta[dstport];
 
-		s = tcp_conn_sock_ns(c, af);
+		s1 = tcp_conn_sock_ns(c, af);
 	}
 
-	if (s < 0) {
-		warn("Couldn't open connectable socket for splice (%d)", s);
-		return s;
+	if (s1 < 0) {
+		warn("Couldn't open connectable socket for splice (%d)", s1);
+		return s1;
 	}
 
-	return tcp_splice_connect(c, conn, s, port);
+	return tcp_splice_connect(c, conn, s1, dstport);
 }
 
 /**
@@ -408,7 +408,7 @@ static int tcp_splice_new(const struct ctx *c, struct tcp_splice_conn *conn,
  * @c:		Execution context
  * @ref:	epoll reference of listening socket
  * @flow:	flow to initialise
- * @s:		Accepted socket
+ * @s0:		Accepted (side 0) socket
  * @sa:		Peer address of connection
  *
  * Return: true if able to create a spliced connection, false otherwise
@@ -416,28 +416,28 @@ static int tcp_splice_new(const struct ctx *c, struct tcp_splice_conn *conn,
  */
 bool tcp_splice_conn_from_sock(const struct ctx *c,
 			       union tcp_listen_epoll_ref ref, union flow *flow,
-			       int s, const union sockaddr_inany *sa)
+			       int s0, const union sockaddr_inany *sa)
 {
 	struct tcp_splice_conn *conn;
-	union inany_addr aany;
-	in_port_t port;
+	union inany_addr src;
+	in_port_t srcport;
 
 	ASSERT(c->mode == MODE_PASTA);
 
-	inany_from_sockaddr(&aany, &port, sa);
-	if (!inany_is_loopback(&aany))
+	inany_from_sockaddr(&src, &srcport, sa);
+	if (!inany_is_loopback(&src))
 		return false;
 
 	conn = FLOW_START(flow, FLOW_TCP_SPLICE, tcp_splice, 0);
 
-	conn->flags = inany_v4(&aany) ? 0 : SPLICE_V6;
-	conn->s[0] = s;
+	conn->flags = inany_v4(&src) ? 0 : SPLICE_V6;
+	conn->s[0] = s0;
 	conn->s[1] = -1;
 	conn->pipe[0][0] = conn->pipe[0][1] = -1;
 	conn->pipe[1][0] = conn->pipe[1][1] = -1;
 
-	if (setsockopt(s, SOL_TCP, TCP_QUICKACK, &((int){ 1 }), sizeof(int)))
-		flow_trace(conn, "failed to set TCP_QUICKACK on %i", s);
+	if (setsockopt(s0, SOL_TCP, TCP_QUICKACK, &((int){ 1 }), sizeof(int)))
+		flow_trace(conn, "failed to set TCP_QUICKACK on %i", s0);
 
 	if (tcp_splice_new(c, conn, ref.port, ref.pif))
 		conn_flag(c, conn, CLOSING);
diff --git a/tcp_splice.h b/tcp_splice.h
index 5a471af0..d0c6ff79 100644
--- a/tcp_splice.h
+++ b/tcp_splice.h
@@ -13,7 +13,7 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
 			     uint32_t events);
 bool tcp_splice_conn_from_sock(const struct ctx *c,
 			       union tcp_listen_epoll_ref ref, union flow *flow,
-			       int s, const union sockaddr_inany *sa);
+			       int s0, const union sockaddr_inany *sa);
 void tcp_splice_init(struct ctx *c);
 
 #endif /* TCP_SPLICE_H */
-- 
@@ -13,7 +13,7 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
 			     uint32_t events);
 bool tcp_splice_conn_from_sock(const struct ctx *c,
 			       union tcp_listen_epoll_ref ref, union flow *flow,
-			       int s, const union sockaddr_inany *sa);
+			       int s0, const union sockaddr_inany *sa);
 void tcp_splice_init(struct ctx *c);
 
 #endif /* TCP_SPLICE_H */
-- 
2.43.0


  parent reply	other threads:[~2024-02-06  1:17 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-06  1:17 [PATCH v2 00/22] More flow table preliminaries: address handling improvements David Gibson
2024-02-06  1:17 ` [PATCH v2 01/22] treewide: Use sa_family_t for address family variables David Gibson
2024-02-06  1:17 ` [PATCH v2 02/22] inany: Helper to test for various address types David Gibson
2024-02-18 20:58   ` Stefano Brivio
2024-02-19  1:48     ` David Gibson
2024-02-06  1:17 ` [PATCH v2 03/22] inany: Add inany_ntop() helper David Gibson
2024-02-06  1:17 ` [PATCH v2 04/22] inany: Provide more conveniently typed constants for special addresses David Gibson
2024-02-06  1:17 ` [PATCH v2 05/22] inany: Introduce union sockaddr_inany David Gibson
2024-02-06  1:17 ` [PATCH v2 06/22] util: Allow IN4_IS_* macros to operate on untyped addresses David Gibson
2024-02-06  1:17 ` [PATCH v2 07/22] tcp, udp: Don't precompute port remappings in epoll references David Gibson
2024-02-06  1:17 ` [PATCH v2 08/22] flow: Add helper to determine a flow's protocol David Gibson
2024-02-06  1:17 ` [PATCH v2 09/22] tcp_splice: Simplify clean up logic David Gibson
2024-02-18 20:59   ` Stefano Brivio
2024-02-19  1:50     ` David Gibson
2024-02-06  1:17 ` [PATCH v2 10/22] tcp_splice: Don't use flow_trace() before setting flow type David Gibson
2024-02-06  1:17 ` [PATCH v2 11/22] flow: Clarify flow entry life cycle, introduce uniform logging David Gibson
2024-02-18 21:00   ` Stefano Brivio
2024-02-19  1:58     ` David Gibson
2024-02-06  1:17 ` [PATCH v2 12/22] tcp, tcp_splice: Helpers for getting sockets from the pools David Gibson
2024-02-18 21:00   ` Stefano Brivio
2024-02-19  1:51     ` David Gibson
2024-02-06  1:17 ` David Gibson [this message]
2024-02-18 21:00   ` [PATCH v2 13/22] tcp_splice: More specific variable names in new splice path Stefano Brivio
2024-02-19  1:53     ` David Gibson
2024-02-06  1:17 ` [PATCH v2 14/22] tcp_splice: Merge tcp_splice_new() into its caller David Gibson
2024-02-06  1:17 ` [PATCH v2 15/22] tcp_splice: Make tcp_splice_connect() create its own sockets David Gibson
2024-02-06  1:17 ` [PATCH v2 16/22] tcp_splice: Improve error reporting on connect path David Gibson
2024-02-18 21:01   ` Stefano Brivio
2024-02-19  3:23     ` David Gibson
2024-02-06  1:17 ` [PATCH v2 17/22] tcp_splice: Improve logic deciding when to splice David Gibson
2024-02-06  1:17 ` [PATCH v2 18/22] tcp, tcp_splice: Parse listening socket epoll ref in tcp_listen_handler() David Gibson
2024-02-06  1:17 ` [PATCH v2 19/22] tcp: Validate TCP endpoint addresses David Gibson
2024-02-22 12:45   ` Stefano Brivio
2024-02-23  3:56     ` David Gibson
2024-02-06  1:17 ` [PATCH v2 20/22] tap: Disallow loopback addresses on tap interface David Gibson
2024-02-06  1:17 ` [PATCH v2 21/22] port_fwd: Fix copypasta error in port_fwd_scan_udp() comments David Gibson
2024-02-06  1:17 ` [PATCH v2 22/22] fwd: Rename port_fwd.[ch] and their contents David Gibson
2024-02-27 14:22 ` [PATCH v2 00/22] More flow table preliminaries: address handling improvements Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240206011734.884138-14-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).