public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 07/16] tcp_splice: More specific variable names in new splice path
Date: Mon, 29 Jan 2024 15:35:48 +1100	[thread overview]
Message-ID: <20240129043557.823451-8-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240129043557.823451-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 16be0c59..bdabe411 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
  * @conn:	connection structure to initialize
- * @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,25 +416,25 @@ 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,
-			       struct tcp_splice_conn *conn, int s,
+			       struct tcp_splice_conn *conn, int s0,
 			       const struct sockaddr *sa)
 {
-	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->flags = inany_v4(&aany) ? 0 : SPLICE_V6;
+	conn->flags = inany_v4(&src) ? 0 : SPLICE_V6;
 
-	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);
 
 	conn->f.type = FLOW_TCP_SPLICE;
-	conn->s[0] = s;
+	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;
diff --git a/tcp_splice.h b/tcp_splice.h
index 18193e4e..d3600807 100644
--- a/tcp_splice.h
+++ b/tcp_splice.h
@@ -12,7 +12,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,
-			       struct tcp_splice_conn *conn, int s,
+			       struct tcp_splice_conn *conn, int s0,
 			       const struct sockaddr *sa);
 void tcp_splice_init(struct ctx *c);
 
-- 
@@ -12,7 +12,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,
-			       struct tcp_splice_conn *conn, int s,
+			       struct tcp_splice_conn *conn, int s0,
 			       const struct sockaddr *sa);
 void tcp_splice_init(struct ctx *c);
 
-- 
2.43.0


  parent reply	other threads:[~2024-01-29  4:36 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-29  4:35 [PATCH 00/16] More flow table preliminaries: address handling improvements David Gibson
2024-01-29  4:35 ` [PATCH 01/16] treewide: Use sa_family_t for address family variables David Gibson
2024-01-29  4:35 ` [PATCH 02/16] tcp, udp: Don't precompute port remappings in epoll references David Gibson
2024-01-29  4:35 ` [PATCH 03/16] flow: Add helper to determine a flow's protocol David Gibson
2024-01-29  4:35 ` [PATCH 04/16] tcp_splice: Simplify clean up logic David Gibson
2024-01-29  4:35 ` [PATCH 05/16] inany: Helper to test for IPv4 or IPv6 loopback address David Gibson
2024-01-29  4:35 ` [PATCH 06/16] tcp, tcp_splice: Helpers for getting sockets from the pools David Gibson
2024-01-29  4:35 ` David Gibson [this message]
2024-01-29  4:35 ` [PATCH 08/16] tcp_splice: Fix incorrect parameter comment for tcp_splice_connect() David Gibson
2024-01-29  4:35 ` [PATCH 09/16] tcp_splice: Merge tcp_splice_new() into its caller David Gibson
2024-01-29  4:35 ` [PATCH 10/16] tcp_splice: Improve error reporting on connect path David Gibson
2024-01-29  4:35 ` [PATCH 11/16] inany: Add inany_ntop() helper David Gibson
2024-01-29  4:35 ` [PATCH 12/16] tcp_splice: Improve logic deciding when to splice David Gibson
2024-01-29  4:35 ` [PATCH 13/16] util: Provide global constants for IPv4 loopback and unspecified address David Gibson
2024-01-29  4:35 ` [PATCH 14/16] inany: Introduce union sockaddr_inany David Gibson
2024-01-29  4:35 ` [PATCH 15/16] tcp, tcp_splice: Better construction of IPv4 or IPv6 sockaddrs David Gibson
2024-01-29  4:35 ` [PATCH 16/16] inany: Extend inany_from_af to easily set unspecified addresses David Gibson
2024-01-29  9:02 ` [PATCH 00/16] More flow table preliminaries: address handling improvements David Gibson

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=20240129043557.823451-8-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).