[PATCH 2/8] udp, tap: Correctly advance through packets in udp_tap_handler()

[David Gibson <david@gibson.dropbear.id.au>]

In both tap4_handler() and tap6_handler(), once we've sorted incoming l3
packets into "sequences", we then step through all the packets in each DUP
sequence calling udp_tap_handler().  Or so it appears.

In fact, udp_tap_handler() doesn't take an index and always starts with
packet 0 of the sequence, even if called repeatedly.  It appears to be
written with the idea that the struct pool is a queue, from which it
consumes packets as it processes them, but that's not how the pool data
structure works.

Correct this by adding an index parameter to udp_tap_handler() and altering
the loops in tap.c to step through the pool properly.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tap.c | 20 ++++++++------------
 udp.c | 15 ++++++++-------
 udp.h |  2 +-
 3 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/tap.c b/tap.c
index 445a5ca..93db989 100644
--- a/tap.c
+++ b/tap.c
@@ -707,24 +707,22 @@ append:
 
 	for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) {
 		struct pool *p = (struct pool *)&seq->p;
+		size_t k;
 
 		tap_packet_debug(NULL, NULL, seq, 0, NULL, p->count);
 
 		if (seq->protocol == IPPROTO_TCP) {
-			size_t k;
-
 			if (c->no_tcp)
 				continue;
 			for (k = 0; k < p->count; )
 				k += tcp_tap_handler(c, AF_INET, &seq->saddr,
 						     &seq->daddr, p, k, now);
 		} else if (seq->protocol == IPPROTO_UDP) {
-			size_t n = p->count;
-
 			if (c->no_udp)
 				continue;
-			while ((n -= udp_tap_handler(c, AF_INET, &seq->saddr,
-						     &seq->daddr, p, now)));
+			for (k = 0; k < p->count; )
+				k += udp_tap_handler(c, AF_INET, &seq->saddr,
+						     &seq->daddr, p, k, now);
 		}
 	}
 
@@ -872,25 +870,23 @@ append:
 
 	for (j = 0, seq = tap6_l4; j < seq_count; j++, seq++) {
 		struct pool *p = (struct pool *)&seq->p;
+		size_t k;
 
 		tap_packet_debug(NULL, NULL, NULL, seq->protocol, seq,
 				 p->count);
 
 		if (seq->protocol == IPPROTO_TCP) {
-			size_t k;
-
 			if (c->no_tcp)
 				continue;
 			for (k = 0; k < p->count; )
 				k += tcp_tap_handler(c, AF_INET6, &seq->saddr,
 						     &seq->daddr, p, k, now);
 		} else if (seq->protocol == IPPROTO_UDP) {
-			size_t n = p->count;
-
 			if (c->no_udp)
 				continue;
-			while ((n -= udp_tap_handler(c, AF_INET6, &seq->saddr,
-						     &seq->daddr, p, now)));
+			for (k = 0; k < p->count; )
+				k += udp_tap_handler(c, AF_INET6, &seq->saddr,
+						     &seq->daddr, p, k, now);
 		}
 	}
 
diff --git a/udp.c b/udp.c
index f4ed660..ed1b7a5 100644
--- a/udp.c
+++ b/udp.c
@@ -789,6 +789,7 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
  * @saddr:	Source address
  * @daddr:	Destination address
  * @p:		Pool of UDP packets, with UDP headers
+ * @idx:	Index of first packet to process
  * @now:	Current timestamp
  *
  * Return: count of consumed packets
@@ -796,7 +797,7 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
  * #syscalls sendmmsg
  */
 int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr,
-		    const struct pool *p, const struct timespec *now)
+		    const struct pool *p, int idx, const struct timespec *now)
 {
 	struct mmsghdr mm[UIO_MAXIOV];
 	struct iovec m[UIO_MAXIOV];
@@ -811,7 +812,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr,
 	(void)c;
 	(void)saddr;
 
-	uh = packet_get(p, 0, 0, sizeof(*uh), NULL);
+	uh = packet_get(p, idx, 0, sizeof(*uh), NULL);
 	if (!uh)
 		return 1;
 
@@ -859,7 +860,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr,
 			s = sock_l4(c, AF_INET, IPPROTO_UDP, &bind_addr,
 				    bind_if, src, uref.u32);
 			if (s < 0)
-				return p->count;
+				return p->count - idx;
 
 			udp_tap_map[V4][src].sock = s;
 			bitmap_set(udp_act[V4][UDP_ACT_TAP], src);
@@ -909,7 +910,7 @@ int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr,
 			s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr,
 				    bind_if, src, uref.u32);
 			if (s < 0)
-				return p->count;
+				return p->count - idx;
 
 			udp_tap_map[V6][src].sock = s;
 			bitmap_set(udp_act[V6][UDP_ACT_TAP], src);
@@ -918,13 +919,13 @@ int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr,
 		udp_tap_map[V6][src].ts = now->tv_sec;
 	}
 
-	for (i = 0; i < (int)p->count; i++) {
+	for (i = 0; i < (int)p->count - idx; i++) {
 		struct udphdr *uh_send;
 		size_t len;
 
-		uh_send = packet_get(p, i, 0, sizeof(*uh), &len);
+		uh_send = packet_get(p, idx + i, 0, sizeof(*uh), &len);
 		if (!uh_send)
-			return p->count;
+			return p->count - idx;
 
 		mm[i].msg_hdr.msg_name = sa;
 		mm[i].msg_hdr.msg_namelen = sl;
diff --git a/udp.h b/udp.h
index f553de2..0ee0695 100644
--- a/udp.h
+++ b/udp.h
@@ -11,7 +11,7 @@
 void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
 		      const struct timespec *now);
 int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr,
-		    const struct pool *p, const struct timespec *now);
+		    const struct pool *p, int idx, const struct timespec *now);
 int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 		  const void *addr, const char *ifname, in_port_t port);
 int udp_init(struct ctx *c);
-- 
@@ -11,7 +11,7 @@
 void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
 		      const struct timespec *now);
 int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr,
-		    const struct pool *p, const struct timespec *now);
+		    const struct pool *p, int idx, const struct timespec *now);
 int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
 		  const void *addr, const char *ifname, in_port_t port);
 int udp_init(struct ctx *c);
-- 
2.41.0