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 4/6] udp: Split socket error handling out from udp_sock_recv()
Date: Fri,  6 Sep 2024 15:17:08 +1000	[thread overview]
Message-ID: <20240906051710.3863211-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240906051710.3863211-1-david@gibson.dropbear.id.au>

Currently udp_sock_recv() both attempts to clear socket errors and read
a batch of datagrams for forwarding.  That made sense initially, since
both listening and reply sockets need to do this.  However, we have certain
error cases which will add additional complexity to the error processing.
Furthermore, if we ever wanted to more thoroughly handle errors received
here - e.g. by synthesising ICMP messages on the tap device - it will
likely require different handling for the listening and reply socket cases.

So, split handling of error events into its own udp_sock_errs() function.
While we're there, allow it to report "unrecoverable errors".  We don't
have any of these so far, but some cases we're working on might require it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 46 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/udp.c b/udp.c
index aae6d142..fd91b539 100644
--- a/udp.c
+++ b/udp.c
@@ -436,6 +436,30 @@ static bool udp_sock_recverr(int s)
 	return true;
 }
 
+/**
+ * udp_sock_errs() - Process errors on a socket
+ * @c:		Execution context
+ * @s:		Socket to receive from
+ * @events:	epoll events bitmap
+ *
+ * Return: Number of errors handled, or < 0 if we have an unrecoverable error
+ */
+static int udp_sock_errs(const struct ctx *c, int s, uint32_t events)
+{
+	unsigned n_err = 0;
+
+	ASSERT(!c->no_udp);
+
+	if (!(events & EPOLLERR))
+		return 0; /* Nothing to do */
+
+	/* Empty the error queue */
+	while (udp_sock_recverr(s))
+		n_err++;
+
+	return n_err;
+}
+
 /**
  * udp_sock_recv() - Receive datagrams from a socket
  * @c:		Execution context
@@ -443,6 +467,8 @@ static bool udp_sock_recverr(int s)
  * @events:	epoll events bitmap
  * @mmh		mmsghdr array to receive into
  *
+ * Return: Number of datagrams received
+ *
  * #syscalls recvmmsg arm:recvmmsg_time64 i686:recvmmsg_time64
  */
 static int udp_sock_recv(const struct ctx *c, int s, uint32_t events,
@@ -459,12 +485,6 @@ static int udp_sock_recv(const struct ctx *c, int s, uint32_t events,
 
 	ASSERT(!c->no_udp);
 
-	/* Clear any errors first */
-	if (events & EPOLLERR) {
-		while (udp_sock_recverr(s))
-			;
-	}
-
 	if (!(events & EPOLLIN))
 		return 0;
 
@@ -492,6 +512,13 @@ void udp_listen_sock_handler(const struct ctx *c, union epoll_ref ref,
 	const socklen_t sasize = sizeof(udp_meta[0].s_in);
 	int n, i;
 
+	if (udp_sock_errs(c, ref.fd, events) < 0) {
+		err("UDP: Unrecoverable error on listening socket:"
+		    " (%s port %hu)", pif_name(ref.udp.pif), ref.udp.port);
+		/* FIXME: what now?  close/re-open socket? */
+		return;
+	}
+
 	if ((n = udp_sock_recv(c, ref.fd, events, udp_mh_recv)) <= 0)
 		return;
 
@@ -566,6 +593,13 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref,
 
 	ASSERT(!c->no_udp && uflow);
 
+	if (udp_sock_errs(c, from_s, events) < 0) {
+		flow_err(uflow, "Unrecoverable error on reply socket");
+		flow_err_details(uflow);
+		udp_flow_close(c, uflow);
+		return;
+	}
+
 	if ((n = udp_sock_recv(c, from_s, events, udp_mh_recv)) <= 0)
 		return;
 
-- 
@@ -436,6 +436,30 @@ static bool udp_sock_recverr(int s)
 	return true;
 }
 
+/**
+ * udp_sock_errs() - Process errors on a socket
+ * @c:		Execution context
+ * @s:		Socket to receive from
+ * @events:	epoll events bitmap
+ *
+ * Return: Number of errors handled, or < 0 if we have an unrecoverable error
+ */
+static int udp_sock_errs(const struct ctx *c, int s, uint32_t events)
+{
+	unsigned n_err = 0;
+
+	ASSERT(!c->no_udp);
+
+	if (!(events & EPOLLERR))
+		return 0; /* Nothing to do */
+
+	/* Empty the error queue */
+	while (udp_sock_recverr(s))
+		n_err++;
+
+	return n_err;
+}
+
 /**
  * udp_sock_recv() - Receive datagrams from a socket
  * @c:		Execution context
@@ -443,6 +467,8 @@ static bool udp_sock_recverr(int s)
  * @events:	epoll events bitmap
  * @mmh		mmsghdr array to receive into
  *
+ * Return: Number of datagrams received
+ *
  * #syscalls recvmmsg arm:recvmmsg_time64 i686:recvmmsg_time64
  */
 static int udp_sock_recv(const struct ctx *c, int s, uint32_t events,
@@ -459,12 +485,6 @@ static int udp_sock_recv(const struct ctx *c, int s, uint32_t events,
 
 	ASSERT(!c->no_udp);
 
-	/* Clear any errors first */
-	if (events & EPOLLERR) {
-		while (udp_sock_recverr(s))
-			;
-	}
-
 	if (!(events & EPOLLIN))
 		return 0;
 
@@ -492,6 +512,13 @@ void udp_listen_sock_handler(const struct ctx *c, union epoll_ref ref,
 	const socklen_t sasize = sizeof(udp_meta[0].s_in);
 	int n, i;
 
+	if (udp_sock_errs(c, ref.fd, events) < 0) {
+		err("UDP: Unrecoverable error on listening socket:"
+		    " (%s port %hu)", pif_name(ref.udp.pif), ref.udp.port);
+		/* FIXME: what now?  close/re-open socket? */
+		return;
+	}
+
 	if ((n = udp_sock_recv(c, ref.fd, events, udp_mh_recv)) <= 0)
 		return;
 
@@ -566,6 +593,13 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref,
 
 	ASSERT(!c->no_udp && uflow);
 
+	if (udp_sock_errs(c, from_s, events) < 0) {
+		flow_err(uflow, "Unrecoverable error on reply socket");
+		flow_err_details(uflow);
+		udp_flow_close(c, uflow);
+		return;
+	}
+
 	if ((n = udp_sock_recv(c, from_s, events, udp_mh_recv)) <= 0)
 		return;
 
-- 
2.46.0


  parent reply	other threads:[~2024-09-06  5:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-06  5:17 [PATCH 0/6] RFC: Possible fixes for bug 94 and bug 95 David Gibson
2024-09-06  5:17 ` [PATCH 1/6] flow: Fix incorrect hash probe in flowside_lookup() David Gibson
2024-09-06  5:17 ` [PATCH 2/6] udp: Allow UDP flows to be prematurely closed David Gibson
2024-09-06  5:17 ` [PATCH 3/6] flow: Helpers to log details of a flow David Gibson
2024-09-06  5:17 ` David Gibson [this message]
2024-09-06  5:17 ` [PATCH 5/6] udp: Treat errors getting errors as unrecoverable David Gibson
2024-09-06  5:17 ` [PATCH 6/6] udp: Handle more error conditions in udp_sock_errs() David Gibson
2024-09-06 11:29 ` [PATCH 0/6] RFC: Possible fixes for bug 94 and bug 95 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=20240906051710.3863211-5-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).