On Tue, Apr 30, 2024 at 10:16:34PM +0200, Stefano Brivio wrote:On Tue, 30 Apr 2024 20:05:41 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:No, I don't think so. There's still a separate IPv4 and IPv6 header, and socket address for each frame. It's just that those are stored in a parallel array, rather than alongside the paylaod buffers. If we are handling multiple guests, we can probably additionally merge the IPv4 and IPv6 header buffers, since I don't think there will be anything we can pre-fill any more.Currently we have separate arrays for IPv4 and IPv6 which contain the headers for guest-bound packets, and also the originating socket address. We can combine these into a single array of "metadata" structures with space for both pre-cooked IPv4 and IPv6 headers, as well as shared space for the tap specific header and socket address (using sockaddr_inany).This is neat, definitely, I just wonder: will we need to essentially revert this if we implement a flow-based mechanism where frames can be sent to different guests/containers?On the other hand, chances are it would help instead because we would have tables of metadata instead of those being buried into frames.Adjusted. I also changed to udp_meta_t and udp_meta[] for consistency with the payload buffers.Because we're using IOVs to separately address the pieces of each frame, these structures don't need to be packed to keep the headers contiguous so we can more naturally arrange for the alignment we want. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- udp.c | 131 ++++++++++++++++++++++++---------------------------------- 1 file changed, 55 insertions(+), 76 deletions(-) diff --git a/udp.c b/udp.c index 64e7dcef..a9cc6ed2 100644 --- a/udp.c +++ b/udp.c @@ -184,44 +184,27 @@ udp_payload_buf[UDP_MAX_FRAMES]; /* Ethernet header for IPv4 frames */ static struct ethhdr udp4_eth_hdr; -/** - * udp4_l2_buf_t - Pre-cooked IPv4 packet buffers for tap connections - * @s_in: Source socket address, filled in by recvmmsg() - * @taph: Tap backend specific header - * @iph: Pre-filled IP header (except for tot_len and saddr) - */ -static struct udp4_l2_buf_t { - struct sockaddr_in s_in; - - struct tap_hdr taph; - struct iphdr iph; -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))) -udp4_l2_buf[UDP_MAX_FRAMES]; - /* Ethernet header for IPv6 frames */ static struct ethhdr udp6_eth_hdr; /** - * udp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections - * @s_in6: Source socket address, filled in by recvmmsg() + * udp_meta - Pre-cooked headers and metadata for UDP packetsPre-existing, but... kernel-doc format wants *struct* x - Description.Fixed. -- David Gibson | 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+ * @ip6h: Pre-filled IPv6 header (except for payload_len and addresses) + * @ip4h: Pre-filled IPv4 header (except for tot_len and saddr) * @taph: Tap backend specific header - * @ip6h: Pre-filled IP header (except for payload_len and addresses) + * @s_in: Source socket address, filled in by recvmmsg() */ -struct udp6_l2_buf_t { - struct sockaddr_in6 s_in6; -#ifdef __AVX2__ - /* Align ip6h to 32-byte boundary. */ - uint8_t pad[64 - (sizeof(struct sockaddr_in6) + sizeof(struct tap_hdr))]; -#endif - - struct tap_hdr taph; +static struct udp_meta { struct ipv6hdr ip6h; + struct iphdr ip4h; + struct tap_hdr taph; + + union sockaddr_inany s_in; +} #ifdef __AVX2__ -} __attribute__ ((packed, aligned(32))) -#else -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))) +__attribute__ ((aligned(32))) #endif -udp6_l2_buf[UDP_MAX_FRAMES]; +udp_meta_buf[UDP_MAX_FRAMES]; /** * enum udp_iov_idx - Indices for the buffers making up a single UDP frame @@ -315,49 +298,46 @@ void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s) static void udp_iov_init_one(const struct ctx *c, size_t i) { struct udp_payload *payload = &udp_payload_buf[i]; + struct udp_meta *meta = &udp_meta_buf[i]; struct iovec *siov = &udp_l2_iov_sock[i]; + *meta = (struct udp_meta) { + .ip4h = L2_BUF_IP4_INIT(IPPROTO_UDP), + .ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP), + }; + +One extra newline should be enough.