[PATCH v5 0/2] tcp: unify IPv4 and IPv6 tap queues
This should save us some memory and code. --- v2: - Setting pointers to pre-set IP and MAC headers on the fly instead of copying them. - Merged patch #2 and #3 from v1 v3: - Changes based on feedback from team v4: - Rebased v5: - Changes based on feedback from team and rebased Jon Maloy (2): tcp: set ip and eth headers in l2 tap queues on the fly tcp: unify l2 TCPv4 and TCPv6 queues and structures tcp.c | 6 +- tcp_buf.c | 272 ++++++++++++++++++------------------------------------ tcp_buf.h | 3 +- 3 files changed, 91 insertions(+), 190 deletions(-) -- 2.45.2
l2 tap queue entries are currently initialized at system start, and
reused with preset headers through its whole life time. The only
fields we need to update per message are things like payload size
and checksums.
If we want to reuse these entries between ipv4 and ipv6 messages we
will need to set the pointer to the right header on the fly per
message, since the header type may differ between entries in the same
queue.
The same needs to be done for the ethernet header.
We do these changes here.
Signed-off-by: Jon Maloy
On Mon, Oct 28, 2024 at 10:13:59PM -0400, Jon Maloy wrote:
l2 tap queue entries are currently initialized at system start, and reused with preset headers through its whole life time. The only fields we need to update per message are things like payload size and checksums.
If we want to reuse these entries between ipv4 and ipv6 messages we will need to set the pointer to the right header on the fly per message, since the header type may differ between entries in the same queue.
The same needs to be done for the ethernet header.
We do these changes here.
Signed-off-by: Jon Maloy
Reviewed-by: David Gibson
--- v2: Setting pointers to pre-initialized IP and MAC headers instead of copying them in on the fly. v3: Adapted to D. Gibson's recent commit eliminitaing overlapping memcpy() v5: Fixed two bugs as per feedback from D. Gibson. --- tcp_buf.c | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/tcp_buf.c b/tcp_buf.c index cb6742c..e249c6b 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -130,8 +130,7 @@ void tcp_sock4_iov_init(const struct ctx *c) iov = tcp4_l2_iov[i];
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_ETH].iov_len = sizeof(struct ethhdr); iov[TCP_IOV_PAYLOAD].iov_base = &tcp4_payload[i]; }
@@ -173,8 +172,7 @@ void tcp_sock6_iov_init(const struct ctx *c) iov = tcp6_l2_iov[i];
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_ETH].iov_len = sizeof(struct ethhdr); iov[TCP_IOV_PAYLOAD].iov_base = &tcp6_payload[i]; }
@@ -273,11 +271,17 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) uint32_t seq; int ret;
- if (CONN_V4(conn)) - iov = tcp4_l2_flags_iov[tcp4_flags_used++]; - else - iov = tcp6_l2_flags_iov[tcp6_flags_used++]; - + if (CONN_V4(conn)) { + iov = tcp4_l2_flags_iov[tcp4_flags_used]; + iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_flags_ip[tcp4_flags_used]); + iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src; + tcp4_flags_used++; + } else { + iov = tcp6_l2_flags_iov[tcp6_flags_used]; + iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_flags_ip[tcp6_flags_used]); + iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src; + tcp6_flags_used++; + } payload = iov[TCP_IOV_PAYLOAD].iov_base;
seq = conn->seq_to_tap; @@ -296,21 +300,19 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
if (flags & DUP_ACK) { struct iovec *dup_iov; - int i;
if (CONN_V4(conn)) dup_iov = tcp4_l2_flags_iov[tcp4_flags_used++]; else dup_iov = tcp6_l2_flags_iov[tcp6_flags_used++];
- for (i = 0; i < TCP_NUM_IOVS; i++) { - /* All frames share the same ethernet header buffer */ - if (i != TCP_IOV_ETH) { - memcpy(dup_iov[i].iov_base, iov[i].iov_base, - iov[i].iov_len); - } - } - dup_iov[TCP_IOV_PAYLOAD].iov_len = iov[TCP_IOV_PAYLOAD].iov_len; + memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base, + iov[TCP_IOV_TAP].iov_len); + dup_iov[TCP_IOV_ETH].iov_base = iov[TCP_IOV_ETH].iov_base; + dup_iov[TCP_IOV_IP] = iov[TCP_IOV_IP]; + memcpy(dup_iov[TCP_IOV_PAYLOAD].iov_base, + iov[TCP_IOV_PAYLOAD].iov_base, l4len); + dup_iov[TCP_IOV_PAYLOAD].iov_len = l4len; }
if (CONN_V4(conn)) { @@ -350,8 +352,10 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, }
tcp4_frame_conns[tcp4_payload_used] = conn; - - iov = tcp4_l2_iov[tcp4_payload_used++]; + iov = tcp4_l2_iov[tcp4_payload_used]; + iov[TCP_IOV_IP] = + IOV_OF_LVALUE(tcp4_payload_ip[tcp4_payload_used++]); + iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src; l4len = tcp_l2_buf_fill_headers(conn, iov, dlen, check, seq, false); iov[TCP_IOV_PAYLOAD].iov_len = l4len; @@ -359,8 +363,10 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, tcp_payload_flush(c); } else if (CONN_V6(conn)) { tcp6_frame_conns[tcp6_payload_used] = conn; - - iov = tcp6_l2_iov[tcp6_payload_used++]; + iov = tcp6_l2_iov[tcp6_payload_used]; + iov[TCP_IOV_IP] = + IOV_OF_LVALUE(tcp6_payload_ip[tcp6_payload_used++]); + iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src; l4len = tcp_l2_buf_fill_headers(conn, iov, dlen, NULL, seq, false); iov[TCP_IOV_PAYLOAD].iov_len = l4len;
-- 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
Following the preparations in the previous commit, we can now remove
the payload and flag queues dedicated for TCPv6 and TCPv4 and move all
traffic into common queues handling both protocol types.
Apart from reducing code and memory footprint, this change reduces
a potential risk for TCPv4 traffic starving out TCPv6 traffic.
Since we always flush out the TCPv4 frame queue before the TCPv6 queue,
the latter will never be handled if the former fails to send all its
frames.
Tests with iperf3 shows no measurable change in performance after this
change.
Reviewed-by: David Gibson
On Mon, Oct 28, 2024 at 10:13:58PM -0400, Jon Maloy wrote:
This should save us some memory and code.
LGTM. I'll rebase my IOV re-orgs on top of these.
--- v2: - Setting pointers to pre-set IP and MAC headers on the fly instead of copying them. - Merged patch #2 and #3 from v1 v3: - Changes based on feedback from team v4: - Rebased v5: - Changes based on feedback from team and rebased
Jon Maloy (2): tcp: set ip and eth headers in l2 tap queues on the fly tcp: unify l2 TCPv4 and TCPv6 queues and structures
tcp.c | 6 +- tcp_buf.c | 272 ++++++++++++++++++------------------------------------ tcp_buf.h | 3 +- 3 files changed, 91 insertions(+), 190 deletions(-)
-- 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
On Mon, 28 Oct 2024 22:13:58 -0400
Jon Maloy
This should save us some memory and code.
--- v2: - Setting pointers to pre-set IP and MAC headers on the fly instead of copying them. - Merged patch #2 and #3 from v1 v3: - Changes based on feedback from team v4: - Rebased v5: - Changes based on feedback from team and rebased
Jon Maloy (2): tcp: set ip and eth headers in l2 tap queues on the fly tcp: unify l2 TCPv4 and TCPv6 queues and structures
tcp.c | 6 +- tcp_buf.c | 272 ++++++++++++++++++------------------------------------ tcp_buf.h | 3 +- 3 files changed, 91 insertions(+), 190 deletions(-)
Applied. I also tested it with SO_PEEK_OFF support (kernel from current net.git HEAD), no issues observed. -- Stefano
participants (3)
-
David Gibson
-
Jon Maloy
-
Stefano Brivio