From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 284C65A027B for ; Fri, 8 Sep 2023 03:50:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1694137797; bh=QueOJNCQFQPbibpfyLgMpImZkmAHqKQWJsjGZZ5brZk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=X3e4Y+HrIKPXCMrKJ+SSZ3lr7L0njtp2QPqJ/W5k95HJWj7XihLRlkLvJQemhvrKI D96QVd4fs6z+BWK7segigSw5KuXWJu4r/2OmOviF8IRwVCycWe23mUa8Pw6mmJZTU4 NEXRFImqxGqGDO1obOx2joN4QZ3XZhp1Lx+kMovc= Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4RhfF10JdJz4xFd; Fri, 8 Sep 2023 11:49:57 +1000 (AEST) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH 2/8] udp, tap: Correctly advance through packets in udp_tap_handler() Date: Fri, 8 Sep 2023 11:49:47 +1000 Message-ID: <20230908014953.822952-3-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230908014953.822952-1-david@gibson.dropbear.id.au> References: <20230908014953.822952-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: 2R4PQ3XQNL4PFOOTYJYJTDKALY2OJGRW X-Message-ID-Hash: 2R4PQ3XQNL4PFOOTYJYJTDKALY2OJGRW X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: jlesev@gmail.com, David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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); -- 2.41.0