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 2/3] icmp: Flow based error reporting
Date: Thu, 29 Feb 2024 15:15:33 +1100	[thread overview]
Message-ID: <20240229041534.2573559-3-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240229041534.2573559-1-david@gibson.dropbear.id.au>

Use flow_dbg() and flow_err() helpers to generate flow-linked error
messages in most places.  Make a few small improvements to the messages
while we're at it.  This allows us to avoid the awkward 'pname' variables
since whether we're dealing with ICMP or ICMPv6 is already built into the
flow type which these helpers include.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 icmp.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/icmp.c b/icmp.c
index 1caf791d..63adafcf 100644
--- a/icmp.c
+++ b/icmp.c
@@ -61,7 +61,6 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 {
 	struct icmp_ping_flow *pingf = af == AF_INET
 		? icmp_id_map[V4][ref.icmp.id] : icmp_id_map[V6][ref.icmp.id];
-	const char *const pname = af == AF_INET ? "ICMP" : "ICMPv6";
 	union sockaddr_inany sr;
 	socklen_t sl = sizeof(sr);
 	char buf[USHRT_MAX];
@@ -75,8 +74,7 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 
 	n = recvfrom(ref.fd, buf, sizeof(buf), 0, &sr.sa, &sl);
 	if (n < 0) {
-		warn("%s: recvfrom() error on ping socket: %s",
-		     pname, strerror(errno));
+		flow_err(pingf, "recvfrom() error: %s", strerror(errno));
 		return;
 	}
 	if (sr.sa_family != af)
@@ -113,8 +111,9 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 		pingf->seq = seq;
 	}
 
-	debug("%s: echo reply to tap, ID: %"PRIu16", seq: %"PRIu16, pname,
-	      ref.icmp.id, seq);
+	flow_dbg(pingf, "echo reply to tap, ID: %"PRIu16", seq: %"PRIu16,
+		 ref.icmp.id, seq);
+
 	if (af == AF_INET)
 		tap_icmp4_send(c, sr.sa4.sin_addr, tap_ip4_daddr(c), buf, n);
 	else if (af == AF_INET6)
@@ -123,7 +122,7 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 	return;
 
 unexpected:
-	warn("%s: Unexpected packet on ping socket", pname);
+	flow_err(pingf, "Unexpected packet on ping socket");
 }
 
 /**
@@ -158,7 +157,6 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 					    struct icmp_ping_flow **id_sock,
 					    sa_family_t af, uint16_t id)
 {
-	const char *const pname = af == AF_INET ? "ICMP" : "ICMPv6";
 	uint8_t flowtype = af == AF_INET ? FLOW_PING4 : FLOW_PING6;
 	union icmp_epoll_ref iref = { .id = id };
 	union flow *flow = flow_alloc();
@@ -195,9 +193,9 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 	if (pingf->sock > FD_REF_MAX)
 		goto cancel;
 
-	*id_sock = pingf;
+	flow_dbg(pingf, "new socket %i for echo ID %"PRIu16, pingf->sock, id);
 
-	debug("%s: new socket %i for echo ID %"PRIu16, pname, pingf->sock, id);
+	*id_sock = pingf;
 
 	return pingf;
 
@@ -222,7 +220,6 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 		     const void *saddr, const void *daddr,
 		     const struct pool *p, const struct timespec *now)
 {
-	const char *const pname = af == AF_INET ? "ICMP" : "ICMPv6";
 	union sockaddr_inany sa = { .sa_family = af };
 	const socklen_t sl = af == AF_INET ? sizeof(sa.sa4) : sizeof(sa.sa6);
 	struct icmp_ping_flow *pingf, **id_sock;
@@ -276,13 +273,13 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 
 	pingf->ts = now->tv_sec;
 
-	if (sendto(pingf->sock, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
-		debug("%s: failed to relay request to socket: %s",
-		      pname, strerror(errno));
-	} else {
-		debug("%s: echo request to socket, ID: %"PRIu16", seq: %"PRIu16,
-		      pname, id, seq);
-	}
+	if (sendto(pingf->sock, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0)
+		flow_dbg(pingf, "failed to relay request to socket: %s",
+			 strerror(errno));
+	else
+		flow_dbg(pingf,
+			 "echo request to socket, ID: %"PRIu16", seq: %"PRIu16,
+			 id, seq);
 
 	return 1;
 }
-- 
@@ -61,7 +61,6 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 {
 	struct icmp_ping_flow *pingf = af == AF_INET
 		? icmp_id_map[V4][ref.icmp.id] : icmp_id_map[V6][ref.icmp.id];
-	const char *const pname = af == AF_INET ? "ICMP" : "ICMPv6";
 	union sockaddr_inany sr;
 	socklen_t sl = sizeof(sr);
 	char buf[USHRT_MAX];
@@ -75,8 +74,7 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 
 	n = recvfrom(ref.fd, buf, sizeof(buf), 0, &sr.sa, &sl);
 	if (n < 0) {
-		warn("%s: recvfrom() error on ping socket: %s",
-		     pname, strerror(errno));
+		flow_err(pingf, "recvfrom() error: %s", strerror(errno));
 		return;
 	}
 	if (sr.sa_family != af)
@@ -113,8 +111,9 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 		pingf->seq = seq;
 	}
 
-	debug("%s: echo reply to tap, ID: %"PRIu16", seq: %"PRIu16, pname,
-	      ref.icmp.id, seq);
+	flow_dbg(pingf, "echo reply to tap, ID: %"PRIu16", seq: %"PRIu16,
+		 ref.icmp.id, seq);
+
 	if (af == AF_INET)
 		tap_icmp4_send(c, sr.sa4.sin_addr, tap_ip4_daddr(c), buf, n);
 	else if (af == AF_INET6)
@@ -123,7 +122,7 @@ void icmp_sock_handler(const struct ctx *c, sa_family_t af, union epoll_ref ref)
 	return;
 
 unexpected:
-	warn("%s: Unexpected packet on ping socket", pname);
+	flow_err(pingf, "Unexpected packet on ping socket");
 }
 
 /**
@@ -158,7 +157,6 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 					    struct icmp_ping_flow **id_sock,
 					    sa_family_t af, uint16_t id)
 {
-	const char *const pname = af == AF_INET ? "ICMP" : "ICMPv6";
 	uint8_t flowtype = af == AF_INET ? FLOW_PING4 : FLOW_PING6;
 	union icmp_epoll_ref iref = { .id = id };
 	union flow *flow = flow_alloc();
@@ -195,9 +193,9 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c,
 	if (pingf->sock > FD_REF_MAX)
 		goto cancel;
 
-	*id_sock = pingf;
+	flow_dbg(pingf, "new socket %i for echo ID %"PRIu16, pingf->sock, id);
 
-	debug("%s: new socket %i for echo ID %"PRIu16, pname, pingf->sock, id);
+	*id_sock = pingf;
 
 	return pingf;
 
@@ -222,7 +220,6 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 		     const void *saddr, const void *daddr,
 		     const struct pool *p, const struct timespec *now)
 {
-	const char *const pname = af == AF_INET ? "ICMP" : "ICMPv6";
 	union sockaddr_inany sa = { .sa_family = af };
 	const socklen_t sl = af == AF_INET ? sizeof(sa.sa4) : sizeof(sa.sa6);
 	struct icmp_ping_flow *pingf, **id_sock;
@@ -276,13 +273,13 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
 
 	pingf->ts = now->tv_sec;
 
-	if (sendto(pingf->sock, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
-		debug("%s: failed to relay request to socket: %s",
-		      pname, strerror(errno));
-	} else {
-		debug("%s: echo request to socket, ID: %"PRIu16", seq: %"PRIu16,
-		      pname, id, seq);
-	}
+	if (sendto(pingf->sock, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0)
+		flow_dbg(pingf, "failed to relay request to socket: %s",
+			 strerror(errno));
+	else
+		flow_dbg(pingf,
+			 "echo request to socket, ID: %"PRIu16", seq: %"PRIu16,
+			 id, seq);
 
 	return 1;
 }
-- 
2.44.0


  parent reply	other threads:[~2024-02-29  4:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-29  4:15 [PATCH 0/3] Basic integration of ICMP with flow table David Gibson
2024-02-29  4:15 ` [PATCH 1/3] icmp: Store ping socket information in " David Gibson
2024-03-07  8:53   ` Stefano Brivio
2024-03-07 10:24     ` David Gibson
2024-03-07 23:25       ` Stefano Brivio
2024-03-08  0:48         ` David Gibson
2024-02-29  4:15 ` David Gibson [this message]
2024-03-07 23:26   ` [PATCH 2/3] icmp: Flow based error reporting Stefano Brivio
2024-03-08  0:49     ` David Gibson
2024-03-08  6:06       ` Stefano Brivio
2024-02-29  4:15 ` [PATCH 3/3] icmp: Use 'flowside' epoll references for ping sockets David Gibson
2024-03-13 10:08 ` [PATCH 0/3] Basic integration of ICMP with flow table 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=20240229041534.2573559-3-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).