sock_l4() creates a socket of the given IP protocol number, and adds it to the epoll state. Currently it determines the correct tag for the epoll data based on the protocol. However, we have some future cases where we might want different semantics, and therefore epoll types, for sockets of the same protocol. So, change sock_l4() to take the epoll type as an explicit parameter, and determine the protocol from that. Signed-off-by: David Gibson --- epoll_type.h | 41 +++++++++++++++++++++++++++++++++++++++++ icmp.c | 2 +- passt.h | 32 -------------------------------- tcp.c | 10 +++++----- udp.c | 12 ++++++------ util.c | 48 ++++++++++++++++++++++++++---------------------- util.h | 3 ++- 7 files changed, 81 insertions(+), 67 deletions(-) create mode 100644 epoll_type.h diff --git a/epoll_type.h b/epoll_type.h new file mode 100644 index 00000000..42e876e5 --- /dev/null +++ b/epoll_type.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later + * Copyright Red Hat + * Author: Davd Gibson + */ + +#ifndef EPOLL_TYPE_H +#define EPOLL_TYPE_H + +/** + * enum epoll_type - Different types of fds we poll over + */ +enum epoll_type { + /* Special value to indicate an invalid type */ + EPOLL_TYPE_NONE = 0, + /* Connected TCP sockets */ + EPOLL_TYPE_TCP, + /* Connected TCP sockets (spliced) */ + EPOLL_TYPE_TCP_SPLICE, + /* Listening TCP sockets */ + EPOLL_TYPE_TCP_LISTEN, + /* timerfds used for TCP timers */ + EPOLL_TYPE_TCP_TIMER, + /* UDP sockets */ + EPOLL_TYPE_UDP, + /* ICMP/ICMPv6 ping sockets */ + EPOLL_TYPE_PING, + /* inotify fd watching for end of netns (pasta) */ + EPOLL_TYPE_NSQUIT_INOTIFY, + /* timer fd watching for end of netns, fallback for inotify (pasta) */ + EPOLL_TYPE_NSQUIT_TIMER, + /* tuntap character device */ + EPOLL_TYPE_TAP_PASTA, + /* socket connected to qemu */ + EPOLL_TYPE_TAP_PASST, + /* socket listening for qemu socket connections */ + EPOLL_TYPE_TAP_LISTEN, + + EPOLL_NUM_TYPES, +}; + +#endif /* EPOLL_TYPE_H */ diff --git a/icmp.c b/icmp.c index 80330f6f..d4ccc722 100644 --- a/icmp.c +++ b/icmp.c @@ -179,7 +179,7 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c, } ref.flowside = FLOW_SIDX(flow, TGTSIDE); - pingf->sock = sock_l4(c, af, flow_proto[flowtype], bind_addr, bind_if, + pingf->sock = sock_l4(c, af, EPOLL_TYPE_PING, bind_addr, bind_if, 0, ref.data); if (pingf->sock < 0) { diff --git a/passt.h b/passt.h index 21cf4c15..867e77b7 100644 --- a/passt.h +++ b/passt.h @@ -23,38 +23,6 @@ union epoll_ref; #include "tcp.h" #include "udp.h" -/** - * enum epoll_type - Different types of fds we poll over - */ -enum epoll_type { - /* Special value to indicate an invalid type */ - EPOLL_TYPE_NONE = 0, - /* Connected TCP sockets */ - EPOLL_TYPE_TCP, - /* Connected TCP sockets (spliced) */ - EPOLL_TYPE_TCP_SPLICE, - /* Listening TCP sockets */ - EPOLL_TYPE_TCP_LISTEN, - /* timerfds used for TCP timers */ - EPOLL_TYPE_TCP_TIMER, - /* UDP sockets */ - EPOLL_TYPE_UDP, - /* ICMP/ICMPv6 ping sockets */ - EPOLL_TYPE_PING, - /* inotify fd watching for end of netns (pasta) */ - EPOLL_TYPE_NSQUIT_INOTIFY, - /* timer fd watching for end of netns, fallback for inotify (pasta) */ - EPOLL_TYPE_NSQUIT_TIMER, - /* tuntap character device */ - EPOLL_TYPE_TAP_PASTA, - /* socket connected to qemu */ - EPOLL_TYPE_TAP_PASST, - /* socket listening for qemu socket connections */ - EPOLL_TYPE_TAP_LISTEN, - - EPOLL_NUM_TYPES, -}; - /** * union epoll_ref - Breakdown of reference for epoll fd bookkeeping * @type: Type of fd (tells us what to do with events) diff --git a/tcp.c b/tcp.c index 698e7ecb..a490920a 100644 --- a/tcp.c +++ b/tcp.c @@ -2467,7 +2467,7 @@ static int tcp_sock_init_af(const struct ctx *c, sa_family_t af, in_port_t port, }; int s; - s = sock_l4(c, af, IPPROTO_TCP, addr, ifname, port, tref.u32); + s = sock_l4(c, af, EPOLL_TYPE_TCP_LISTEN, addr, ifname, port, tref.u32); if (c->tcp.fwd_in.mode == FWD_AUTO) { if (af == AF_INET || af == AF_UNSPEC) @@ -2531,8 +2531,8 @@ static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port) ASSERT(c->mode == MODE_PASTA); - s = sock_l4(c, AF_INET, IPPROTO_TCP, &in4addr_loopback, NULL, port, - tref.u32); + s = sock_l4(c, AF_INET, EPOLL_TYPE_TCP_LISTEN, &in4addr_loopback, + NULL, port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else @@ -2557,8 +2557,8 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port) ASSERT(c->mode == MODE_PASTA); - s = sock_l4(c, AF_INET6, IPPROTO_TCP, &in6addr_loopback, NULL, port, - tref.u32); + s = sock_l4(c, AF_INET6, EPOLL_TYPE_TCP_LISTEN, &in6addr_loopback, + NULL, port, tref.u32); if (s >= 0) tcp_sock_set_bufsize(c, s); else diff --git a/udp.c b/udp.c index e089ef95..eadf4872 100644 --- a/udp.c +++ b/udp.c @@ -917,7 +917,7 @@ int udp_tap_handler(struct ctx *c, uint8_t pif, if (!IN4_IS_ADDR_LOOPBACK(&s_in.sin_addr)) bind_addr = c->ip4.addr_out; - s = sock_l4(c, AF_INET, IPPROTO_UDP, &bind_addr, + s = sock_l4(c, AF_INET, EPOLL_TYPE_UDP, &bind_addr, bind_if, src, uref.u32); if (s < 0) return p->count - idx; @@ -972,7 +972,7 @@ int udp_tap_handler(struct ctx *c, uint8_t pif, !IN6_IS_ADDR_LINKLOCAL(&s_in6.sin6_addr)) bind_addr = &c->ip6.addr_out; - s = sock_l4(c, AF_INET6, IPPROTO_UDP, bind_addr, + s = sock_l4(c, AF_INET6, EPOLL_TYPE_UDP, bind_addr, bind_if, src, uref.u32); if (s < 0) return p->count - idx; @@ -1047,13 +1047,13 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, uref.v6 = 0; if (!ns) { - r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, addr, + r4 = s = sock_l4(c, AF_INET, EPOLL_TYPE_UDP, addr, ifname, port, uref.u32); udp_tap_map[V4][port].sock = s < 0 ? -1 : s; udp_splice_init[V4][port].sock = s < 0 ? -1 : s; } else { - r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, + r4 = s = sock_l4(c, AF_INET, EPOLL_TYPE_UDP, &in4addr_loopback, ifname, port, uref.u32); udp_splice_ns[V4][port].sock = s < 0 ? -1 : s; @@ -1064,13 +1064,13 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, uref.v6 = 1; if (!ns) { - r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr, + r6 = s = sock_l4(c, AF_INET6, EPOLL_TYPE_UDP, addr, ifname, port, uref.u32); udp_tap_map[V6][port].sock = s < 0 ? -1 : s; udp_splice_init[V6][port].sock = s < 0 ? -1 : s; } else { - r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP, + r6 = s = sock_l4(c, AF_INET6, EPOLL_TYPE_UDP, &in6addr_loopback, ifname, port, uref.u32); udp_splice_ns[V6][port].sock = s < 0 ? -1 : s; diff --git a/util.c b/util.c index dd2e57f6..9a73fbb9 100644 --- a/util.c +++ b/util.c @@ -35,7 +35,7 @@ /** * sock_l4_sa() - Create and bind socket to socket address, add to epoll list * @c: Execution context - * @proto: Protocol number + * @type: epoll type * @sa: Socket address to bind to * @sl: Length of @sa * @ifname: Interface for binding, NULL for any @@ -44,34 +44,38 @@ * * Return: newly created socket, negative error code on failure */ -static int sock_l4_sa(const struct ctx *c, uint8_t proto, +static int sock_l4_sa(const struct ctx *c, enum epoll_type type, const void *sa, socklen_t sl, const char *ifname, bool v6only, uint32_t data) { sa_family_t af = ((const struct sockaddr *)sa)->sa_family; - union epoll_ref ref = { .data = data }; + union epoll_ref ref = { .type = type, .data = data }; struct epoll_event ev; int fd, y = 1, ret; + uint8_t proto; + int socktype; - switch (proto) { - case IPPROTO_TCP: - ref.type = EPOLL_TYPE_TCP_LISTEN; + switch (type) { + case EPOLL_TYPE_TCP_LISTEN: + proto = IPPROTO_TCP; + socktype = SOCK_STREAM | SOCK_NONBLOCK; break; - case IPPROTO_UDP: - ref.type = EPOLL_TYPE_UDP; + case EPOLL_TYPE_UDP: + proto = IPPROTO_UDP; + socktype = SOCK_DGRAM | SOCK_NONBLOCK; break; - case IPPROTO_ICMP: - case IPPROTO_ICMPV6: - ref.type = EPOLL_TYPE_PING; + case EPOLL_TYPE_PING: + if (af == AF_INET) + proto = IPPROTO_ICMP; + else + proto = IPPROTO_ICMPV6; + socktype = SOCK_DGRAM | SOCK_NONBLOCK; break; default: - return -EPFNOSUPPORT; /* Not implemented. */ + ASSERT(0); } - if (proto == IPPROTO_TCP) - fd = socket(af, SOCK_STREAM | SOCK_NONBLOCK, proto); - else - fd = socket(af, SOCK_DGRAM | SOCK_NONBLOCK, proto); + fd = socket(af, socktype, proto); ret = -errno; if (fd < 0) { @@ -118,14 +122,14 @@ static int sock_l4_sa(const struct ctx *c, uint8_t proto, * this is fine. This might also fail for ICMP because of a * broken SELinux policy, see icmp_tap_handler(). */ - if (proto != IPPROTO_ICMP && proto != IPPROTO_ICMPV6) { + if (type != EPOLL_TYPE_PING) { ret = -errno; close(fd); return ret; } } - if (proto == IPPROTO_TCP && listen(fd, 128) < 0) { + if (type == EPOLL_TYPE_TCP_LISTEN && listen(fd, 128) < 0) { ret = -errno; warn("TCP socket listen: %s", strerror(-ret)); close(fd); @@ -146,7 +150,7 @@ static int sock_l4_sa(const struct ctx *c, uint8_t proto, * sock_l4() - Create and bind socket for given L4, add to epoll list * @c: Execution context * @af: Address family, AF_INET or AF_INET6 - * @proto: Protocol number + * @type: epoll type * @bind_addr: Address for binding, NULL for any * @ifname: Interface for binding, NULL for any * @port: Port, host order @@ -154,7 +158,7 @@ static int sock_l4_sa(const struct ctx *c, uint8_t proto, * * Return: newly created socket, negative error code on failure */ -int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto, +int sock_l4(const struct ctx *c, sa_family_t af, enum epoll_type type, const void *bind_addr, const char *ifname, uint16_t port, uint32_t data) { @@ -167,7 +171,7 @@ int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto, }; if (bind_addr) addr4.sin_addr = *(struct in_addr *)bind_addr; - return sock_l4_sa(c, proto, &addr4, sizeof(addr4), ifname, + return sock_l4_sa(c, type, &addr4, sizeof(addr4), ifname, false, data); } @@ -188,7 +192,7 @@ int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto, sizeof(c->ip6.addr_ll))) addr6.sin6_scope_id = c->ifi6; } - return sock_l4_sa(c, proto, &addr6, sizeof(addr6), ifname, + return sock_l4_sa(c, type, &addr6, sizeof(addr6), ifname, af == AF_INET6, data); } default: diff --git a/util.h b/util.h index eebb027b..d0150396 100644 --- a/util.h +++ b/util.h @@ -137,13 +137,14 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, #include #include +#include "epoll_type.h" #include "packet.h" struct ctx; /* cppcheck-suppress funcArgNamesDifferent */ __attribute__ ((weak)) int ffsl(long int i) { return __builtin_ffsl(i); } -int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto, +int sock_l4(const struct ctx *c, sa_family_t af, enum epoll_type type, const void *bind_addr, const char *ifname, uint16_t port, uint32_t data); void sock_probe_mem(struct ctx *c); -- 2.45.2

udp_buf_sock_handler() takes the epoll reference from the receiving socket, and passes the UDP relevant part on to several other functions. Future changes are going to need several different epoll types for UDP, and to pass that information through to some of those functions. To avoid extra noise in the patches making the real changes, change those functions now to take the full epoll reference, rather than just the UDP part. Signed-off-by: David Gibson --- udp.c | 63 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/udp.c b/udp.c index eadf4872..6301bda2 100644 --- a/udp.c +++ b/udp.c @@ -477,25 +477,26 @@ static int udp_splice_new_ns(void *arg) /** * udp_mmh_splice_port() - Is source address of message suitable for splicing? - * @uref: UDP epoll reference for incoming message's origin socket + * @ref: Epoll reference for incoming message's origin socket * @mmh: mmsghdr of incoming message * * Return: if source address of message in @mmh refers to localhost (127.0.0.1 * or ::1) its source port (host order), otherwise -1. */ -static int udp_mmh_splice_port(union udp_epoll_ref uref, - const struct mmsghdr *mmh) +static int udp_mmh_splice_port(union epoll_ref ref, const struct mmsghdr *mmh) { const struct sockaddr_in6 *sa6 = mmh->msg_hdr.msg_name; const struct sockaddr_in *sa4 = mmh->msg_hdr.msg_name; - if (!uref.splice) + ASSERT(ref.type == EPOLL_TYPE_UDP); + + if (!ref.udp.splice) return -1; - if (uref.v6 && IN6_IS_ADDR_LOOPBACK(&sa6->sin6_addr)) + if (ref.udp.v6 && IN6_IS_ADDR_LOOPBACK(&sa6->sin6_addr)) return ntohs(sa6->sin6_port); - if (!uref.v6 && IN4_IS_ADDR_LOOPBACK(&sa4->sin_addr)) + if (!ref.udp.v6 && IN4_IS_ADDR_LOOPBACK(&sa4->sin_addr)) return ntohs(sa4->sin_port); return -1; @@ -507,7 +508,7 @@ static int udp_mmh_splice_port(union udp_epoll_ref uref, * @start: Index of first datagram in udp[46]_l2_buf * @n: Total number of datagrams in udp[46]_l2_buf pool * @dst: Datagrams will be sent to this port (on destination side) - * @uref: UDP epoll reference for origin socket + * @ref: epoll reference for origin socket * @now: Timestamp * * This consumes as many datagrams as are sendable via a single socket. It @@ -518,7 +519,7 @@ static int udp_mmh_splice_port(union udp_epoll_ref uref, * Return: Number of datagrams forwarded */ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, - in_port_t dst, union udp_epoll_ref uref, + in_port_t dst, union epoll_ref ref, const struct timespec *now) { in_port_t src = udp_meta[start].splicesrc; @@ -527,8 +528,9 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, int s; ASSERT(udp_meta[start].splicesrc >= 0); + ASSERT(ref.type == EPOLL_TYPE_UDP); - if (uref.v6) { + if (ref.udp.v6) { mmh_recv = udp6_l2_mh_sock; mmh_send = udp6_mh_splice; udp6_localname.sin6_port = htons(dst); @@ -544,27 +546,27 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, if (++i >= n) break; - udp_meta[i].splicesrc = udp_mmh_splice_port(uref, &mmh_recv[i]); + udp_meta[i].splicesrc = udp_mmh_splice_port(ref, &mmh_recv[i]); } while (udp_meta[i].splicesrc == src); - if (uref.pif == PIF_SPLICE) { + if (ref.udp.pif == PIF_SPLICE) { src += c->udp.fwd_in.rdelta[src]; - s = udp_splice_init[uref.v6][src].sock; - if (s < 0 && uref.orig) - s = udp_splice_new(c, uref.v6, src, false); + s = udp_splice_init[ref.udp.v6][src].sock; + if (s < 0 && ref.udp.orig) + s = udp_splice_new(c, ref.udp.v6, src, false); if (s < 0) goto out; - udp_splice_ns[uref.v6][dst].ts = now->tv_sec; - udp_splice_init[uref.v6][src].ts = now->tv_sec; + udp_splice_ns[ref.udp.v6][dst].ts = now->tv_sec; + udp_splice_init[ref.udp.v6][src].ts = now->tv_sec; } else { - ASSERT(uref.pif == PIF_HOST); + ASSERT(ref.udp.pif == PIF_HOST); src += c->udp.fwd_out.rdelta[src]; - s = udp_splice_ns[uref.v6][src].sock; - if (s < 0 && uref.orig) { + s = udp_splice_ns[ref.udp.v6][src].sock; + if (s < 0 && ref.udp.orig) { struct udp_splice_new_ns_arg arg = { - c, uref.v6, src, -1, + c, ref.udp.v6, src, -1, }; NS_CALL(udp_splice_new_ns, &arg); @@ -573,8 +575,8 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, if (s < 0) goto out; - udp_splice_init[uref.v6][dst].ts = now->tv_sec; - udp_splice_ns[uref.v6][src].ts = now->tv_sec; + udp_splice_init[ref.udp.v6][dst].ts = now->tv_sec; + udp_splice_ns[ref.udp.v6][src].ts = now->tv_sec; } sendmmsg(s, mmh_send + start, i - start, MSG_NOSIGNAL); @@ -716,7 +718,7 @@ static size_t udp_update_hdr6(const struct ctx *c, * @start: Index of first datagram in udp[46]_l2_buf pool * @n: Total number of datagrams in udp[46]_l2_buf pool * @dstport: Destination port number on destination side - * @uref: UDP epoll reference for origin socket + * @ref: Epoll reference for origin socket * @now: Current timestamp * * This consumes as many frames as are sendable via tap. It requires that @@ -726,7 +728,7 @@ static size_t udp_update_hdr6(const struct ctx *c, * Return: Number of frames sent via tap */ static unsigned udp_tap_send(const struct ctx *c, size_t start, size_t n, - in_port_t dstport, union udp_epoll_ref uref, + in_port_t dstport, union epoll_ref ref, const struct timespec *now) { struct iovec (*tap_iov)[UDP_NUM_IOVS]; @@ -734,8 +736,9 @@ static unsigned udp_tap_send(const struct ctx *c, size_t start, size_t n, size_t i = start; ASSERT(udp_meta[start].splicesrc == -1); + ASSERT(ref.type == EPOLL_TYPE_UDP); - if (uref.v6) { + if (ref.udp.v6) { tap_iov = udp6_l2_iov_tap; mmh_recv = udp6_l2_mh_sock; } else { @@ -748,7 +751,7 @@ static unsigned udp_tap_send(const struct ctx *c, size_t start, size_t n, struct udp_meta_t *bm = &udp_meta[i]; size_t l4len; - if (uref.v6) { + if (ref.udp.v6) { l4len = udp_update_hdr6(c, &bm->ip6h, &bm->s_in.sa6, bp, dstport, udp6_l2_mh_sock[i].msg_len, now); @@ -766,7 +769,7 @@ static unsigned udp_tap_send(const struct ctx *c, size_t start, size_t n, if (++i >= n) break; - udp_meta[i].splicesrc = udp_mmh_splice_port(uref, &mmh_recv[i]); + udp_meta[i].splicesrc = udp_mmh_splice_port(ref, &mmh_recv[i]); } while (udp_meta[i].splicesrc == -1); tap_send_frames(c, &tap_iov[start][0], UDP_NUM_IOVS, i - start); @@ -823,12 +826,12 @@ void udp_buf_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t eve * present). So we fill in entry 0 before the loop, then udp_*_send() * populate one entry past where they consume. */ - udp_meta[0].splicesrc = udp_mmh_splice_port(ref.udp, mmh_recv); + udp_meta[0].splicesrc = udp_mmh_splice_port(ref, mmh_recv); for (i = 0; i < n; i += m) { if (udp_meta[i].splicesrc >= 0) - m = udp_splice_send(c, i, n, dstport, ref.udp, now); + m = udp_splice_send(c, i, n, dstport, ref, now); else - m = udp_tap_send(c, i, n, dstport, ref.udp, now); + m = udp_tap_send(c, i, n, dstport, ref, now); } } -- 2.45.2

We have separate mmsghdr arrays for splicing IPv4 and IPv6 packets, where the only difference is that they point to different sockaddr buffers for the destination address. Unify these by having the common array point at a sockaddr_inany as the address. This does mean slightly more work when we're about to splice, because we need to write the whole socket address, rather than just the port. However it removes 32 mmsghdr structures and we're going to need more flexibility constructing that target address for the flow table. Because future changes might mean that the address isn't always loopback, change the name of the common address from *_localname to udp_splicename. Signed-off-by: David Gibson --- udp.c | 47 ++++++++++++++++++++--------------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/udp.c b/udp.c index 663a2dce..b4ea2b35 100644 --- a/udp.c +++ b/udp.c @@ -235,18 +235,10 @@ static struct mmsghdr udp4_mh_recv [UDP_MAX_FRAMES]; static struct mmsghdr udp6_mh_recv [UDP_MAX_FRAMES]; /* IOVs and msghdr arrays for sending "spliced" datagrams to sockets */ -static struct sockaddr_in udp4_spliceto = { - .sin_family = AF_INET, - .sin_addr = IN4ADDR_LOOPBACK_INIT, -}; -static struct sockaddr_in6 udp6_spliceto = { - .sin6_family = AF_INET6, - .sin6_addr = IN6ADDR_LOOPBACK_INIT, -}; +static union sockaddr_inany udp_spliceto; static struct iovec udp_iov_splice [UDP_MAX_FRAMES]; -static struct mmsghdr udp4_mh_splice [UDP_MAX_FRAMES]; -static struct mmsghdr udp6_mh_splice [UDP_MAX_FRAMES]; +static struct mmsghdr udp_mh_splice [UDP_MAX_FRAMES]; /* IOVs for L2 frames */ static struct iovec udp4_l2_iov [UDP_MAX_FRAMES][UDP_NUM_IOVS]; @@ -523,7 +515,7 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, const struct timespec *now) { in_port_t src = udp_meta[start].splicesrc; - struct mmsghdr *mmh_recv, *mmh_send; + struct mmsghdr *mmh_recv; unsigned int i = start; int s; @@ -532,16 +524,22 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, if (ref.udp.v6) { mmh_recv = udp6_mh_recv; - mmh_send = udp6_mh_splice; - udp6_spliceto.sin6_port = htons(dst); + udp_spliceto.sa6 = (struct sockaddr_in6) { + .sin6_family = AF_INET6, + .sin6_addr = in6addr_loopback, + .sin6_port = htons(dst), + }; } else { mmh_recv = udp4_mh_recv; - mmh_send = udp4_mh_splice; - udp4_spliceto.sin_port = htons(dst); + udp_spliceto.sa4 = (struct sockaddr_in) { + .sin_family = AF_INET, + .sin_addr = in4addr_loopback, + .sin_port = htons(dst), + }; } do { - mmh_send[i].msg_hdr.msg_iov->iov_len = mmh_recv[i].msg_len; + udp_mh_splice[i].msg_hdr.msg_iov->iov_len = mmh_recv[i].msg_len; if (++i >= n) break; @@ -579,7 +577,7 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, udp_splice_ns[ref.udp.v6][src].ts = now->tv_sec; } - sendmmsg(s, mmh_send + start, i - start, MSG_NOSIGNAL); + sendmmsg(s, udp_mh_splice + start, i - start, MSG_NOSIGNAL); out: return i - start; } @@ -1094,20 +1092,15 @@ static void udp_splice_iov_init(void) int i; for (i = 0; i < UDP_MAX_FRAMES; i++) { - struct msghdr *mh4 = &udp4_mh_splice[i].msg_hdr; - struct msghdr *mh6 = &udp6_mh_splice[i].msg_hdr; - - mh4->msg_name = &udp4_spliceto; - mh4->msg_namelen = sizeof(udp4_spliceto); + struct msghdr *mh = &udp_mh_splice[i].msg_hdr; - mh6->msg_name = &udp6_spliceto; - mh6->msg_namelen = sizeof(udp6_spliceto); + mh->msg_name = &udp_spliceto; + mh->msg_namelen = sizeof(udp_spliceto); udp_iov_splice[i].iov_base = udp_payload[i].data; - mh4->msg_iov = &udp_iov_splice[i]; - mh6->msg_iov = &udp_iov_splice[i]; - mh4->msg_iovlen = mh6->msg_iovlen = 1; + mh->msg_iov = &udp_iov_splice[i]; + mh->msg_iovlen = 1; } } -- 2.45.2

Since we split our packet frame buffers into different pieces, we have a single buffer per IP version for the ethernet header, rather than one per frame. This makes sense since our ethernet header is alwaus the same. However we initialise those buffers udp[46]_eth_hdr inside a per frame loop. Pull that outside the loop so we just initialise them once. Signed-off-by: David Gibson --- udp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/udp.c b/udp.c index 454f9e74..c841d389 100644 --- a/udp.c +++ b/udp.c @@ -314,8 +314,6 @@ static void udp_iov_init_one(const struct ctx *c, size_t i) }; *siov = IOV_OF_LVALUE(payload->data); - udp4_eth_hdr.h_proto = htons_constant(ETH_P_IP); - udp6_eth_hdr.h_proto = htons_constant(ETH_P_IPV6); tiov[UDP_IOV_TAP] = tap_hdr_iov(c, &meta->taph); tiov[UDP_IOV_PAYLOAD].iov_base = payload; @@ -351,6 +349,9 @@ static void udp_iov_init(const struct ctx *c) { size_t i; + udp4_eth_hdr.h_proto = htons_constant(ETH_P_IP); + udp6_eth_hdr.h_proto = htons_constant(ETH_P_IPV6); + for (i = 0; i < UDP_MAX_FRAMES; i++) udp_iov_init_one(c, i); } -- 2.45.2

When we receive datagrams on a socket, we need to split them into batches depending on how they need to be forwarded (either via a specific splice socket, or via tap). The logic to do this, is somewhat awkwardly split between udp_buf_sock_handler() itself, udp_splice_send() and udp_tap_send(). Move all the batching logic into udp_buf_sock_handler(), leaving udp_splice_send() to just send the prepared batch. udp_tap_send() reduces to just a call to tap_send_frames() so open-code that call in udp_buf_sock_handler(). This will allow separating the batching logic from the rest of the datagram forwarding logic, which we'll need for upcoming flow table support. Signed-off-by: David Gibson --- udp.c | 128 ++++++++++++++++++---------------------------------------- 1 file changed, 39 insertions(+), 89 deletions(-) diff --git a/udp.c b/udp.c index 8ed59639..a317e986 100644 --- a/udp.c +++ b/udp.c @@ -501,42 +501,29 @@ static void udp_splice_prepare(struct mmsghdr *mmh, unsigned idx) } /** - * udp_splice_send() - Send datagrams from socket to socket + * udp_splice_send() - Send a batch of datagrams from socket to socket * @c: Execution context - * @start: Index of first datagram in udp[46]_l2_buf - * @n: Total number of datagrams in udp[46]_l2_buf pool - * @dst: Datagrams will be sent to this port (on destination side) + * @start: Index of batch's first datagram in udp[46]_l2_buf + * @n: Number of datagrams in batch + * @src: Source port for datagram (target side) + * @dst: Destination port for datagrams (target side) * @ref: epoll reference for origin socket * @now: Timestamp - * - * This consumes as many datagrams as are sendable via a single socket. It - * requires that udp_meta[@start].splicesrc is initialised, and will initialise - * udp_meta[].splicesrc for each datagram it consumes *and one more* (if - * present). - * - * Return: Number of datagrams forwarded */ -static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, - in_port_t dst, union epoll_ref ref, - const struct timespec *now) +static void udp_splice_send(const struct ctx *c, size_t start, size_t n, + in_port_t src, in_port_t dst, + union epoll_ref ref, + const struct timespec *now) { - in_port_t src = udp_meta[start].splicesrc; - struct mmsghdr *mmh_recv; - unsigned int i = start; int s; - ASSERT(udp_meta[start].splicesrc >= 0); - ASSERT(ref.type == EPOLL_TYPE_UDP); - if (ref.udp.v6) { - mmh_recv = udp6_mh_recv; udp_spliceto.sa6 = (struct sockaddr_in6) { .sin6_family = AF_INET6, .sin6_addr = in6addr_loopback, .sin6_port = htons(dst), }; } else { - mmh_recv = udp4_mh_recv; udp_spliceto.sa4 = (struct sockaddr_in) { .sin_family = AF_INET, .sin_addr = in4addr_loopback, @@ -544,15 +531,6 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, }; } - do { - udp_splice_prepare(mmh_recv, i); - - if (++i >= n) - break; - - udp_meta[i].splicesrc = udp_mmh_splice_port(ref, &mmh_recv[i]); - } while (udp_meta[i].splicesrc == src); - if (ref.udp.pif == PIF_SPLICE) { src += c->udp.fwd_in.rdelta[src]; s = udp_splice_init[ref.udp.v6][src].sock; @@ -560,7 +538,7 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, s = udp_splice_new(c, ref.udp.v6, src, false); if (s < 0) - goto out; + return; udp_splice_ns[ref.udp.v6][dst].ts = now->tv_sec; udp_splice_init[ref.udp.v6][src].ts = now->tv_sec; @@ -577,15 +555,13 @@ static unsigned udp_splice_send(const struct ctx *c, size_t start, size_t n, s = arg.s; } if (s < 0) - goto out; + return; udp_splice_init[ref.udp.v6][dst].ts = now->tv_sec; udp_splice_ns[ref.udp.v6][src].ts = now->tv_sec; } - sendmmsg(s, udp_mh_splice + start, i - start, MSG_NOSIGNAL); -out: - return i - start; + sendmmsg(s, udp_mh_splice + start, n, MSG_NOSIGNAL); } /** @@ -725,7 +701,7 @@ static size_t udp_update_hdr6(const struct ctx *c, * @v6: Prepare for IPv6? * @now: Current timestamp */ -static void udp_tap_prepare(const struct ctx *c, struct mmsghdr *mmh, +static void udp_tap_prepare(const struct ctx *c, const struct mmsghdr *mmh, unsigned idx, in_port_t dstport, bool v6, const struct timespec *now) { @@ -752,49 +728,6 @@ static void udp_tap_prepare(const struct ctx *c, struct mmsghdr *mmh, (*tap_iov)[UDP_IOV_PAYLOAD].iov_len = l4len; } -/** - * udp_tap_send() - Prepare UDP datagrams and send to tap interface - * @c: Execution context - * @start: Index of first datagram in udp[46]_l2_buf pool - * @n: Total number of datagrams in udp[46]_l2_buf pool - * @dstport: Destination port number on destination side - * @ref: Epoll reference for origin socket - * @now: Current timestamp - * - * This consumes as many frames as are sendable via tap. It requires that - * udp_meta[@start].splicesrc is initialised, and will initialise - * udp_meta[].splicesrc for each frame it consumes *and one more* (if present). - * - * Return: Number of frames sent via tap - */ -static unsigned udp_tap_send(const struct ctx *c, size_t start, size_t n, - in_port_t dstport, union epoll_ref ref, - const struct timespec *now) -{ - struct mmsghdr *mmh_recv; - size_t i = start; - - ASSERT(udp_meta[start].splicesrc == -1); - ASSERT(ref.type == EPOLL_TYPE_UDP); - - if (ref.udp.v6) - mmh_recv = udp6_mh_recv; - else - mmh_recv = udp4_mh_recv; - - do { - udp_tap_prepare(c, mmh_recv, i, dstport, ref.udp.v6, now); - - if (++i >= n) - break; - - udp_meta[i].splicesrc = udp_mmh_splice_port(ref, &mmh_recv[i]); - } while (udp_meta[i].splicesrc == -1); - - tap_send_frames(c, &udp_l2_iov[start][0], UDP_NUM_IOVS, i - start); - return i - start; -} - /** * udp_sock_recv() - Receive datagrams from a socket * @c: Execution context @@ -842,7 +775,7 @@ void udp_buf_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t eve { struct mmsghdr *mmh_recv = ref.udp.v6 ? udp6_mh_recv : udp4_mh_recv; in_port_t dstport = ref.udp.port; - int n, m, i; + int n, i; if ((n = udp_sock_recv(c, ref.fd, events, mmh_recv)) <= 0) return; @@ -852,19 +785,36 @@ void udp_buf_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t eve else if (ref.udp.pif == PIF_HOST) dstport += c->udp.fwd_in.f.delta[dstport]; - /* We divide things into batches based on how we need to send them, + /* We divide datagrams into batches based on how we need to send them, * determined by udp_meta[i].splicesrc. To avoid either two passes * through the array, or recalculating splicesrc for a single entry, we - * have to populate it one entry *ahead* of the loop counter (if - * present). So we fill in entry 0 before the loop, then udp_*_send() - * populate one entry past where they consume. + * have to populate it one entry *ahead* of the loop counter. */ udp_meta[0].splicesrc = udp_mmh_splice_port(ref, mmh_recv); - for (i = 0; i < n; i += m) { - if (udp_meta[i].splicesrc >= 0) - m = udp_splice_send(c, i, n, dstport, ref, now); + for (i = 0; i < n; ) { + int batchsrc = udp_meta[i].splicesrc; + int batchstart = i; + + do { + if (batchsrc >= 0) + udp_splice_prepare(mmh_recv, i); + else + udp_tap_prepare(c, mmh_recv, i, dstport, + ref.udp.v6, now); + + if (++i >= n) + break; + + udp_meta[i].splicesrc = udp_mmh_splice_port(ref, + &mmh_recv[i]); + } while (udp_meta[i].splicesrc == batchsrc); + + if (batchsrc >= 0) + udp_splice_send(c, batchstart, i - batchstart, + batchsrc, dstport, ref, now); else - m = udp_tap_send(c, i, n, dstport, ref, now); + tap_send_frames(c, &udp_l2_iov[batchstart][0], + UDP_NUM_IOVS, i - batchstart); } } -- 2.45.2

Add a test program verifying that we're able to discard datagrams from a socket without needing a big discard buffer, by using a zero length recv(). Signed-off-by: David Gibson --- contrib/udp-behaviour/.gitignore | 1 + contrib/udp-behaviour/Makefile | 6 +-- contrib/udp-behaviour/recv-zero.c | 74 +++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 contrib/udp-behaviour/recv-zero.c diff --git a/contrib/udp-behaviour/.gitignore b/contrib/udp-behaviour/.gitignore index c1baa98e..555031d8 100644 --- a/contrib/udp-behaviour/.gitignore +++ b/contrib/udp-behaviour/.gitignore @@ -1 +1,2 @@ /reuseaddr-priority +/recv-zero diff --git a/contrib/udp-behaviour/Makefile b/contrib/udp-behaviour/Makefile index 6e1d966c..82aaac29 100644 --- a/contrib/udp-behaviour/Makefile +++ b/contrib/udp-behaviour/Makefile @@ -3,8 +3,8 @@ # Copyright Red Hat # Author: David Gibson -TARGETS = reuseaddr-priority -SRCS = reuseaddr-priority.c +TARGETS = reuseaddr-priority recv-zero +SRCS = reuseaddr-priority.c recv-zero.c CFLAGS = -Wall all: cppcheck clang-tidy $(TARGETS:%=check-%) @@ -16,7 +16,7 @@ check-%: % cppcheck: cppcheck --std=c11 --error-exitcode=1 --enable=all --force \ - --check-level=exhaustive \ + --check-level=exhaustive --inline-suppr \ --inconclusive --library=posix --quiet \ --suppress=missingIncludeSystem \ $(SRCS) diff --git a/contrib/udp-behaviour/recv-zero.c b/contrib/udp-behaviour/recv-zero.c new file mode 100644 index 00000000..f161e5c2 --- /dev/null +++ b/contrib/udp-behaviour/recv-zero.c @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/* recv-zero.c + * + * Verify that we're able to discard datagrams by recv()ing into a zero-length + * buffer. + * + * Copyright Red Hat + * Author: David Gibson + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "common.h" + +#define DSTPORT 13257U + +/* 127.0.0.1:DSTPORT */ +static const struct sockaddr_in lo_dst = SOCKADDR_INIT(INADDR_LOOPBACK, DSTPORT); + +static void test_discard(void) +{ + long token1, token2; + int recv_s, send_s; + ssize_t rc; + + token1 = random(); + token2 = random(); + + recv_s = sock_reuseaddr(); + if (bind(recv_s, (struct sockaddr *)&lo_dst, sizeof(lo_dst)) < 0) + die("bind(): %s\n", strerror(errno)); + + send_s = sock_reuseaddr(); + if (connect(send_s, (struct sockaddr *)&lo_dst, sizeof(lo_dst)) < 0) + die("connect(): %s\n", strerror(errno)); + + send_token(send_s, token1); + send_token(send_s, token2); + + /* cppcheck-suppress nullPointer */ + rc = recv(recv_s, NULL, 0, MSG_DONTWAIT); + if (rc < 0) + die("discarding recv(): %s\n", strerror(errno)); + + recv_token(recv_s, token2); + + /* cppcheck-suppress nullPointer */ + rc = recv(recv_s, NULL, 0, MSG_DONTWAIT); + if (rc < 0 && errno != EAGAIN) + die("redundant discarding recv(): %s\n", strerror(errno)); + if (rc >= 0) + die("Unexpected receive: rc=%zd\n", rc); +} + +int main(int argc, char *argv[]) +{ + (void)argc; + (void)argv; + + test_discard(); + + printf("Discarding datagrams with a 0-length recv() seems to work\n"); + + exit(0); +} -- 2.45.2