udp, tap: Correctly advance through packets in udp_tap_handler()

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>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
David Gibson 2023-09-08 11:49:47 +10:00 committed by Stefano Brivio
parent 043a70b885
commit 7b56117dae
3 changed files with 17 additions and 20 deletions

20
tap.c
View file

@ -707,24 +707,22 @@ append:
for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) { for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) {
struct pool *p = (struct pool *)&seq->p; struct pool *p = (struct pool *)&seq->p;
size_t k;
tap_packet_debug(NULL, NULL, seq, 0, NULL, p->count); tap_packet_debug(NULL, NULL, seq, 0, NULL, p->count);
if (seq->protocol == IPPROTO_TCP) { if (seq->protocol == IPPROTO_TCP) {
size_t k;
if (c->no_tcp) if (c->no_tcp)
continue; continue;
for (k = 0; k < p->count; ) for (k = 0; k < p->count; )
k += tcp_tap_handler(c, AF_INET, &seq->saddr, k += tcp_tap_handler(c, AF_INET, &seq->saddr,
&seq->daddr, p, k, now); &seq->daddr, p, k, now);
} else if (seq->protocol == IPPROTO_UDP) { } else if (seq->protocol == IPPROTO_UDP) {
size_t n = p->count;
if (c->no_udp) if (c->no_udp)
continue; continue;
while ((n -= udp_tap_handler(c, AF_INET, &seq->saddr, for (k = 0; k < p->count; )
&seq->daddr, p, now))); 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++) { for (j = 0, seq = tap6_l4; j < seq_count; j++, seq++) {
struct pool *p = (struct pool *)&seq->p; struct pool *p = (struct pool *)&seq->p;
size_t k;
tap_packet_debug(NULL, NULL, NULL, seq->protocol, seq, tap_packet_debug(NULL, NULL, NULL, seq->protocol, seq,
p->count); p->count);
if (seq->protocol == IPPROTO_TCP) { if (seq->protocol == IPPROTO_TCP) {
size_t k;
if (c->no_tcp) if (c->no_tcp)
continue; continue;
for (k = 0; k < p->count; ) for (k = 0; k < p->count; )
k += tcp_tap_handler(c, AF_INET6, &seq->saddr, k += tcp_tap_handler(c, AF_INET6, &seq->saddr,
&seq->daddr, p, k, now); &seq->daddr, p, k, now);
} else if (seq->protocol == IPPROTO_UDP) { } else if (seq->protocol == IPPROTO_UDP) {
size_t n = p->count;
if (c->no_udp) if (c->no_udp)
continue; continue;
while ((n -= udp_tap_handler(c, AF_INET6, &seq->saddr, for (k = 0; k < p->count; )
&seq->daddr, p, now))); k += udp_tap_handler(c, AF_INET6, &seq->saddr,
&seq->daddr, p, k, now);
} }
} }

15
udp.c
View file

@ -789,6 +789,7 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
* @saddr: Source address * @saddr: Source address
* @daddr: Destination address * @daddr: Destination address
* @p: Pool of UDP packets, with UDP headers * @p: Pool of UDP packets, with UDP headers
* @idx: Index of first packet to process
* @now: Current timestamp * @now: Current timestamp
* *
* Return: count of consumed packets * 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 * #syscalls sendmmsg
*/ */
int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr, 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 mmsghdr mm[UIO_MAXIOV];
struct iovec m[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)c;
(void)saddr; (void)saddr;
uh = packet_get(p, 0, 0, sizeof(*uh), NULL); uh = packet_get(p, idx, 0, sizeof(*uh), NULL);
if (!uh) if (!uh)
return 1; 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, s = sock_l4(c, AF_INET, IPPROTO_UDP, &bind_addr,
bind_if, src, uref.u32); bind_if, src, uref.u32);
if (s < 0) if (s < 0)
return p->count; return p->count - idx;
udp_tap_map[V4][src].sock = s; udp_tap_map[V4][src].sock = s;
bitmap_set(udp_act[V4][UDP_ACT_TAP], src); 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, s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr,
bind_if, src, uref.u32); bind_if, src, uref.u32);
if (s < 0) if (s < 0)
return p->count; return p->count - idx;
udp_tap_map[V6][src].sock = s; udp_tap_map[V6][src].sock = s;
bitmap_set(udp_act[V6][UDP_ACT_TAP], src); 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; 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; struct udphdr *uh_send;
size_t len; 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) if (!uh_send)
return p->count; return p->count - idx;
mm[i].msg_hdr.msg_name = sa; mm[i].msg_hdr.msg_name = sa;
mm[i].msg_hdr.msg_namelen = sl; mm[i].msg_hdr.msg_namelen = sl;

2
udp.h
View file

@ -11,7 +11,7 @@
void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
const struct timespec *now); const struct timespec *now);
int udp_tap_handler(struct ctx *c, int af, const void *saddr, const void *daddr, 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, int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
const void *addr, const char *ifname, in_port_t port); const void *addr, const char *ifname, in_port_t port);
int udp_init(struct ctx *c); int udp_init(struct ctx *c);