public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH 0/4] Assorted small fix ups
@ 2025-02-18  8:59 David Gibson
  2025-02-18  8:59 ` [PATCH 1/4] tcp: More type safety for tcp_flow_migrate_target_ext() David Gibson
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: David Gibson @ 2025-02-18  8:59 UTC (permalink / raw)
  To: Stefano Brivio, passt-dev; +Cc: David Gibson

Here are several small cleanups.  3 of the 4 are related to the
recently merged migration code, one is just general.

David Gibson (4):
  tcp: More type safety for tcp_flow_migrate_target_ext()
  tcp: Remove spurious prototype for tcp_flow_migrate_shrink_window
  tcp: Don't pass both flow pointer and flow index
  flow: Add flow_perror() helper

 flow.c       | 16 +++++++++-------
 flow.h       | 18 ++++++++++++++----
 icmp.c       |  5 ++---
 tcp.c        | 46 ++++++++++++++++++++--------------------------
 tcp_conn.h   |  6 ++----
 tcp_splice.c |  9 ++++-----
 udp_flow.c   | 19 +++++++------------
 7 files changed, 58 insertions(+), 61 deletions(-)

-- 
2.48.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] tcp: More type safety for tcp_flow_migrate_target_ext()
  2025-02-18  8:59 [PATCH 0/4] Assorted small fix ups David Gibson
@ 2025-02-18  8:59 ` David Gibson
  2025-02-18  8:59 ` [PATCH 2/4] tcp: Remove spurious prototype for tcp_flow_migrate_shrink_window David Gibson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: David Gibson @ 2025-02-18  8:59 UTC (permalink / raw)
  To: Stefano Brivio, passt-dev; +Cc: David Gibson

tcp_flow_migrate_target_ext() takes a raw union flow *, although it is TCP
specific, and requires a FLOW_TYPE_TCP entry.  Our usual convention is that
such functions should take a struct tcp_tap_conn * instead.  Convert it to
do so.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 flow.c     | 2 +-
 tcp.c      | 7 +++----
 tcp_conn.h | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/flow.c b/flow.c
index cc881e86..abe95b25 100644
--- a/flow.c
+++ b/flow.c
@@ -1106,7 +1106,7 @@ int flow_migrate_target(struct ctx *c, const struct migrate_stage *stage,
 	repair_flush(c);
 
 	for (i = 0; i < count; i++) {
-		rc = tcp_flow_migrate_target_ext(c, flowtab + i, fd);
+		rc = tcp_flow_migrate_target_ext(c, &flowtab[i].tcp, fd);
 		if (rc) {
 			debug("Migration data failure at flow %u: %s, abort",
 			      i, strerror_(-rc));
diff --git a/tcp.c b/tcp.c
index 98e1c6a1..272e4cd5 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3394,14 +3394,13 @@ int tcp_flow_migrate_target(struct ctx *c, int fd)
 /**
  * tcp_flow_migrate_target_ext() - Receive extended data for flow, set, connect
  * @c:		Execution context
- * @flow:	Existing flow for this connection data
+ * @conn:	Connection entry to complete with extra data
  * @fd:		Descriptor for state migration
  *
  * Return: 0 on success, negative on fatal failure, but 0 on single flow failure
  */
-int tcp_flow_migrate_target_ext(struct ctx *c, union flow *flow, int fd)
+int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd)
 {
-	struct tcp_tap_conn *conn = &flow->tcp;
 	uint32_t peek_offset = conn->seq_to_tap - conn->seq_ack_from_tap;
 	struct tcp_tap_transfer_ext t;
 	int s = conn->sock, rc;
@@ -3413,7 +3412,7 @@ int tcp_flow_migrate_target_ext(struct ctx *c, union flow *flow, int fd)
 	}
 
 	if (!t.tcpi_state) { /* Source wants us to skip this flow */
-		flow_err(flow, "Dropping as requested by source");
+		flow_err(conn, "Dropping as requested by source");
 		goto fail;
 	}
 
diff --git a/tcp_conn.h b/tcp_conn.h
index 42dff481..53887c0e 100644
--- a/tcp_conn.h
+++ b/tcp_conn.h
@@ -239,7 +239,7 @@ int tcp_flow_migrate_source_ext(int fd, int fidx,
 				const struct tcp_tap_conn *conn);
 
 int tcp_flow_migrate_target(struct ctx *c, int fd);
-int tcp_flow_migrate_target_ext(struct ctx *c, union flow *flow, int fd);
+int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd);
 
 bool tcp_flow_is_established(const struct tcp_tap_conn *conn);
 
-- 
@@ -239,7 +239,7 @@ int tcp_flow_migrate_source_ext(int fd, int fidx,
 				const struct tcp_tap_conn *conn);
 
 int tcp_flow_migrate_target(struct ctx *c, int fd);
-int tcp_flow_migrate_target_ext(struct ctx *c, union flow *flow, int fd);
+int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd);
 
 bool tcp_flow_is_established(const struct tcp_tap_conn *conn);
 
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/4] tcp: Remove spurious prototype for tcp_flow_migrate_shrink_window
  2025-02-18  8:59 [PATCH 0/4] Assorted small fix ups David Gibson
  2025-02-18  8:59 ` [PATCH 1/4] tcp: More type safety for tcp_flow_migrate_target_ext() David Gibson
@ 2025-02-18  8:59 ` David Gibson
  2025-02-18  8:59 ` [PATCH 3/4] tcp: Don't pass both flow pointer and flow index David Gibson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: David Gibson @ 2025-02-18  8:59 UTC (permalink / raw)
  To: Stefano Brivio, passt-dev; +Cc: David Gibson

This function existed in drafts of the migration code, but not the final
version.  Get rid of the prototype.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp_conn.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tcp_conn.h b/tcp_conn.h
index 53887c0e..8a15b08d 100644
--- a/tcp_conn.h
+++ b/tcp_conn.h
@@ -233,7 +233,6 @@ bool tcp_flow_defer(const struct tcp_tap_conn *conn);
 int tcp_flow_repair_on(struct ctx *c, const struct tcp_tap_conn *conn);
 int tcp_flow_repair_off(struct ctx *c, const struct tcp_tap_conn *conn);
 
-int tcp_flow_migrate_shrink_window(int fidx, const struct tcp_tap_conn *conn);
 int tcp_flow_migrate_source(int fd, struct tcp_tap_conn *conn);
 int tcp_flow_migrate_source_ext(int fd, int fidx,
 				const struct tcp_tap_conn *conn);
-- 
@@ -233,7 +233,6 @@ bool tcp_flow_defer(const struct tcp_tap_conn *conn);
 int tcp_flow_repair_on(struct ctx *c, const struct tcp_tap_conn *conn);
 int tcp_flow_repair_off(struct ctx *c, const struct tcp_tap_conn *conn);
 
-int tcp_flow_migrate_shrink_window(int fidx, const struct tcp_tap_conn *conn);
 int tcp_flow_migrate_source(int fd, struct tcp_tap_conn *conn);
 int tcp_flow_migrate_source_ext(int fd, int fidx,
 				const struct tcp_tap_conn *conn);
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/4] tcp: Don't pass both flow pointer and flow index
  2025-02-18  8:59 [PATCH 0/4] Assorted small fix ups David Gibson
  2025-02-18  8:59 ` [PATCH 1/4] tcp: More type safety for tcp_flow_migrate_target_ext() David Gibson
  2025-02-18  8:59 ` [PATCH 2/4] tcp: Remove spurious prototype for tcp_flow_migrate_shrink_window David Gibson
@ 2025-02-18  8:59 ` David Gibson
  2025-02-18 15:30   ` Stefano Brivio
  2025-02-18  8:59 ` [PATCH 4/4] flow: Add flow_perror() helper David Gibson
  2025-02-18 15:31 ` [PATCH 0/4] Assorted small fix ups Stefano Brivio
  4 siblings, 1 reply; 7+ messages in thread
From: David Gibson @ 2025-02-18  8:59 UTC (permalink / raw)
  To: Stefano Brivio, passt-dev; +Cc: David Gibson

tcp_flow_migrate_source_ext() is passed both the index of the flow it
operates on and the pointer to the connection structure.  However, the
former is trivially derived from the latter.  Simplify the interface.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 flow.c     | 2 +-
 tcp.c      | 6 ++----
 tcp_conn.h | 3 +--
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/flow.c b/flow.c
index abe95b25..cc393e06 100644
--- a/flow.c
+++ b/flow.c
@@ -1053,7 +1053,7 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage,
 	 * as EIO).
 	 */
 	foreach_established_tcp_flow(i, flow, FLOW_MAX) {
-		rc = tcp_flow_migrate_source_ext(fd, i, &flow->tcp);
+		rc = tcp_flow_migrate_source_ext(fd, &flow->tcp);
 		if (rc) {
 			err("Extended data for flow %u: %s", i, strerror_(-rc));
 
diff --git a/tcp.c b/tcp.c
index 272e4cd5..21b6c6c5 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3141,16 +3141,14 @@ int tcp_flow_migrate_source(int fd, struct tcp_tap_conn *conn)
 /**
  * tcp_flow_migrate_source_ext() - Dump queues, close sockets, send final data
  * @fd:		Descriptor for state migration
- * @fidx:	Flow index
  * @conn:	Pointer to the TCP connection structure
  *
  * Return: 0 on success, negative (not -EIO) on failure, -EIO on sending failure
  */
-int tcp_flow_migrate_source_ext(int fd, int fidx,
-				const struct tcp_tap_conn *conn)
+int tcp_flow_migrate_source_ext(int fd, const struct tcp_tap_conn *conn)
 {
 	uint32_t peek_offset = conn->seq_to_tap - conn->seq_ack_from_tap;
-	struct tcp_tap_transfer_ext *t = &migrate_ext[fidx];
+	struct tcp_tap_transfer_ext *t = &migrate_ext[FLOW_IDX(conn)];
 	int s = conn->sock;
 	int rc;
 
diff --git a/tcp_conn.h b/tcp_conn.h
index 8a15b08d..9126a36f 100644
--- a/tcp_conn.h
+++ b/tcp_conn.h
@@ -234,8 +234,7 @@ int tcp_flow_repair_on(struct ctx *c, const struct tcp_tap_conn *conn);
 int tcp_flow_repair_off(struct ctx *c, const struct tcp_tap_conn *conn);
 
 int tcp_flow_migrate_source(int fd, struct tcp_tap_conn *conn);
-int tcp_flow_migrate_source_ext(int fd, int fidx,
-				const struct tcp_tap_conn *conn);
+int tcp_flow_migrate_source_ext(int fd, const struct tcp_tap_conn *conn);
 
 int tcp_flow_migrate_target(struct ctx *c, int fd);
 int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd);
-- 
@@ -234,8 +234,7 @@ int tcp_flow_repair_on(struct ctx *c, const struct tcp_tap_conn *conn);
 int tcp_flow_repair_off(struct ctx *c, const struct tcp_tap_conn *conn);
 
 int tcp_flow_migrate_source(int fd, struct tcp_tap_conn *conn);
-int tcp_flow_migrate_source_ext(int fd, int fidx,
-				const struct tcp_tap_conn *conn);
+int tcp_flow_migrate_source_ext(int fd, const struct tcp_tap_conn *conn);
 
 int tcp_flow_migrate_target(struct ctx *c, int fd);
 int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd);
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/4] flow: Add flow_perror() helper
  2025-02-18  8:59 [PATCH 0/4] Assorted small fix ups David Gibson
                   ` (2 preceding siblings ...)
  2025-02-18  8:59 ` [PATCH 3/4] tcp: Don't pass both flow pointer and flow index David Gibson
@ 2025-02-18  8:59 ` David Gibson
  2025-02-18 15:31 ` [PATCH 0/4] Assorted small fix ups Stefano Brivio
  4 siblings, 0 replies; 7+ messages in thread
From: David Gibson @ 2025-02-18  8:59 UTC (permalink / raw)
  To: Stefano Brivio, passt-dev; +Cc: David Gibson

Our general logging helpers include a number of _perror() variants which,
like perror(3) include the description of the current errno.  We didn't
have those for our flow specific logging helpers, though.  Fill this gap
with flow_perror() and flow_dbg_perror(), and use them where it's useful.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 flow.c       | 12 +++++++-----
 flow.h       | 18 ++++++++++++++----
 icmp.c       |  5 ++---
 tcp.c        | 33 +++++++++++++++------------------
 tcp_splice.c |  9 ++++-----
 udp_flow.c   | 19 +++++++------------
 6 files changed, 49 insertions(+), 47 deletions(-)

diff --git a/flow.c b/flow.c
index cc393e06..c68f6bb1 100644
--- a/flow.c
+++ b/flow.c
@@ -289,11 +289,13 @@ int flowside_connect(const struct ctx *c, int s,
 
 /** flow_log_ - Log flow-related message
  * @f:		flow the message is related to
+ * @newline:	Append newline at the end of the message, if missing
  * @pri:	Log priority
  * @fmt:	Format string
  * @...:	printf-arguments
  */
-void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...)
+void flow_log_(const struct flow_common *f, bool newline, int pri,
+	       const char *fmt, ...)
 {
 	const char *type_or_state;
 	char msg[BUFSIZ];
@@ -309,7 +311,7 @@ void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...)
 	else
 		type_or_state = FLOW_TYPE(f);
 
-	logmsg(true, false, pri,
+	logmsg(newline, false, pri,
 	       "Flow %u (%s): %s", flow_idx(f), type_or_state, msg);
 }
 
@@ -329,7 +331,7 @@ void flow_log_details_(const struct flow_common *f, int pri,
 	const struct flowside *tgt = &f->side[TGTSIDE];
 
 	if (state >= FLOW_STATE_TGT)
-		flow_log_(f, pri,
+		flow_log_(f, true, pri,
 			  "%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu",
 			  pif_name(f->pif[INISIDE]),
 			  inany_ntop(&ini->eaddr, estr0, sizeof(estr0)),
@@ -342,7 +344,7 @@ void flow_log_details_(const struct flow_common *f, int pri,
 			  inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)),
 			  tgt->eport);
 	else if (state >= FLOW_STATE_INI)
-		flow_log_(f, pri, "%s [%s]:%hu -> [%s]:%hu => ?",
+		flow_log_(f, true, pri, "%s [%s]:%hu -> [%s]:%hu => ?",
 			  pif_name(f->pif[INISIDE]),
 			  inany_ntop(&ini->eaddr, estr0, sizeof(estr0)),
 			  ini->eport,
@@ -363,7 +365,7 @@ static void flow_set_state(struct flow_common *f, enum flow_state state)
 	ASSERT(oldstate < FLOW_NUM_STATES);
 
 	f->state = state;
-	flow_log_(f, LOG_DEBUG, "%s -> %s", flow_state_str[oldstate],
+	flow_log_(f, true, LOG_DEBUG, "%s -> %s", flow_state_str[oldstate],
 		  FLOW_STATE(f));
 
 	flow_log_details_(f, LOG_DEBUG, MAX(state, oldstate));
diff --git a/flow.h b/flow.h
index 675726eb..dcf7645a 100644
--- a/flow.h
+++ b/flow.h
@@ -258,11 +258,11 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage,
 int flow_migrate_target(struct ctx *c, const struct migrate_stage *stage,
 			int fd);
 
-void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...)
-	__attribute__((format(printf, 3, 4)));
-
-#define flow_log(f_, pri, ...)	flow_log_(&(f_)->f, (pri), __VA_ARGS__)
+void flow_log_(const struct flow_common *f, bool newline, int pri,
+	       const char *fmt, ...)
+	__attribute__((format(printf, 4, 5)));
 
+#define flow_log(f_, pri, ...)	flow_log_(&(f_)->f, true, (pri), __VA_ARGS__)
 #define flow_dbg(f, ...)	flow_log((f), LOG_DEBUG, __VA_ARGS__)
 #define flow_err(f, ...)	flow_log((f), LOG_ERR, __VA_ARGS__)
 
@@ -272,6 +272,16 @@ void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...)
 			flow_dbg((f), __VA_ARGS__);			\
 	} while (0)
 
+#define flow_log_perror_(f, pri, ...)					\
+	do {								\
+		int errno_ = errno;					\
+		flow_log_((f), false, (pri), __VA_ARGS__);		\
+		logmsg(true, true, (pri), ": %s", strerror_(errno_));	\
+	} while (0)
+
+#define flow_dbg_perror(f_, ...) flow_log_perror_(&(f_)->f, LOG_DEBUG, __VA_ARGS__)
+#define flow_perror(f_, ...)	flow_log_perror_(&(f_)->f, LOG_ERR, __VA_ARGS__)
+
 void flow_log_details_(const struct flow_common *f, int pri,
 		       enum flow_state state);
 #define flow_log_details(f_, pri) \
diff --git a/icmp.c b/icmp.c
index bcf498d2..7e2b3423 100644
--- a/icmp.c
+++ b/icmp.c
@@ -85,7 +85,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref)
 
 	n = recvfrom(ref.fd, buf, sizeof(buf), 0, &sr.sa, &sl);
 	if (n < 0) {
-		flow_err(pingf, "recvfrom() error: %s", strerror_(errno));
+		flow_perror(pingf, "recvfrom() error");
 		return;
 	}
 
@@ -300,8 +300,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 
 	pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0);
 	if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
-		flow_dbg(pingf, "failed to relay request to socket: %s",
-			 strerror_(errno));
+		flow_dbg_perror(pingf, "failed to relay request to socket");
 	} else {
 		flow_dbg(pingf,
 			 "echo request to socket, ID: %"PRIu16", seq: %"PRIu16,
diff --git a/tcp.c b/tcp.c
index 21b6c6c5..f498f5bc 100644
--- a/tcp.c
+++ b/tcp.c
@@ -551,8 +551,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
 
 		fd = timerfd_create(CLOCK_MONOTONIC, 0);
 		if (fd == -1 || fd > FD_REF_MAX) {
-			flow_dbg(conn, "failed to get timer: %s",
-				 strerror_(errno));
+			flow_dbg_perror(conn, "failed to get timer");
 			if (fd > -1)
 				close(fd);
 			conn->timer = -1;
@@ -561,8 +560,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
 		conn->timer = fd;
 
 		if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, conn->timer, &ev)) {
-			flow_dbg(conn, "failed to add timer: %s",
-				 strerror_(errno));
+			flow_dbg_perror(conn, "failed to add timer");
 			close(conn->timer);
 			conn->timer = -1;
 			return;
@@ -587,7 +585,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
 		 (unsigned long long)it.it_value.tv_nsec / 1000 / 1000);
 
 	if (timerfd_settime(conn->timer, 0, &it, NULL))
-		flow_err(conn, "failed to set timer: %s", strerror_(errno));
+		flow_perror(conn, "failed to set timer");
 }
 
 /**
@@ -1386,10 +1384,10 @@ static void tcp_bind_outbound(const struct ctx *c,
 		if (bind(s, &bind_sa.sa, sl)) {
 			char sstr[INANY_ADDRSTRLEN];
 
-			flow_dbg(conn,
-				 "Can't bind TCP outbound socket to %s:%hu: %s",
-				 inany_ntop(&tgt->oaddr, sstr, sizeof(sstr)),
-				 tgt->oport, strerror_(errno));
+			flow_dbg_perror(conn,
+					"Can't bind TCP outbound socket to %s:%hu",
+					inany_ntop(&tgt->oaddr, sstr, sizeof(sstr)),
+					tgt->oport);
 		}
 	}
 
@@ -1398,9 +1396,9 @@ static void tcp_bind_outbound(const struct ctx *c,
 			if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE,
 				       c->ip4.ifname_out,
 				       strlen(c->ip4.ifname_out))) {
-				flow_dbg(conn, "Can't bind IPv4 TCP socket to"
-					 " interface %s: %s", c->ip4.ifname_out,
-					 strerror_(errno));
+				flow_dbg_perror(conn,
+						"Can't bind IPv4 TCP socket to interface %s",
+						c->ip4.ifname_out);
 			}
 		}
 	} else if (bind_sa.sa_family == AF_INET6) {
@@ -1408,9 +1406,9 @@ static void tcp_bind_outbound(const struct ctx *c,
 			if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE,
 				       c->ip6.ifname_out,
 				       strlen(c->ip6.ifname_out))) {
-				flow_dbg(conn, "Can't bind IPv6 TCP socket to"
-					 " interface %s: %s", c->ip6.ifname_out,
-					 strerror_(errno));
+				flow_dbg_perror(conn,
+						"Can't bind IPv6 TCP socket to interface %s",
+						c->ip6.ifname_out);
 			}
 		}
 	}
@@ -2193,7 +2191,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
 	 * and we just set the timer to a new point in the future: discard it.
 	 */
 	if (timerfd_gettime(conn->timer, &check_armed))
-		flow_err(conn, "failed to read timer: %s", strerror_(errno));
+		flow_perror(conn, "failed to read timer");
 
 	if (check_armed.it_value.tv_sec || check_armed.it_value.tv_nsec)
 		return;
@@ -2235,8 +2233,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
 		 * ~ACK_TO_TAP_DUE or ~ACK_FROM_TAP_DUE.
 		 */
 		if (timerfd_settime(conn->timer, 0, &new, &old))
-			flow_err(conn, "failed to set timer: %s",
-				 strerror_(errno));
+			flow_perror(conn, "failed to set timer");
 
 		if (old.it_value.tv_sec == ACT_TIMEOUT) {
 			flow_dbg(conn, "activity timeout");
diff --git a/tcp_splice.c b/tcp_splice.c
index 5d845c92..0d10e3d4 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -164,7 +164,7 @@ static int tcp_splice_epoll_ctl(const struct ctx *c,
 	if (epoll_ctl(c->epollfd, m, conn->s[0], &ev[0]) ||
 	    epoll_ctl(c->epollfd, m, conn->s[1], &ev[1])) {
 		int ret = -errno;
-		flow_err(conn, "ERROR on epoll_ctl(): %s", strerror_(errno));
+		flow_perror(conn, "ERROR on epoll_ctl()");
 		return ret;
 	}
 
@@ -317,8 +317,8 @@ static int tcp_splice_connect_finish(const struct ctx *c,
 
 		if (conn->pipe[sidei][0] < 0) {
 			if (pipe2(conn->pipe[sidei], O_NONBLOCK | O_CLOEXEC)) {
-				flow_err(conn, "cannot create %d->%d pipe: %s",
-					 sidei, !sidei, strerror_(errno));
+				flow_perror(conn, "cannot create %d->%d pipe",
+					    sidei, !sidei);
 				conn_flag(c, conn, CLOSING);
 				return -EIO;
 			}
@@ -482,8 +482,7 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
 
 		rc = getsockopt(ref.fd, SOL_SOCKET, SO_ERROR, &err, &sl);
 		if (rc)
-			flow_err(conn, "Error retrieving SO_ERROR: %s",
-				 strerror_(errno));
+			flow_perror(conn, "Error retrieving SO_ERROR");
 		else
 			flow_trace(conn, "Error event on socket: %s",
 				   strerror_(err));
diff --git a/udp_flow.c b/udp_flow.c
index 83c2568a..c6b8630a 100644
--- a/udp_flow.c
+++ b/udp_flow.c
@@ -93,9 +93,8 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow,
 		 */
 		uflow->s[INISIDE] = fcntl(s_ini, F_DUPFD_CLOEXEC, 0);
 		if (uflow->s[INISIDE] < 0) {
-			flow_err(uflow,
-				 "Couldn't duplicate listening socket: %s",
-				 strerror_(errno));
+			flow_perror(uflow,
+				    "Couldn't duplicate listening socket");
 			goto cancel;
 		}
 	}
@@ -113,16 +112,13 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow,
 		uflow->s[TGTSIDE] = flowside_sock_l4(c, EPOLL_TYPE_UDP_REPLY,
 						     tgtpif, tgt, fref.data);
 		if (uflow->s[TGTSIDE] < 0) {
-			flow_dbg(uflow,
-				 "Couldn't open socket for spliced flow: %s",
-				 strerror_(errno));
+			flow_dbg_perror(uflow,
+					"Couldn't open socket for spliced flow");
 			goto cancel;
 		}
 
 		if (flowside_connect(c, uflow->s[TGTSIDE], tgtpif, tgt) < 0) {
-			flow_dbg(uflow,
-				 "Couldn't connect flow socket: %s",
-				 strerror_(errno));
+			flow_dbg_perror(uflow, "Couldn't connect flow socket");
 			goto cancel;
 		}
 
@@ -142,9 +138,8 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow,
 			flow_trace(uflow,
 				   "Discarded %d spurious reply datagrams", rc);
 		} else if (errno != EAGAIN) {
-			flow_err(uflow,
-				 "Unexpected error discarding datagrams: %s",
-				 strerror_(errno));
+			flow_perror(uflow,
+				    "Unexpected error discarding datagrams");
 		}
 	}
 
-- 
@@ -93,9 +93,8 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow,
 		 */
 		uflow->s[INISIDE] = fcntl(s_ini, F_DUPFD_CLOEXEC, 0);
 		if (uflow->s[INISIDE] < 0) {
-			flow_err(uflow,
-				 "Couldn't duplicate listening socket: %s",
-				 strerror_(errno));
+			flow_perror(uflow,
+				    "Couldn't duplicate listening socket");
 			goto cancel;
 		}
 	}
@@ -113,16 +112,13 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow,
 		uflow->s[TGTSIDE] = flowside_sock_l4(c, EPOLL_TYPE_UDP_REPLY,
 						     tgtpif, tgt, fref.data);
 		if (uflow->s[TGTSIDE] < 0) {
-			flow_dbg(uflow,
-				 "Couldn't open socket for spliced flow: %s",
-				 strerror_(errno));
+			flow_dbg_perror(uflow,
+					"Couldn't open socket for spliced flow");
 			goto cancel;
 		}
 
 		if (flowside_connect(c, uflow->s[TGTSIDE], tgtpif, tgt) < 0) {
-			flow_dbg(uflow,
-				 "Couldn't connect flow socket: %s",
-				 strerror_(errno));
+			flow_dbg_perror(uflow, "Couldn't connect flow socket");
 			goto cancel;
 		}
 
@@ -142,9 +138,8 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow,
 			flow_trace(uflow,
 				   "Discarded %d spurious reply datagrams", rc);
 		} else if (errno != EAGAIN) {
-			flow_err(uflow,
-				 "Unexpected error discarding datagrams: %s",
-				 strerror_(errno));
+			flow_perror(uflow,
+				    "Unexpected error discarding datagrams");
 		}
 	}
 
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/4] tcp: Don't pass both flow pointer and flow index
  2025-02-18  8:59 ` [PATCH 3/4] tcp: Don't pass both flow pointer and flow index David Gibson
@ 2025-02-18 15:30   ` Stefano Brivio
  0 siblings, 0 replies; 7+ messages in thread
From: Stefano Brivio @ 2025-02-18 15:30 UTC (permalink / raw)
  To: David Gibson; +Cc: passt-dev

On Tue, 18 Feb 2025 19:59:23 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> -	struct tcp_tap_transfer_ext *t = &migrate_ext[fidx];
> +	struct tcp_tap_transfer_ext *t = &migrate_ext[FLOW_IDX(conn)];

Ah, right, of course, that's how it's done!

-- 
Stefano


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/4] Assorted small fix ups
  2025-02-18  8:59 [PATCH 0/4] Assorted small fix ups David Gibson
                   ` (3 preceding siblings ...)
  2025-02-18  8:59 ` [PATCH 4/4] flow: Add flow_perror() helper David Gibson
@ 2025-02-18 15:31 ` Stefano Brivio
  4 siblings, 0 replies; 7+ messages in thread
From: Stefano Brivio @ 2025-02-18 15:31 UTC (permalink / raw)
  To: David Gibson; +Cc: passt-dev

On Tue, 18 Feb 2025 19:59:20 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Here are several small cleanups.  3 of the 4 are related to the
> recently merged migration code, one is just general.
> 
> David Gibson (4):
>   tcp: More type safety for tcp_flow_migrate_target_ext()
>   tcp: Remove spurious prototype for tcp_flow_migrate_shrink_window
>   tcp: Don't pass both flow pointer and flow index
>   flow: Add flow_perror() helper

Applied.

-- 
Stefano


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-02-18 15:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-18  8:59 [PATCH 0/4] Assorted small fix ups David Gibson
2025-02-18  8:59 ` [PATCH 1/4] tcp: More type safety for tcp_flow_migrate_target_ext() David Gibson
2025-02-18  8:59 ` [PATCH 2/4] tcp: Remove spurious prototype for tcp_flow_migrate_shrink_window David Gibson
2025-02-18  8:59 ` [PATCH 3/4] tcp: Don't pass both flow pointer and flow index David Gibson
2025-02-18 15:30   ` Stefano Brivio
2025-02-18  8:59 ` [PATCH 4/4] flow: Add flow_perror() helper David Gibson
2025-02-18 15:31 ` [PATCH 0/4] Assorted small fix ups Stefano Brivio

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).