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 3/5] udp, tcp: Tweak handling of no_udp and no_tcp flags
Date: Tue, 16 Jul 2024 15:29:34 +1000	[thread overview]
Message-ID: <20240716052936.1204164-4-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240716052936.1204164-1-david@gibson.dropbear.id.au>

We abort the UDP socket handler if the no_udp flag is set.  But if UDP
was disabled we should never have had a UDP socket to trigger the handler
in the first place.  If we somehow did, ignoring it here isn't really going
to help because aborting without doing anything is likely to lead to an
epoll loop.  The same is the case for the TCP socket and timer handlers and
the no_tcp flag.

Change these checks on the flag to ASSERT()s.  Similarly add ASSERT()s to
several other entry points to the protocol specific code which should never
be called if the protocol is disabled.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c | 14 +++++++++++---
 udp.c | 13 +++++++++++--
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/tcp.c b/tcp.c
index 75b959a2..de3f9f64 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2286,7 +2286,9 @@ void tcp_listen_handler(struct ctx *c, union epoll_ref ref,
 	union flow *flow;
 	int s;
 
-	if (c->no_tcp || !(flow = flow_alloc()))
+	ASSERT(!c->no_tcp);
+
+	if (!(flow = flow_alloc()))
 		return;
 
 	s = accept4(ref.fd, &sa.sa, &sl, SOCK_NONBLOCK);
@@ -2339,8 +2341,7 @@ void tcp_timer_handler(struct ctx *c, union epoll_ref ref)
 	struct itimerspec check_armed = { { 0 }, { 0 } };
 	struct tcp_tap_conn *conn = CONN(ref.flow);
 
-	if (c->no_tcp)
-		return;
+	ASSERT(!c->no_tcp);
 
 	/* We don't reset timers on ~ACK_FROM_TAP_DUE, ~ACK_TO_TAP_DUE. If the
 	 * timer is currently armed, this event came from a previous setting,
@@ -2398,6 +2399,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events)
 {
 	struct tcp_tap_conn *conn = CONN(ref.flowside.flow);
 
+	ASSERT(!c->no_tcp);
 	ASSERT(conn->f.type == FLOW_TCP);
 	ASSERT(conn->f.pif[ref.flowside.side] != PIF_TAP);
 
@@ -2497,6 +2499,8 @@ int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
 {
 	int r4 = FD_REF_MAX + 1, r6 = FD_REF_MAX + 1;
 
+	ASSERT(!c->no_tcp);
+
 	if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
 		/* Attempt to get a dual stack socket */
 		if (tcp_sock_init_af(c, AF_UNSPEC, port, addr, ifname) >= 0)
@@ -2574,6 +2578,8 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port)
  */
 void tcp_ns_sock_init(const struct ctx *c, in_port_t port)
 {
+	ASSERT(!c->no_tcp);
+
 	if (c->ifi4)
 		tcp_ns_sock_init4(c, port);
 	if (c->ifi6)
@@ -2661,6 +2667,8 @@ int tcp_init(struct ctx *c)
 {
 	unsigned b;
 
+	ASSERT(!c->no_tcp);
+
 	for (b = 0; b < TCP_HASH_TABLE_SIZE; b++)
 		tc_hash[b] = FLOW_SIDX_NONE;
 
diff --git a/udp.c b/udp.c
index 187483f0..fbf3ce73 100644
--- a/udp.c
+++ b/udp.c
@@ -749,7 +749,9 @@ static int udp_sock_recv(const struct ctx *c, int s, uint32_t events,
 	 */
 	int n = (c->mode == MODE_PASTA ? 1 : UDP_MAX_FRAMES);
 
-	if (c->no_udp || !(events & EPOLLIN))
+	ASSERT(!c->no_udp);
+
+	if (!(events & EPOLLIN))
 		return 0;
 
 	n = recvmmsg(s, mmh, n, 0, NULL);
@@ -849,10 +851,11 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
 	in_port_t src, dst;
 	socklen_t sl;
 
-	(void)c;
 	(void)saddr;
 	(void)pif;
 
+	ASSERT(!c->no_udp);
+
 	uh = packet_get(p, idx, 0, sizeof(*uh), NULL);
 	if (!uh)
 		return 1;
@@ -1026,6 +1029,8 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 				     .orig = true, .port = port };
 	int s, r4 = FD_REF_MAX + 1, r6 = FD_REF_MAX + 1;
 
+	ASSERT(!c->no_udp);
+
 	if (ns)
 		uref.pif = PIF_SPLICE;
 	else
@@ -1214,6 +1219,8 @@ void udp_timer(struct ctx *c, const struct timespec *now)
 	unsigned int i;
 	long *word, tmp;
 
+	ASSERT(!c->no_udp);
+
 	if (c->mode == MODE_PASTA) {
 		if (c->udp.fwd_out.f.mode == FWD_AUTO) {
 			fwd_scan_ports_udp(&c->udp.fwd_out.f, &c->udp.fwd_in.f,
@@ -1257,6 +1264,8 @@ v6:
  */
 int udp_init(struct ctx *c)
 {
+	ASSERT(!c->no_udp);
+
 	udp_iov_init(c);
 
 	udp_invert_portmap(&c->udp.fwd_in);
-- 
@@ -749,7 +749,9 @@ static int udp_sock_recv(const struct ctx *c, int s, uint32_t events,
 	 */
 	int n = (c->mode == MODE_PASTA ? 1 : UDP_MAX_FRAMES);
 
-	if (c->no_udp || !(events & EPOLLIN))
+	ASSERT(!c->no_udp);
+
+	if (!(events & EPOLLIN))
 		return 0;
 
 	n = recvmmsg(s, mmh, n, 0, NULL);
@@ -849,10 +851,11 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
 	in_port_t src, dst;
 	socklen_t sl;
 
-	(void)c;
 	(void)saddr;
 	(void)pif;
 
+	ASSERT(!c->no_udp);
+
 	uh = packet_get(p, idx, 0, sizeof(*uh), NULL);
 	if (!uh)
 		return 1;
@@ -1026,6 +1029,8 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 				     .orig = true, .port = port };
 	int s, r4 = FD_REF_MAX + 1, r6 = FD_REF_MAX + 1;
 
+	ASSERT(!c->no_udp);
+
 	if (ns)
 		uref.pif = PIF_SPLICE;
 	else
@@ -1214,6 +1219,8 @@ void udp_timer(struct ctx *c, const struct timespec *now)
 	unsigned int i;
 	long *word, tmp;
 
+	ASSERT(!c->no_udp);
+
 	if (c->mode == MODE_PASTA) {
 		if (c->udp.fwd_out.f.mode == FWD_AUTO) {
 			fwd_scan_ports_udp(&c->udp.fwd_out.f, &c->udp.fwd_in.f,
@@ -1257,6 +1264,8 @@ v6:
  */
 int udp_init(struct ctx *c)
 {
+	ASSERT(!c->no_udp);
+
 	udp_iov_init(c);
 
 	udp_invert_portmap(&c->udp.fwd_in);
-- 
2.45.2


  parent reply	other threads:[~2024-07-16  5:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-16  5:29 [PATCH 0/5] Handle error events on UDP sockets David Gibson
2024-07-16  5:29 ` [PATCH 1/5] conf: Don't configure port forwarding for a disabled protocol David Gibson
2024-07-16  5:29 ` [PATCH 2/5] udp: Make udp_sock_recv static David Gibson
2024-07-16  5:29 ` David Gibson [this message]
2024-07-16  5:29 ` [PATCH 4/5] util: Add AF_UNSPEC support to sockaddr_ntop() David Gibson
2024-07-16  5:29 ` [PATCH 5/5] udp: Handle errors on UDP sockets David Gibson
2024-07-16  7:25   ` Stefano Brivio
2024-07-17  0:04     ` 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=20240716052936.1204164-4-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).