Re: [PATCH 2/7] tcp: use iov_tail to access headers in tcp_fill_headers()

On Mon, Mar 23, 2026 at 05:52:54PM +0100, Laurent Vivier wrote:

Instead of receiving individual pointers to each protocol header (eh, ip4h, ip6h, th), have tcp_fill_headers() take an iov_tail starting at the Ethernet header and walk through it using with_header() and IOV_DROP_HEADER() to access each header in turn.

Replace the ip4h/ip6h NULL-pointer convention with a bool ipv4 parameter, and move Ethernet header filling (MAC address and ethertype) into tcp_fill_headers() as well, since the function now owns the full header chain.

This simplifies callers, which no longer need to extract and pass individual header pointers.

Signed-off-by: Laurent Vivier --- tcp.c | 106 +++++++++++++++++++++++++++---------------------- tcp_buf.c | 16 ++------ tcp_internal.h | 4 +- tcp_vu.c | 12 +++--- 4 files changed, 70 insertions(+), 68 deletions(-)

diff --git a/tcp.c b/tcp.c index 158a5be0327e..058792d5b184 100644 --- a/tcp.c +++ b/tcp.c @@ -938,11 +938,8 @@ static void tcp_fill_header(struct tcphdr *th, * tcp_fill_headers() - Fill 802.3, IP, TCP headers * @c: Execution context * @conn: Connection pointer - * @eh: Pointer to Ethernet header - * @ip4h: Pointer to IPv4 header, or NULL - * @ip6h: Pointer to IPv6 header, or NULL - * @th: Pointer to TCP header - * @payload: TCP payload + * @ipv4: True for IPv4, false for IPv6 + * @payload: IOV tail starting at the Ethernet header

As UDP I'm not sure I like removing the separater parameters. A little more so, since this doesn't (any more) have the ugly udp_payload_t equivalent. At minimum @payload is no longer a good name.

* @ip4_check: IPv4 checksum, if already known * @seq: Sequence number for this segment * @no_tcp_csum: Do not set TCP checksum @@ -950,74 +947,89 @@ static void tcp_fill_header(struct tcphdr *th, * Return: frame length (including L2 headers) */ size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn, - struct ethhdr *eh, - struct iphdr *ip4h, struct ipv6hdr *ip6h, - struct tcphdr *th, struct iov_tail *payload, + bool ipv4, const struct iov_tail *payload, int ip4_check, uint32_t seq, bool no_tcp_csum) { const struct flowside *tapside = TAPFLOW(conn); - size_t l4len = iov_tail_size(payload) + sizeof(*th); + struct iov_tail current = *payload; uint8_t *omac = conn->f.tap_omac; - size_t l3len = l4len; + size_t l3len, l4len; uint32_t psum = 0;

- if (ip4h) { + with_header(struct ethhdr, eh, ¤t) { + if (ipv4) + eh->h_proto = htons_constant(ETH_P_IP); + else + eh->h_proto = htons_constant(ETH_P_IPV6); + + /* Find if neighbour table has a recorded MAC address */ + if (MAC_IS_UNDEF(omac)) + fwd_neigh_mac_get(c, &tapside->oaddr, omac); + eth_update_mac(eh, NULL, omac); + } + IOV_DROP_HEADER(¤t, struct ethhdr); + + l3len = iov_tail_size(¤t); + + if (ipv4) { const struct in_addr *src4 = inany_v4(&tapside->oaddr); const struct in_addr *dst4 = inany_v4(&tapside->eaddr);

assert(src4 && dst4);

- l3len += + sizeof(*ip4h); + l4len = l3len - sizeof(struct iphdr);

- ip4h->tot_len = htons(l3len); - ip4h->saddr = src4->s_addr; - ip4h->daddr = dst4->s_addr; + with_header(struct iphdr, ip4h, ¤t) { + ip4h->tot_len = htons(l3len); + ip4h->saddr = src4->s_addr; + ip4h->daddr = dst4->s_addr;

- if (ip4_check != -1) - ip4h->check = ip4_check; - else - ip4h->check = csum_ip4_header(l3len, IPPROTO_TCP, - *src4, *dst4); + if (ip4_check != -1) + ip4h->check = ip4_check; + else + ip4h->check = csum_ip4_header(l3len, + IPPROTO_TCP, + *src4, *dst4); + } + IOV_DROP_HEADER(¤t, struct iphdr);

if (!no_tcp_csum) { psum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, *src4, *dst4); } - eh->h_proto = htons_constant(ETH_P_IP); - } - - if (ip6h) { - l3len += sizeof(*ip6h); + } else { + l4len = l3len - sizeof(struct ipv6hdr);

- ip6h->payload_len = htons(l4len); - ip6h->saddr = tapside->oaddr.a6; - ip6h->daddr = tapside->eaddr.a6; + with_header(struct ipv6hdr, ip6h, ¤t) { + ip6h->payload_len = htons(l4len); + ip6h->saddr = tapside->oaddr.a6; + ip6h->daddr = tapside->eaddr.a6;

- ip6h->hop_limit = 255; - ip6h->version = 6; - ip6h->nexthdr = IPPROTO_TCP; + ip6h->hop_limit = 255; + ip6h->version = 6; + ip6h->nexthdr = IPPROTO_TCP;

- ip6_set_flow_lbl(ip6h, conn->sock); + ip6_set_flow_lbl(ip6h, conn->sock);

- if (!no_tcp_csum) { - psum = proto_ipv6_header_psum(l4len, IPPROTO_TCP, - &ip6h->saddr, - &ip6h->daddr); + if (!no_tcp_csum) { + psum = proto_ipv6_header_psum(l4len, + IPPROTO_TCP, + &ip6h->saddr, + &ip6h->daddr); + } } - eh->h_proto = htons_constant(ETH_P_IPV6); + IOV_DROP_HEADER(¤t, struct ipv6hdr); }

- /* Find if neighbour table has a recorded MAC address */ - if (MAC_IS_UNDEF(omac)) - fwd_neigh_mac_get(c, &tapside->oaddr, omac); - eth_update_mac(eh, NULL, omac); - - tcp_fill_header(th, conn, seq); - - if (no_tcp_csum) + with_header(struct tcphdr, th, ¤t) { + tcp_fill_header(th, conn, seq); th->check = 0; - else - tcp_update_csum(psum, th, payload); + } + + if (!no_tcp_csum) { + with_header(struct tcphdr, th, ¤t) + th->check = csum_iov_tail(¤t, psum); + }

return MAX(l3len + sizeof(struct ethhdr), ETH_ZLEN); } diff --git a/tcp_buf.c b/tcp_buf.c index bc0f58dd7a5e..891043c96dcb 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -175,23 +175,15 @@ static void tcp_l2_buf_fill_headers(const struct ctx *c, struct iovec *iov, int check, uint32_t seq, bool no_tcp_csum) { - struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0); - struct tcphdr th_storage, *th = IOV_REMOVE_HEADER(&tail, th_storage); + struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_ETH], + TCP_IOV_PAYLOAD + 1 - TCP_IOV_ETH, 0); 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); - struct ethhdr *eh = iov[TCP_IOV_ETH].iov_base; - struct ipv6hdr *ip6h = NULL; - struct iphdr *ip4h = NULL; + bool ipv4 = inany_v4(&tapside->oaddr) != NULL; size_t l2len;

- if (a4) - ip4h = iov[TCP_IOV_IP].iov_base; - else - ip6h = iov[TCP_IOV_IP].iov_base; + l2len = tcp_fill_headers(c, conn, ipv4, &tail, check, seq, no_tcp_csum);

- l2len = tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &tail, check, seq, - no_tcp_csum); tap_hdr_update(taph, l2len); }

diff --git a/tcp_internal.h b/tcp_internal.h index bb7a6629839c..136e947f6e70 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -184,9 +184,7 @@ void tcp_rst_do(const struct ctx *c, struct tcp_tap_conn *conn); struct tcp_info_linux;

size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn, - struct ethhdr *eh, - struct iphdr *ip4h, struct ipv6hdr *ip6h, - struct tcphdr *th, struct iov_tail *payload, + bool ipv4, const struct iov_tail *payload, int ip4_check, uint32_t seq, bool no_tcp_csum);

int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, diff --git a/tcp_vu.c b/tcp_vu.c index a21ee3499aed..c6206b7a689c 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -132,13 +132,12 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) }

vu_pad(&flags_iov[0], 1, 0, hdrlen + optlen); - payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);

if (flags & KEEPALIVE) seq--;

- tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload, - -1, seq, !*c->pcap); + payload = IOV_TAIL(flags_elem[0].in_sg, 1, VNET_HLEN); + tcp_fill_headers(c, conn, CONN_V4(conn), &payload, -1, seq, !*c->pcap);

if (*c->pcap) pcap_iov(&flags_elem[0].in_sg[0], 1, VNET_HLEN); @@ -288,10 +287,10 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn, const struct flowside *toside = TAPFLOW(conn); bool v6 = !(inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr)); size_t hdrlen = tcp_vu_hdrlen(v6); - struct iov_tail payload = IOV_TAIL(iov, iov_cnt, hdrlen); char *base = iov[0].iov_base; struct ipv6hdr *ip6h = NULL; struct iphdr *ip4h = NULL; + struct iov_tail payload; struct tcphdr *th; struct ethhdr *eh;

@@ -326,8 +325,9 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn, th->ack = 1; th->psh = push;

- tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload, - *check, conn->seq_to_tap, no_tcp_csum); + payload = IOV_TAIL(iov, iov_cnt, VNET_HLEN); + tcp_fill_headers(c, conn, !v6, &payload, *check, conn->seq_to_tap, + no_tcp_csum); if (ip4h) *check = ip4h->check; } -- 2.53.0

-- 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