On Fri, Sep 06, 2024 at 05:34:24PM -0400, Jon Maloy wrote:As a preparation for unifying the l2 queues for TCPv4 and TCPv6 traffic we prepare a unified struct that has space for both IP address types. With this change, the arrays tcp4_l2_iov and tcp4_l2_iov, as well as tcp4_l2_flags_iov and tcp4_l2_flags_iov, are identical in structure and size. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tcp_buf.c | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/tcp_buf.c b/tcp_buf.c index c31e9f3..1af4786 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -52,6 +52,18 @@ struct tcp_payload_t { } __attribute__ ((packed, aligned(__alignof__(unsigned int)))); #endif +/** + * struct iphdr_t - Area with space for both IPv4 and IPv6 header + * @ip4: IPv4 header + * @ip6: IPv6 header + */ +struct iphdr_t { + union { + struct iphdr ip4; + struct ipv6hdr ip6; + }; +};So, I haven't read the rest of the series yet, so it's possible something there would clarify this. Having the two headers in a union does require that essentially the whole header be re-initialized for every packet. For UDP, what we do instead is have both headers in paralllel, partially initialised. For each packet we update the appropriate header as needed, and point our IOV to the correct header (v4 or v6). I don't know if that's a better approach or not: it's plausible the dcache savings outweigh the extra re-initialisation, but it's plausible they don't too. I wonder if it might be better sticking with the same approach as UDP for consistency, at least until we have specific data suggesting one or the other approach is better.+ /** * struct tcp_flags_t - TCP header and data to send zero-length * segments (flags) @@ -72,7 +84,7 @@ static struct ethhdr tcp4_eth_src; static struct tap_hdr tcp4_payload_tap_hdr[TCP_FRAMES_MEM]; /* IPv4 headers */ -static struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; +static struct iphdr_t tcp4_payload_ip[TCP_FRAMES_MEM]; /* TCP segments with payload for IPv4 frames */ static struct tcp_payload_t tcp4_payload[TCP_FRAMES_MEM]; @@ -84,7 +96,7 @@ static unsigned int tcp4_payload_used; static struct tap_hdr tcp4_flags_tap_hdr[TCP_FRAMES_MEM]; /* IPv4 headers for TCP segment without payload */ -static struct iphdr tcp4_flags_ip[TCP_FRAMES_MEM]; +static struct iphdr_t tcp4_flags_ip[TCP_FRAMES_MEM]; /* TCP segments without payload for IPv4 frames */ static struct tcp_flags_t tcp4_flags[TCP_FRAMES_MEM]; @@ -95,7 +107,7 @@ static struct ethhdr tcp6_eth_src; static struct tap_hdr tcp6_payload_tap_hdr[TCP_FRAMES_MEM]; /* IPv6 headers */ -static struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM]; +static struct iphdr_t tcp6_payload_ip[TCP_FRAMES_MEM]; /* TCP headers and data for IPv6 frames */ static struct tcp_payload_t tcp6_payload[TCP_FRAMES_MEM]; @@ -107,7 +119,7 @@ static unsigned int tcp6_payload_used; static struct tap_hdr tcp6_flags_tap_hdr[TCP_FRAMES_MEM]; /* IPv6 headers for TCP segment without payload */ -static struct ipv6hdr tcp6_flags_ip[TCP_FRAMES_MEM]; +static struct iphdr_t tcp6_flags_ip[TCP_FRAMES_MEM]; /* TCP segment without payload for IPv6 frames */ static struct tcp_flags_t tcp6_flags[TCP_FRAMES_MEM]; @@ -144,13 +156,13 @@ void tcp_sock4_iov_init(const struct ctx *c) tcp4_eth_src.h_proto = htons_constant(ETH_P_IP); for (i = 0; i < ARRAY_SIZE(tcp4_payload); i++) { - tcp4_payload_ip[i] = iph; + tcp4_payload_ip[i].ip4 = iph; tcp4_payload[i].th.doff = sizeof(struct tcphdr) / 4; tcp4_payload[i].th.ack = 1; } for (i = 0; i < ARRAY_SIZE(tcp4_flags); i++) { - tcp4_flags_ip[i] = iph; + tcp4_flags_ip[i].ip4 = iph; tcp4_flags[i].th.doff = sizeof(struct tcphdr) / 4; tcp4_flags[i].th.ack = 1; } @@ -160,7 +172,8 @@ void tcp_sock4_iov_init(const struct ctx *c) iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp4_payload_tap_hdr[i]); iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp4_eth_src); - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_payload_ip[i]); + iov[TCP_IOV_IP].iov_base = &tcp4_payload_ip[i]; + iov[TCP_IOV_IP].iov_len = sizeof(tcp4_payload_ip[i].ip4); iov[TCP_IOV_PAYLOAD].iov_base = &tcp4_payload[i]; } @@ -170,7 +183,8 @@ void tcp_sock4_iov_init(const struct ctx *c) iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp4_flags_tap_hdr[i]); iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src; iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp4_eth_src); - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_flags_ip[i]); + iov[TCP_IOV_IP].iov_base = &tcp4_flags_ip[i]; + iov[TCP_IOV_IP].iov_len = sizeof(tcp4_flags_ip[i].ip4); iov[TCP_IOV_PAYLOAD].iov_base = &tcp4_flags[i]; } } @@ -188,13 +202,13 @@ void tcp_sock6_iov_init(const struct ctx *c) tcp6_eth_src.h_proto = htons_constant(ETH_P_IPV6); for (i = 0; i < ARRAY_SIZE(tcp6_payload); i++) { - tcp6_payload_ip[i] = ip6; + tcp6_payload_ip[i].ip6 = ip6; tcp6_payload[i].th.doff = sizeof(struct tcphdr) / 4; tcp6_payload[i].th.ack = 1; } for (i = 0; i < ARRAY_SIZE(tcp6_flags); i++) { - tcp6_flags_ip[i] = ip6; + tcp6_flags_ip[i].ip6 = ip6; tcp6_flags[i].th.doff = sizeof(struct tcphdr) / 4; tcp6_flags[i].th .ack = 1; } @@ -204,7 +218,8 @@ void tcp_sock6_iov_init(const struct ctx *c) iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp6_payload_tap_hdr[i]); iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp6_eth_src); - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[i]); + iov[TCP_IOV_IP].iov_base = &tcp6_payload_ip[i]; + iov[TCP_IOV_IP].iov_len = sizeof(tcp6_payload_ip[i].ip6); iov[TCP_IOV_PAYLOAD].iov_base = &tcp6_payload[i]; } @@ -213,7 +228,8 @@ void tcp_sock6_iov_init(const struct ctx *c) iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp6_flags_tap_hdr[i]); iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp6_eth_src); - iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_flags_ip[i]); + iov[TCP_IOV_IP].iov_base = &tcp6_flags_ip[i]; + iov[TCP_IOV_IP].iov_len = sizeof(tcp6_flags_ip[i].ip6); iov[TCP_IOV_PAYLOAD].iov_base = &tcp6_flags[i]; } }-- David Gibson (he or they) | 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