[PATCH 0/3] udp: Further preliminaries towards using connected sockets on both sides

[PATCH 0/3] udp: Further preliminaries towards using connected sockets on both sides

[David Gibson]

Some additional small cleanups

David Gibson (3):
  udp: Remove redundant udp_at_sidx() call in udp_tap_handler()
  udp: Simplify updates to UDP flow timestamp
  udp: Correct some seccomp filter annotations

 udp.c    | 23 +++++++++--------------
 udp_vu.c |  9 +--------
 udp_vu.h |  3 +--
 3 files changed, 11 insertions(+), 24 deletions(-)

-- 
2.49.0

[PATCH 1/3] udp: Remove redundant udp_at_sidx() call in udp_tap_handler()

[David Gibson]

We've already have a pointer to the UDP flow in variable uflow, we can just
re-use it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/udp.c b/udp.c
index 39431d72..ac168db9 100644
--- a/udp.c
+++ b/udp.c
@@ -907,7 +907,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
 	}
 	toside = flowside_at_sidx(tosidx);
 
-	s = udp_at_sidx(tosidx)->s[tosidx.sidei];
+	s = uflow->s[tosidx.sidei];
 	ASSERT(s >= 0);
 
 	pif_sockaddr(c, &to_sa, &sl, topif, &toside->eaddr, toside->eport);
-- 
@@ -907,7 +907,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
 	}
 	toside = flowside_at_sidx(tosidx);
 
-	s = udp_at_sidx(tosidx)->s[tosidx.sidei];
+	s = uflow->s[tosidx.sidei];
 	ASSERT(s >= 0);
 
 	pif_sockaddr(c, &to_sa, &sl, topif, &toside->eaddr, toside->eport);
-- 
2.49.0

[PATCH 2/3] udp: Simplify updates to UDP flow timestamp

[David Gibson]

Since UDP has no built in knowledge of connections, the only way we
know when we're done with a UDP flow is a timeout with no activity.
To keep track of this struct udp_flow includes a timestamp to record
the last time we saw traffic on the flow.

For data from listening sockets and from tap, this is done implicitly via
udp_flow_from_{sock,tap}() but for reply sockets it's done explicitly.
However, that logic is duplicated between the vhost-user and "buf" paths.
Make it common in udp_reply_sock_handler() instead.

Technically this is a behavioural change: previously if we got an EPOLLIN
event, but there wasn't actually any data we wouldn't update the timestamp,
now we will.  This should be harmless: if there's an EPOLLIN we expect
there to be data, and even if there isn't the worst we can do is mildly
delay the cleanup of a stale flow.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 udp.c    | 15 ++++++---------
 udp_vu.c |  9 +--------
 udp_vu.h |  3 +--
 3 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/udp.c b/udp.c
index ac168db9..44b58d1c 100644
--- a/udp.c
+++ b/udp.c
@@ -758,27 +758,21 @@ void udp_listen_sock_handler(const struct ctx *c,
  * @c:		Execution context
  * @s:		Socket to read data from
  * @tosidx:	Flow & side to forward data from @s to
- * @now:	Current timestamp
  *
  * Return: true on success, false if can't forward from socket to flow's pif
  *
  * #syscalls recvmmsg
  */
 static bool udp_buf_reply_sock_data(const struct ctx *c,
-				    int s, flow_sidx_t tosidx,
-				    const struct timespec *now)
+				    int s, flow_sidx_t tosidx)
 {
 	const struct flowside *toside = flowside_at_sidx(tosidx);
-	struct udp_flow *uflow = udp_at_sidx(tosidx);
 	uint8_t topif = pif_at_sidx(tosidx);
 	int n, i;
 
 	if ((n = udp_sock_recv(c, s, udp_mh_recv)) <= 0)
 		return true;
 
-	flow_trace(uflow, "Received %d datagrams on reply socket", n);
-	uflow->ts = now->tv_sec;
-
 	for (i = 0; i < n; i++) {
 		if (pif_is_socket(topif))
 			udp_splice_prepare(udp_mh_recv, i);
@@ -825,10 +819,13 @@ void udp_reply_sock_handler(const struct ctx *c, union epoll_ref ref,
 		int s = ref.fd;
 		bool ret;
 
+		flow_trace(uflow, "Received data on reply socket");
+		uflow->ts = now->tv_sec;
+
 		if (c->mode == MODE_VU)
-			ret = udp_vu_reply_sock_data(c, s, tosidx, now);
+			ret = udp_vu_reply_sock_data(c, s, tosidx);
 		else
-			ret = udp_buf_reply_sock_data(c, s, tosidx, now);
+			ret = udp_buf_reply_sock_data(c, s, tosidx);
 
 		if (!ret) {
 			flow_err(uflow, "Unable to forward UDP");
diff --git a/udp_vu.c b/udp_vu.c
index 06bdeae6..4153b6c1 100644
--- a/udp_vu.c
+++ b/udp_vu.c
@@ -275,22 +275,17 @@ void udp_vu_listen_sock_data(const struct ctx *c, union epoll_ref ref,
  * @c:		Execution context
  * @s:		Socket to read data from
  * @tosidx:	Flow & side to forward data from @s to
- * @now:	Current timestamp
  *
  * Return: true on success, false if can't forward from socket to flow's pif
  */
-bool udp_vu_reply_sock_data(const struct ctx *c, int s, flow_sidx_t tosidx,
-			    const struct timespec *now)
+bool udp_vu_reply_sock_data(const struct ctx *c, int s, flow_sidx_t tosidx)
 {
 	const struct flowside *toside = flowside_at_sidx(tosidx);
 	bool v6 = !(inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr));
-	struct udp_flow *uflow = udp_at_sidx(tosidx);
 	struct vu_dev *vdev = c->vdev;
 	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
 	int i;
 
-	ASSERT(uflow);
-
 	if (pif_at_sidx(tosidx) != PIF_TAP)
 		return false;
 
@@ -301,8 +296,6 @@ bool udp_vu_reply_sock_data(const struct ctx *c, int s, flow_sidx_t tosidx,
 		iov_used = udp_vu_sock_recv(c, s, v6, &dlen);
 		if (iov_used <= 0)
 			break;
-		flow_trace(uflow, "Received 1 datagram on reply socket");
-		uflow->ts = now->tv_sec;
 
 		udp_vu_prepare(c, toside, dlen);
 		if (*c->pcap) {
diff --git a/udp_vu.h b/udp_vu.h
index 2299b51f..6d541a4e 100644
--- a/udp_vu.h
+++ b/udp_vu.h
@@ -8,7 +8,6 @@
 
 void udp_vu_listen_sock_data(const struct ctx *c, union epoll_ref ref,
 			     const struct timespec *now);
-bool udp_vu_reply_sock_data(const struct ctx *c, int s, flow_sidx_t tosidx,
-			    const struct timespec *now);
+bool udp_vu_reply_sock_data(const struct ctx *c, int s, flow_sidx_t tosidx);
 
 #endif /* UDP_VU_H */
-- 
@@ -8,7 +8,6 @@
 
 void udp_vu_listen_sock_data(const struct ctx *c, union epoll_ref ref,
 			     const struct timespec *now);
-bool udp_vu_reply_sock_data(const struct ctx *c, int s, flow_sidx_t tosidx,
-			    const struct timespec *now);
+bool udp_vu_reply_sock_data(const struct ctx *c, int s, flow_sidx_t tosidx);
 
 #endif /* UDP_VU_H */
-- 
2.49.0

[PATCH 3/3] udp: Correct some seccomp filter annotations

[David Gibson]

Both udp_buf_listen_sock_data() and udp_buf_reply_sock_data() have comments
stating they use recvmmsg().  That's not correct, they only do so via
udp_sock_recv() which lists recvmmsg() itself.

In contrast udp_splice_send() and udp_tap_handler() both directly use
sendmmsg(), but only the latter lists it.  Add it to the former as well.

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

diff --git a/udp.c b/udp.c
index 44b58d1c..ab3e9d20 100644
--- a/udp.c
+++ b/udp.c
@@ -272,6 +272,8 @@ static void udp_splice_prepare(struct mmsghdr *mmh, unsigned idx)
  * @dst:	Destination port for datagrams (target side)
  * @ref:	epoll reference for origin socket
  * @now:	Timestamp
+ *
+ * #syscalls sendmmsg
  */
 static void udp_splice_send(const struct ctx *c, size_t start, size_t n,
 			    flow_sidx_t tosidx)
@@ -662,8 +664,6 @@ static int udp_sock_recv(const struct ctx *c, int s, struct mmsghdr *mmh)
  * @c:		Execution context
  * @ref:	epoll reference
  * @now:	Current timestamp
- *
- * #syscalls recvmmsg
  */
 static void udp_buf_listen_sock_data(const struct ctx *c, union epoll_ref ref,
 				     const struct timespec *now)
@@ -760,8 +760,6 @@ void udp_listen_sock_handler(const struct ctx *c,
  * @tosidx:	Flow & side to forward data from @s to
  *
  * Return: true on success, false if can't forward from socket to flow's pif
- *
- * #syscalls recvmmsg
  */
 static bool udp_buf_reply_sock_data(const struct ctx *c,
 				    int s, flow_sidx_t tosidx)
-- 
@@ -272,6 +272,8 @@ static void udp_splice_prepare(struct mmsghdr *mmh, unsigned idx)
  * @dst:	Destination port for datagrams (target side)
  * @ref:	epoll reference for origin socket
  * @now:	Timestamp
+ *
+ * #syscalls sendmmsg
  */
 static void udp_splice_send(const struct ctx *c, size_t start, size_t n,
 			    flow_sidx_t tosidx)
@@ -662,8 +664,6 @@ static int udp_sock_recv(const struct ctx *c, int s, struct mmsghdr *mmh)
  * @c:		Execution context
  * @ref:	epoll reference
  * @now:	Current timestamp
- *
- * #syscalls recvmmsg
  */
 static void udp_buf_listen_sock_data(const struct ctx *c, union epoll_ref ref,
 				     const struct timespec *now)
@@ -760,8 +760,6 @@ void udp_listen_sock_handler(const struct ctx *c,
  * @tosidx:	Flow & side to forward data from @s to
  *
  * Return: true on success, false if can't forward from socket to flow's pif
- *
- * #syscalls recvmmsg
  */
 static bool udp_buf_reply_sock_data(const struct ctx *c,
 				    int s, flow_sidx_t tosidx)
-- 
2.49.0

Re: [PATCH 0/3] udp: Further preliminaries towards using connected sockets on both sides

[Stefano Brivio]

On Tue,  1 Apr 2025 19:57:07 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Some additional small cleanups
> 
> David Gibson (3):
>   udp: Remove redundant udp_at_sidx() call in udp_tap_handler()
>   udp: Simplify updates to UDP flow timestamp
>   udp: Correct some seccomp filter annotations

Applied.

-- 
Stefano

Re: [PATCH 0/3] udp: Further preliminaries towards using connected sockets on both sides

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

On Wed, Apr 02, 2025 at 03:44:17PM +0200, Stefano Brivio wrote:
> On Tue,  1 Apr 2025 19:57:07 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Some additional small cleanups
> > 
> > David Gibson (3):
> >   udp: Remove redundant udp_at_sidx() call in udp_tap_handler()
> >   udp: Simplify updates to UDP flow timestamp
> >   udp: Correct some seccomp filter annotations
> 
> Applied.

Did you forget to push?  I'm not seeing these in the tree.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/3] udp: Further preliminaries towards using connected sockets on both sides

[Stefano Brivio]

On Thu, 3 Apr 2025 11:02:10 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Apr 02, 2025 at 03:44:17PM +0200, Stefano Brivio wrote:
> > On Tue,  1 Apr 2025 19:57:07 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > Some additional small cleanups
> > > 
> > > David Gibson (3):
> > >   udp: Remove redundant udp_at_sidx() call in udp_tap_handler()
> > >   udp: Simplify updates to UDP flow timestamp
> > >   udp: Correct some seccomp filter annotations  
> > 
> > Applied.  
> 
> Did you forget to push?  I'm not seeing these in the tree.

Oops, sorry, yes. Pushed now.

-- 
Stefano