Currently these expects both the TCP header and payload in a single IOV, and goes to some trouble to locate the checksum field within it. In the current caller we've already know where the TCP header is, so we might as well just pass it in. This will need to work a bit differently for vhost-user, but that code already needs to locate the TCP header for other reasons, so again we can just pass it in. Signed-off-by: David Gibson --- tcp.c | 104 ++++++++++++++-------------------------------------------- 1 file changed, 24 insertions(+), 80 deletions(-) diff --git a/tcp.c b/tcp.c index 0569dc6..f2898ff 100644 --- a/tcp.c +++ b/tcp.c @@ -753,104 +753,48 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s) /** * tcp_update_check_tcp4() - Calculate TCP checksum for IPv4 * @iph: IPv4 header - * @iov: Pointer to the array of IO vectors - * @iov_cnt: Length of the array - * @l4offset: IPv4 payload offset in the iovec array + * @th: TCP header (updated) + * @iov: IO vector containing the TCP payload + * @iov_cnt: Length of @iov + * @doffset: TCP payload offset in @iov */ -static void tcp_update_check_tcp4(const struct iphdr *iph, +static void tcp_update_check_tcp4(const struct iphdr *iph, struct tcphdr *th, const struct iovec *iov, int iov_cnt, - size_t l4offset) + size_t doffset) { uint16_t l4len = ntohs(iph->tot_len) - sizeof(struct iphdr); struct in_addr saddr = { .s_addr = iph->saddr }; struct in_addr daddr = { .s_addr = iph->daddr }; - size_t check_ofs; - __sum16 *check; - int check_idx; uint32_t sum; - char *ptr; sum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, saddr, daddr); - check_idx = iov_skip_bytes(iov, iov_cnt, - l4offset + offsetof(struct tcphdr, check), - &check_ofs); - - if (check_idx >= iov_cnt) { - err("TCP4 buffer is too small, iov size %zd, check offset %zd", - iov_size(iov, iov_cnt), - l4offset + offsetof(struct tcphdr, check)); - return; - } - - if (check_ofs + sizeof(*check) > iov[check_idx].iov_len) { - err("TCP4 checksum field memory is not contiguous " - "check_ofs %zd check_idx %d iov_len %zd", - check_ofs, check_idx, iov[check_idx].iov_len); - return; - } - - ptr = (char *)iov[check_idx].iov_base + check_ofs; - if ((uintptr_t)ptr & (__alignof__(*check) - 1)) { - err("TCP4 checksum field is not correctly aligned in memory"); - return; - } - - check = (__sum16 *)ptr; - - *check = 0; - *check = csum_iov(iov, iov_cnt, l4offset, sum); + th->check = 0; + sum = csum_unfolded(th, sizeof(*th), sum); + th->check = csum_iov(iov, iov_cnt, doffset, sum); } /** * tcp_update_check_tcp6() - Calculate TCP checksum for IPv6 * @ip6h: IPv6 header - * @iov: Pointer to the array of IO vectors - * @iov_cnt: Length of the array - * @l4offset: IPv6 payload offset in the iovec array + * @th: TCP header (updated) + * @iov: IO vector containing the TCP payload + * @iov_cnt: Length of @iov + * @doffset: TCP payload offset in @iov */ -static void tcp_update_check_tcp6(const struct ipv6hdr *ip6h, +static void tcp_update_check_tcp6(const struct ipv6hdr *ip6h, struct tcphdr *th, const struct iovec *iov, int iov_cnt, - size_t l4offset) + size_t doffset) { uint16_t l4len = ntohs(ip6h->payload_len); - size_t check_ofs; - __sum16 *check; - int check_idx; uint32_t sum; - char *ptr; sum = proto_ipv6_header_psum(l4len, IPPROTO_TCP, &ip6h->saddr, &ip6h->daddr); - check_idx = iov_skip_bytes(iov, iov_cnt, - l4offset + offsetof(struct tcphdr, check), - &check_ofs); - - if (check_idx >= iov_cnt) { - err("TCP6 buffer is too small, iov size %zd, check offset %zd", - iov_size(iov, iov_cnt), - l4offset + offsetof(struct tcphdr, check)); - return; - } - - if (check_ofs + sizeof(*check) > iov[check_idx].iov_len) { - err("TCP6 checksum field memory is not contiguous " - "check_ofs %zd check_idx %d iov_len %zd", - check_ofs, check_idx, iov[check_idx].iov_len); - return; - } - - ptr = (char *)iov[check_idx].iov_base + check_ofs; - if ((uintptr_t)ptr & (__alignof__(*check) - 1)) { - err("TCP6 checksum field is not correctly aligned in memory"); - return; - } - - check = (__sum16 *)ptr; - - *check = 0; - *check = csum_iov(iov, iov_cnt, l4offset, sum); + th->check = 0; + sum = csum_unfolded(th, sizeof(*th), sum); + th->check = csum_iov(iov, iov_cnt, doffset, sum); } /** @@ -1005,11 +949,11 @@ static size_t tcp_fill_headers4(const struct tcp_tap_conn *conn, bp->th.check = 0; } else { const struct iovec iov = { - .iov_base = bp, - .iov_len = ntohs(iph->tot_len) - sizeof(struct iphdr), + .iov_base = bp->data, + .iov_len = dlen, }; - tcp_update_check_tcp4(iph, &iov, 1, 0); + tcp_update_check_tcp4(iph, &bp->th, &iov, 1, 0); } tap_hdr_update(taph, l3len + sizeof(struct ethhdr)); @@ -1056,11 +1000,11 @@ static size_t tcp_fill_headers6(const struct tcp_tap_conn *conn, bp->th.check = 0; } else { const struct iovec iov = { - .iov_base = bp, - .iov_len = ntohs(ip6h->payload_len) + .iov_base = bp->data, + .iov_len = dlen, }; - tcp_update_check_tcp6(ip6h, &iov, 1, 0); + tcp_update_check_tcp6(ip6h, &bp->th, &iov, 1, 0); } tap_hdr_update(taph, l4len + sizeof(*ip6h) + sizeof(struct ethhdr)); -- 2.47.0

tcp_l2_buf_fill_headers() is always followed by updating the payload IOV entry to the correct length of the frame. It already needs knowledge of the frame/IOV layout, so we might as well perform that update inside the function. Rename it to tcp_buf_make_frame() to reflect its expanded duties. While we're there use some temporaries to make our dissection of the IOV a bit clearer. Signed-off-by: David Gibson --- tcp_buf.c | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/tcp_buf.c b/tcp_buf.c index dbe565c..deb7be4 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -257,35 +257,38 @@ void tcp_payload_flush(const struct ctx *c) } /** - * tcp_buf_fill_headers() - Fill 802.3, IP, TCP headers in pre-cooked buffers + * tcp_buf_make_frame() - Adjust IOV, complete headers to build a TCP frame * @conn: Connection pointer * @iov: Pointer to an array of iovec of TCP pre-cooked buffers * @dlen: TCP payload length * @check: Checksum, if already known * @seq: Sequence number for this segment * @no_tcp_csum: Do not set TCP checksum - * - * Return: IP payload length, host order */ -static size_t tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn, - struct iovec *iov, size_t dlen, - const uint16_t *check, uint32_t seq, - bool no_tcp_csum) +static void tcp_buf_make_frame(const struct tcp_tap_conn *conn, + struct iovec *iov, size_t dlen, + const uint16_t *check, uint32_t seq, + bool no_tcp_csum) { + struct tcp_payload_t *payload = iov[TCP_IOV_PAYLOAD].iov_base; + struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base; const struct flowside *tapside = TAPFLOW(conn); const struct in_addr *a4 = inany_v4(&tapside->oaddr); + size_t l4len; if (a4) { - return tcp_fill_headers4(conn, iov[TCP_IOV_TAP].iov_base, - iov[TCP_IOV_IP].iov_base, - iov[TCP_IOV_PAYLOAD].iov_base, dlen, - check, seq, no_tcp_csum); + struct iphdr *iph = iov[TCP_IOV_IP].iov_base; + + l4len = tcp_fill_headers4(conn, taph, iph, payload, dlen, + check, seq, no_tcp_csum); + } else { + struct ipv6hdr *ip6h = iov[TCP_IOV_IP].iov_base; + + l4len = tcp_fill_headers6(conn, taph, ip6h, payload, dlen, + seq, no_tcp_csum); } - return tcp_fill_headers6(conn, iov[TCP_IOV_TAP].iov_base, - iov[TCP_IOV_IP].iov_base, - iov[TCP_IOV_PAYLOAD].iov_base, dlen, - seq, no_tcp_csum); + iov[TCP_IOV_PAYLOAD].iov_len = l4len; } /** @@ -301,7 +304,6 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) struct tcp_flags_t *payload; struct iovec *iov; size_t optlen; - size_t l4len; uint32_t seq; int ret; @@ -323,8 +325,7 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) return ret; } - l4len = tcp_l2_buf_fill_headers(conn, iov, optlen, NULL, seq, false); - iov[TCP_IOV_PAYLOAD].iov_len = l4len; + tcp_buf_make_frame(conn, iov, optlen, NULL, seq, false); if (flags & DUP_ACK) { struct iovec *dup_iov; @@ -368,7 +369,6 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, ssize_t dlen, int no_csum, uint32_t seq) { struct iovec *iov; - size_t l4len; conn->seq_to_tap = seq + dlen; @@ -384,18 +384,14 @@ 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++]; - l4len = tcp_l2_buf_fill_headers(conn, iov, dlen, check, seq, - false); - iov[TCP_IOV_PAYLOAD].iov_len = l4len; + tcp_buf_make_frame(conn, iov, dlen, check, seq, false); if (tcp4_payload_used > TCP_FRAMES_MEM - 1) tcp_payload_flush(c); } else if (CONN_V6(conn)) { tcp6_frame_conns[tcp6_payload_used] = conn; iov = tcp6_l2_iov[tcp6_payload_used++]; - l4len = tcp_l2_buf_fill_headers(conn, iov, dlen, NULL, seq, - false); - iov[TCP_IOV_PAYLOAD].iov_len = l4len; + tcp_buf_make_frame(conn, iov, dlen, NULL, seq, false); if (tcp6_payload_used > TCP_FRAMES_MEM - 1) tcp_payload_flush(c); } -- 2.47.0

At the moment these take separate pointers to the tap specific and IP headers, but expect the TCP header and payload as a single tcp_payload_t. As well as being slightly inconsistent, this involves some slightly iffy pointer shenanigans when called on the flags path with a tcp_flags_t instead of a tcp_payload_t. More importantly, it's inconvenient for the upcoming vhost-user case, where the TCP header and payload might not be contiguous. Furthermore, the payload itself might not be contiguous. So, pass the TCP header as its own pointer, and the TCP payload as an IO vector. Signed-off-by: David Gibson --- tcp.c | 56 +++++++++++++++++++++++--------------------------- tcp_buf.c | 7 ++++--- tcp_internal.h | 6 ++++-- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/tcp.c b/tcp.c index d1e71ec..787bc19 100644 --- a/tcp.c +++ b/tcp.c @@ -914,21 +914,25 @@ static void tcp_fill_header(struct tcphdr *th, * @conn: Connection pointer * @taph: tap backend specific header * @iph: Pointer to IPv4 header - * @bp: Pointer to TCP header followed by TCP payload - * @dlen: TCP payload length + * @th: Pointer to TCP header + * @iov: IO vector containing payload + * @iov_cnt: Number of entries in @iov + * @doffset: Offset of the TCP payload within @iov * @check: Checksum, if already known * @seq: Sequence number for this segment * @no_tcp_csum: Do not set TCP checksum */ void tcp_fill_headers4(const struct tcp_tap_conn *conn, struct tap_hdr *taph, struct iphdr *iph, - struct tcp_payload_t *bp, size_t dlen, + struct tcphdr *th, + const struct iovec *iov, size_t iov_cnt, size_t doffset, const uint16_t *check, uint32_t seq, bool no_tcp_csum) { const struct flowside *tapside = TAPFLOW(conn); const struct in_addr *src4 = inany_v4(&tapside->oaddr); const struct in_addr *dst4 = inany_v4(&tapside->eaddr); - size_t l4len = dlen + sizeof(bp->th); + size_t dlen = iov_size(iov, iov_cnt) - doffset; + size_t l4len = dlen + sizeof(*th); size_t l3len = l4len + sizeof(*iph); ASSERT(src4 && dst4); @@ -940,18 +944,12 @@ void tcp_fill_headers4(const struct tcp_tap_conn *conn, iph->check = check ? *check : csum_ip4_header(l3len, IPPROTO_TCP, *src4, *dst4); - tcp_fill_header(&bp->th, conn, seq); + tcp_fill_header(th, conn, seq); - if (no_tcp_csum) { - bp->th.check = 0; - } else { - const struct iovec iov = { - .iov_base = bp->data, - .iov_len = dlen, - }; - - tcp_update_check_tcp4(iph, &bp->th, &iov, 1, 0); - } + if (no_tcp_csum) + th->check = 0; + else + tcp_update_check_tcp4(iph, th, iov, iov_cnt, doffset); tap_hdr_update(taph, l3len + sizeof(struct ethhdr)); } @@ -961,19 +959,23 @@ void tcp_fill_headers4(const struct tcp_tap_conn *conn, * @conn: Connection pointer * @taph: tap backend specific header * @ip6h: Pointer to IPv6 header - * @bp: Pointer to TCP header followed by TCP payload - * @dlen: TCP payload length + * @th: Pointer to TCP header + * @iov: IO vector containing payload + * @iov_cnt: Number of entries in @iov + * @doffset: Offset of the TCP payload within @iov * @check: Checksum, if already known * @seq: Sequence number for this segment * @no_tcp_csum: Do not set TCP checksum */ void tcp_fill_headers6(const struct tcp_tap_conn *conn, struct tap_hdr *taph, struct ipv6hdr *ip6h, - struct tcp_payload_t *bp, size_t dlen, + struct tcphdr *th, + const struct iovec *iov, size_t iov_cnt, size_t doffset, uint32_t seq, bool no_tcp_csum) { const struct flowside *tapside = TAPFLOW(conn); - size_t l4len = dlen + sizeof(bp->th); + size_t dlen = iov_size(iov, iov_cnt) - doffset; + size_t l4len = dlen + sizeof(*th); ip6h->payload_len = htons(l4len); ip6h->saddr = tapside->oaddr.a6; @@ -987,18 +989,12 @@ void tcp_fill_headers6(const struct tcp_tap_conn *conn, ip6h->flow_lbl[1] = (conn->sock >> 8) & 0xff; ip6h->flow_lbl[2] = (conn->sock >> 0) & 0xff; - tcp_fill_header(&bp->th, conn, seq); + tcp_fill_header(th, conn, seq); - if (no_tcp_csum) { - bp->th.check = 0; - } else { - const struct iovec iov = { - .iov_base = bp->data, - .iov_len = dlen, - }; - - tcp_update_check_tcp6(ip6h, &bp->th, &iov, 1, 0); - } + if (no_tcp_csum) + th->check = 0; + else + tcp_update_check_tcp6(ip6h, th, iov, iov_cnt, doffset); tap_hdr_update(taph, l4len + sizeof(*ip6h) + sizeof(struct ethhdr)); } diff --git a/tcp_buf.c b/tcp_buf.c index b2d806a..c03dd33 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -270,8 +270,9 @@ static void tcp_buf_make_frame(const struct tcp_tap_conn *conn, const uint16_t *check, uint32_t seq, bool no_tcp_csum) { - struct tcp_payload_t *payload = iov[TCP_IOV_PAYLOAD].iov_base; + struct tcphdr *th = iov[TCP_IOV_PAYLOAD].iov_base; struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base; + const struct iovec *tail = &iov[TCP_IOV_PAYLOAD]; const struct flowside *tapside = TAPFLOW(conn); const struct in_addr *a4 = inany_v4(&tapside->oaddr); @@ -280,12 +281,12 @@ static void tcp_buf_make_frame(const struct tcp_tap_conn *conn, if (a4) { struct iphdr *iph = iov[TCP_IOV_IP].iov_base; - tcp_fill_headers4(conn, taph, iph, payload, dlen, + tcp_fill_headers4(conn, taph, iph, th, tail, 1, sizeof(*th), check, seq, no_tcp_csum); } else { struct ipv6hdr *ip6h = iov[TCP_IOV_IP].iov_base; - tcp_fill_headers6(conn, taph, ip6h, payload, dlen, + tcp_fill_headers6(conn, taph, ip6h, th, tail, 1, sizeof(*th), seq, no_tcp_csum); } } diff --git a/tcp_internal.h b/tcp_internal.h index 6cf5ad6..8bdfc77 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -179,11 +179,13 @@ struct tcp_info_linux; void tcp_fill_headers4(const struct tcp_tap_conn *conn, struct tap_hdr *taph, struct iphdr *iph, - struct tcp_payload_t *bp, size_t dlen, + struct tcphdr *th, + const struct iovec *iov, size_t iov_cnt, size_t doffset, const uint16_t *check, uint32_t seq, bool no_tcp_csum); void tcp_fill_headers6(const struct tcp_tap_conn *conn, struct tap_hdr *taph, struct ipv6hdr *ip6h, - struct tcp_payload_t *bp, size_t dlen, + struct tcphdr *th, + const struct iovec *iov, size_t iov_cnt, size_t doffset, uint32_t seq, bool no_tcp_csum); int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, -- 2.47.0

tcp_update_csum() is now simple enough that it makes sense to just fold it into tcp_fill_header(), meaning the latter now really does fill all the header fields. Signed-off-by: David Gibson --- tcp.c | 66 ++++++++++++++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/tcp.c b/tcp.c index e9f62a4..88a285b 100644 --- a/tcp.c +++ b/tcp.c @@ -750,23 +750,6 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s) trace("TCP: failed to set SO_SNDBUF to %i", v); } -/** - * tcp_update_csum() - Calculate TCP checksum - * @psum: Unfolded partial checksum of the IPv4 or IPv6 pseudo-header - * @th: TCP header (updated) - * @iov: IO vector containing the TCP payload - * @iov_cnt: Length of @iov - * @doffset: TCP payload offset in @iov - */ -static void tcp_update_csum(uint32_t psum, struct tcphdr *th, - const struct iovec *iov, int iov_cnt, - size_t doffset) -{ - th->check = 0; - psum = csum_unfolded(th, sizeof(*th), psum); - th->check = csum_iov(iov, iov_cnt, doffset, psum); -} - /** * tcp_opt_get() - Get option, and value if any, from TCP header * @opts: Pointer to start of TCP options in header @@ -860,9 +843,16 @@ void tcp_defer_handler(struct ctx *c) * @th: Pointer to the TCP header structure * @conn: Pointer to the TCP connection structure * @seq: Sequence number + * @ppsum: Pointer to pseudo-header checksum, or NULL to omit checksum + * @iov: IO vector containing the TCP payload + * @iov_cnt: Length of @iov + * @doffset: TCP payload offset in @iov */ static void tcp_fill_header(struct tcphdr *th, - const struct tcp_tap_conn *conn, uint32_t seq) + const struct tcp_tap_conn *conn, uint32_t seq, + const uint32_t *ppsum, + const struct iovec *iov, size_t iov_cnt, + size_t doffset) { const struct flowside *tapside = TAPFLOW(conn); @@ -877,6 +867,11 @@ static void tcp_fill_header(struct tcphdr *th, th->window = htons(MIN(wnd, USHRT_MAX)); } + th->check = 0; + if (ppsum) { + uint32_t sum = csum_unfolded(th, sizeof(*th), *ppsum); + th->check = csum_iov(iov, iov_cnt, doffset, sum); + } } /** @@ -904,6 +899,8 @@ void tcp_fill_headers4(const struct tcp_tap_conn *conn, size_t dlen = iov_size(iov, iov_cnt) - doffset; size_t l4len = dlen + sizeof(*th); size_t l3len = l4len + sizeof(*iph); + const uint32_t *ppsum = NULL; + uint32_t psum; ASSERT(src4 && dst4); @@ -914,17 +911,13 @@ void tcp_fill_headers4(const struct tcp_tap_conn *conn, iph->check = check ? *check : csum_ip4_header(l3len, IPPROTO_TCP, *src4, *dst4); - tcp_fill_header(th, conn, seq); - - if (no_tcp_csum) { - th->check = 0; - } else { - uint32_t psum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, - *src4, *dst4); - - tcp_update_csum(psum, th, iov, iov_cnt, doffset); + if (!no_tcp_csum) { + psum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, *src4, *dst4); + ppsum = &psum; } + tcp_fill_header(th, conn, seq, ppsum, iov, iov_cnt, doffset); + tap_hdr_update(taph, l3len + sizeof(struct ethhdr)); } @@ -950,6 +943,8 @@ void tcp_fill_headers6(const struct tcp_tap_conn *conn, const struct flowside *tapside = TAPFLOW(conn); size_t dlen = iov_size(iov, iov_cnt) - doffset; size_t l4len = dlen + sizeof(*th); + const uint32_t *ppsum = NULL; + uint32_t psum; ip6h->payload_len = htons(l4len); ip6h->saddr = tapside->oaddr.a6; @@ -963,18 +958,15 @@ void tcp_fill_headers6(const struct tcp_tap_conn *conn, ip6h->flow_lbl[1] = (conn->sock >> 8) & 0xff; ip6h->flow_lbl[2] = (conn->sock >> 0) & 0xff; - tcp_fill_header(th, conn, seq); - - if (no_tcp_csum) { - th->check = 0; - } else { - uint32_t psum = proto_ipv6_header_psum(l4len, IPPROTO_TCP, - &ip6h->saddr, - &ip6h->daddr); - - tcp_update_csum(psum, th, iov, iov_cnt, doffset); + if (!no_tcp_csum) { + psum = proto_ipv6_header_psum(l4len, IPPROTO_TCP, + &ip6h->saddr, + &ip6h->daddr); + ppsum = &psum; } + tcp_fill_header(th, conn, seq, ppsum, iov, iov_cnt, doffset); + tap_hdr_update(taph, l4len + sizeof(*ip6h) + sizeof(struct ethhdr)); } -- 2.47.0