Re: [PATCH 3/7] treewide: Standardise variable names for various packet lengths

[Stefano Brivio <sbrivio@redhat.com>]

On Mon, 29 Apr 2024 17:09:29 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> At various points we need to track the lengths of a packet including or
> excluding various different sets of headers.  We don't always use the same
> variable names for doing so.  Worse in some places we use the same name
> for different things: e.g. tcp_fill_headers[46]() use ip_len for the
> length including the IP headers, but then tcp_send_flag() which calls it
> uses it to mean the IP payload length only.
> 
> To improve clarity, standardise on these names:
>    plen:		L4 protocol payload length
>    l4len:		plen + length of L4 protocol header
>    l3len:		l4len + length of IPv4/IPv6 header

If we're dealing with IPv4, I guess tot_len is still a reasonable
synonym for this, as you left for csum_ip4_header().

>    l2len:		l3len + length of L2 (ethernet) header
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  checksum.c | 42 +++++++++++++++++-----------------
>  checksum.h | 10 ++++-----
>  icmp.c     |  8 +++----
>  passt.h    |  4 ++--
>  tap.c      | 48 +++++++++++++++++++--------------------
>  tcp.c      | 66 +++++++++++++++++++++++++++---------------------------
>  udp.c      | 24 ++++++++++----------
>  7 files changed, 101 insertions(+), 101 deletions(-)
> 
> diff --git a/checksum.c b/checksum.c
> index 9cbe0b29..3602215a 100644
> --- a/checksum.c
> +++ b/checksum.c
> @@ -116,7 +116,7 @@ uint16_t csum_fold(uint32_t sum)
>  
>  /**
>   * csum_ip4_header() - Calculate IPv4 header checksum
> - * @tot_len:	IPv4 payload length (data + IP header, network order)
> + * @tot_len:	IPv4 packet length (data + IP header, network order)
>   * @protocol:	Protocol number (network order)
>   * @saddr:	IPv4 source address (network order)
>   * @daddr:	IPv4 destination address (network order)
> @@ -140,13 +140,13 @@ uint16_t csum_ip4_header(uint16_t tot_len, uint8_t protocol,
>  /**
>   * proto_ipv4_header_psum() - Calculates the partial checksum of an
>   * 			      IPv4 header for UDP or TCP
> - * @tot_len:	IPv4 Payload length (host order)
> + * @l4len:	IPv4 Payload length (host order)

Ouch, how did this end up being called tot_len...

>   * @proto:	Protocol number (host order)
>   * @saddr:	Source address  (network order)
>   * @daddr:	Destination address (network order)
>   * Returns:	Partial checksum of the IPv4 header
>   */
> -uint32_t proto_ipv4_header_psum(uint16_t tot_len, uint8_t protocol,
> +uint32_t proto_ipv4_header_psum(uint16_t l4len, uint8_t protocol,
>  				struct in_addr saddr, struct in_addr daddr)
>  {
>  	uint32_t psum = htons(protocol);
> @@ -155,7 +155,7 @@ uint32_t proto_ipv4_header_psum(uint16_t tot_len, uint8_t protocol,
>  	psum += saddr.s_addr & 0xffff;
>  	psum += (daddr.s_addr >> 16) & 0xffff;
>  	psum += daddr.s_addr & 0xffff;
> -	psum += htons(tot_len);
> +	psum += htons(l4len);
>  
>  	return psum;
>  }
> @@ -165,22 +165,22 @@ uint32_t proto_ipv4_header_psum(uint16_t tot_len, uint8_t protocol,
>   * @udp4hr:	UDP header, initialised apart from checksum
>   * @saddr:	IPv4 source address
>   * @daddr:	IPv4 destination address
> - * @payload:	ICMPv4 packet payload
> - * @len:	Length of @payload (not including UDP)
> + * @payload:	UDP packet payload
> + * @plen:	Length of @payload (not including UDP header)
>   */
>  void csum_udp4(struct udphdr *udp4hr,
>  	       struct in_addr saddr, struct in_addr daddr,
> -	       const void *payload, size_t len)
> +	       const void *payload, size_t plen)
>  {
>  	/* UDP checksums are optional, so don't bother */
>  	udp4hr->check = 0;
>  
>  	if (UDP4_REAL_CHECKSUMS) {
> -		uint16_t tot_len = len + sizeof(struct udphdr);
> -		uint32_t psum = proto_ipv4_header_psum(tot_len, IPPROTO_UDP,
> +		uint16_t l4len = plen + sizeof(struct udphdr);
> +		uint32_t psum = proto_ipv4_header_psum(l4len, IPPROTO_UDP,
>  						       saddr, daddr);
>  		psum = csum_unfolded(udp4hr, sizeof(struct udphdr), psum);
> -		udp4hr->check = csum(payload, len, psum);
> +		udp4hr->check = csum(payload, plen, psum);
>  	}
>  }
>  
> @@ -188,9 +188,9 @@ void csum_udp4(struct udphdr *udp4hr,
>   * csum_icmp4() - Calculate and set checksum for an ICMP packet
>   * @icmp4hr:	ICMP header, initialised apart from checksum
>   * @payload:	ICMP packet payload
> - * @len:	Length of @payload (not including ICMP header)
> + * @plen:	Length of @payload (not including ICMP header)
>   */
> -void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len)
> +void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t plen)
>  {
>  	uint32_t psum;
>  
> @@ -199,7 +199,7 @@ void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len)
>  	/* Partial checksum for ICMP header alone */
>  	psum = sum_16b(icmp4hr, sizeof(*icmp4hr));
>  
> -	icmp4hr->checksum = csum(payload, len, psum);
> +	icmp4hr->checksum = csum(payload, plen, psum);
>  }
>  
>  /**
> @@ -227,18 +227,18 @@ uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol,
>   * csum_udp6() - Calculate and set checksum for a UDP over IPv6 packet
>   * @udp6hr:	UDP header, initialised apart from checksum
>   * @payload:	UDP packet payload
> - * @len:	Length of @payload (not including UDP header)
> + * @plen:	Length of @payload (not including UDP header)
>   */
>  void csum_udp6(struct udphdr *udp6hr,
>  	       const struct in6_addr *saddr, const struct in6_addr *daddr,
> -	       const void *payload, size_t len)
> +	       const void *payload, size_t plen)
>  {
> -	uint32_t psum = proto_ipv6_header_psum(len + sizeof(struct udphdr),
> +	uint32_t psum = proto_ipv6_header_psum(plen + sizeof(struct udphdr),
>  					       IPPROTO_UDP, saddr, daddr);
>  	udp6hr->check = 0;
>  
>  	psum = csum_unfolded(udp6hr, sizeof(struct udphdr), psum);
> -	udp6hr->check = csum(payload, len, psum);
> +	udp6hr->check = csum(payload, plen, psum);
>  }
>  
>  /**
> @@ -247,19 +247,19 @@ void csum_udp6(struct udphdr *udp6hr,
>   * @saddr:	IPv6 source address
>   * @daddr:	IPv6 destination address
>   * @payload:	ICMP packet payload
> - * @len:	Length of @payload (not including ICMPv6 header)
> + * @plen:	Length of @payload (not including ICMPv6 header)
>   */
>  void csum_icmp6(struct icmp6hdr *icmp6hr,
>  		const struct in6_addr *saddr, const struct in6_addr *daddr,
> -		const void *payload, size_t len)
> +		const void *payload, size_t plen)
>  {
> -	uint32_t psum = proto_ipv6_header_psum(len + sizeof(*icmp6hr),
> +	uint32_t psum = proto_ipv6_header_psum(plen + sizeof(*icmp6hr),
>  					       IPPROTO_ICMPV6, saddr, daddr);
>  
>  	icmp6hr->icmp6_cksum = 0;
>  	/* Add in partial checksum for the ICMPv6 header alone */
>  	psum += sum_16b(icmp6hr, sizeof(*icmp6hr));
> -	icmp6hr->icmp6_cksum = csum(payload, len, psum);
> +	icmp6hr->icmp6_cksum = csum(payload, plen, psum);
>  }
>  
>  #ifdef __AVX2__
> diff --git a/checksum.h b/checksum.h
> index 0f396767..726ed7b5 100644
> --- a/checksum.h
> +++ b/checksum.h
> @@ -15,21 +15,21 @@ uint16_t csum_fold(uint32_t sum);
>  uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init);
>  uint16_t csum_ip4_header(uint16_t tot_len, uint8_t protocol,
>  			 struct in_addr saddr, struct in_addr daddr);
> -uint32_t proto_ipv4_header_psum(uint16_t tot_len, uint8_t protocol,
> +uint32_t proto_ipv4_header_psum(uint16_t l4len, uint8_t protocol,
>  				struct in_addr saddr, struct in_addr daddr);
>  void csum_udp4(struct udphdr *udp4hr,
>  	       struct in_addr saddr, struct in_addr daddr,
> -	       const void *payload, size_t len);
> -void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len);
> +	       const void *payload, size_t plen);
> +void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t plen);
>  uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t protocol,
>  				const struct in6_addr *saddr,
>  				const struct in6_addr *daddr);
>  void csum_udp6(struct udphdr *udp6hr,
>  	       const struct in6_addr *saddr, const struct in6_addr *daddr,
> -	       const void *payload, size_t len);
> +	       const void *payload, size_t plen);
>  void csum_icmp6(struct icmp6hdr *icmp6hr,
>  		const struct in6_addr *saddr, const struct in6_addr *daddr,
> -		const void *payload, size_t len);
> +		const void *payload, size_t plen);
>  uint32_t csum_unfolded(const void *buf, size_t len, uint32_t init);
>  uint16_t csum(const void *buf, size_t len, uint32_t init);
>  uint16_t csum_iov(const struct iovec *iov, size_t n, uint32_t init);
> diff --git a/icmp.c b/icmp.c
> index 76bb9e9f..263cd9ca 100644
> --- a/icmp.c
> +++ b/icmp.c
> @@ -224,8 +224,8 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>  	union sockaddr_inany sa = { .sa_family = af };
>  	const socklen_t sl = af == AF_INET ? sizeof(sa.sa4) : sizeof(sa.sa6);
>  	struct icmp_ping_flow *pingf, **id_sock;
> +	size_t plen, l4len;
>  	uint16_t id, seq;
> -	size_t plen;
>  	void *pkt;
>  
>  	(void)saddr;
> @@ -238,7 +238,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>  			return 1;
>  
>  		ih =  (struct icmphdr *)pkt;
> -		plen += sizeof(*ih);
> +		l4len = plen + sizeof(*ih);
>  
>  		if (ih->type != ICMP_ECHO)
>  			return 1;
> @@ -254,7 +254,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>  			return 1;
>  
>  		ih = (struct icmp6hdr *)pkt;
> -		plen += sizeof(*ih);
> +		l4len = plen + sizeof(*ih);
>  
>  		if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
>  			return 1;
> @@ -274,7 +274,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>  
>  	pingf->ts = now->tv_sec;
>  
> -	if (sendto(pingf->sock, pkt, plen, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
> +	if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
>  		flow_dbg(pingf, "failed to relay request to socket: %s",
>  			 strerror(errno));
>  	} else {
> diff --git a/passt.h b/passt.h
> index 76026f09..d1add470 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -22,11 +22,11 @@ struct tap_msg {
>  /**
>   * struct tap_l4_msg - Layer-4 message descriptor for protocol handlers
>   * @pkt_buf_offset:	Offset of message from @pkt_buf
> - * @l4_len:		Length of Layer-4 payload, host order
> + * @l4len:		Length of Layer-4 payload, host order

This is (and was) ambiguous, but in any case, we don't use the struct
anymore since commit bb708111833e ("treewide: Packet abstraction with
mandatory boundary checks"), so I'm fine either way, we don't necessarily
need to drop it here, I think.

>   */
>  struct tap_l4_msg {
>  	uint32_t pkt_buf_offset;
> -	uint16_t l4_len;
> +	uint16_t l4len;
>  };
>  
>  union epoll_ref;
> diff --git a/tap.c b/tap.c
> index 13e4da79..6a08cca6 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -177,15 +177,15 @@ void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport,
>  		   struct in_addr dst, in_port_t dport,
>  		   const void *in, size_t len)
>  {
> -	size_t udplen = len + sizeof(struct udphdr);
> +	size_t l4len = len + sizeof(struct udphdr);
>  	char buf[USHRT_MAX];
>  	struct iphdr *ip4h = tap_push_l2h(c, buf, ETH_P_IP);
> -	struct udphdr *uh = tap_push_ip4h(ip4h, src, dst, udplen, IPPROTO_UDP);
> +	struct udphdr *uh = tap_push_ip4h(ip4h, src, dst, l4len, IPPROTO_UDP);
>  	char *data = (char *)(uh + 1);
>  
>  	uh->source = htons(sport);
>  	uh->dest = htons(dport);
> -	uh->len = htons(udplen);
> +	uh->len = htons(l4len);
>  	csum_udp4(uh, src, dst, in, len);
>  	memcpy(data, in, len);
>  
> @@ -259,16 +259,16 @@ void tap_udp6_send(const struct ctx *c,
>  		   const struct in6_addr *dst, in_port_t dport,
>  		   uint32_t flow, const void *in, size_t len)
>  {
> -	size_t udplen = len + sizeof(struct udphdr);
> +	size_t l4len = len + sizeof(struct udphdr);
>  	char buf[USHRT_MAX];
>  	struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6);
>  	struct udphdr *uh = tap_push_ip6h(ip6h, src, dst,
> -					  udplen, IPPROTO_UDP, flow);
> +					  l4len, IPPROTO_UDP, flow);
>  	char *data = (char *)(uh + 1);
>  
>  	uh->source = htons(sport);
>  	uh->dest = htons(dport);
> -	uh->len = htons(udplen);
> +	uh->len = htons(l4len);
>  	csum_udp6(uh, src, dst, in, len);
>  	memcpy(data, in, len);
>  
> @@ -589,21 +589,21 @@ static int tap4_handler(struct ctx *c, const struct pool *in,
>  	i = 0;
>  resume:
>  	for (seq_count = 0, seq = NULL; i < in->count; i++) {
> -		size_t l2_len, l3_len, hlen, l4_len;
> +		size_t l2len, l3len, hlen, l4len;
>  		const struct ethhdr *eh;
>  		const struct udphdr *uh;
>  		struct iphdr *iph;
>  		const char *l4h;
>  
> -		packet_get(in, i, 0, 0, &l2_len);
> +		packet_get(in, i, 0, 0, &l2len);
>  
> -		eh = packet_get(in, i, 0, sizeof(*eh), &l3_len);
> +		eh = packet_get(in, i, 0, sizeof(*eh), &l3len);
>  		if (!eh)
>  			continue;
>  		if (ntohs(eh->h_proto) == ETH_P_ARP) {
>  			PACKET_POOL_P(pkt, 1, in->buf, sizeof(pkt_buf));
>  
> -			packet_add(pkt, l2_len, (char *)eh);
> +			packet_add(pkt, l2len, (char *)eh);
>  			arp(c, pkt);
>  			continue;
>  		}
> @@ -613,15 +613,15 @@ resume:
>  			continue;
>  
>  		hlen = iph->ihl * 4UL;
> -		if (hlen < sizeof(*iph) || htons(iph->tot_len) > l3_len ||
> -		    hlen > l3_len)
> +		if (hlen < sizeof(*iph) || htons(iph->tot_len) > l3len ||
> +		    hlen > l3len)
>  			continue;
>  
>  		/* We don't handle IP fragments, drop them */
>  		if (tap4_is_fragment(iph, now))
>  			continue;
>  
> -		l4_len = htons(iph->tot_len) - hlen;
> +		l4len = htons(iph->tot_len) - hlen;
>  
>  		if (IN4_IS_ADDR_LOOPBACK(&iph->saddr) ||
>  		    IN4_IS_ADDR_LOOPBACK(&iph->daddr)) {
> @@ -636,7 +636,7 @@ resume:
>  		if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
>  			c->ip4.addr_seen.s_addr = iph->saddr;
>  
> -		l4h = packet_get(in, i, sizeof(*eh) + hlen, l4_len, NULL);
> +		l4h = packet_get(in, i, sizeof(*eh) + hlen, l4len, NULL);
>  		if (!l4h)
>  			continue;
>  
> @@ -648,7 +648,7 @@ resume:
>  
>  			tap_packet_debug(iph, NULL, NULL, 0, NULL, 1);
>  
> -			packet_add(pkt, l4_len, l4h);
> +			packet_add(pkt, l4len, l4h);
>  			icmp_tap_handler(c, PIF_TAP, AF_INET,
>  					 &iph->saddr, &iph->daddr,
>  					 pkt, now);
> @@ -662,7 +662,7 @@ resume:
>  		if (iph->protocol == IPPROTO_UDP) {
>  			PACKET_POOL_P(pkt, 1, in->buf, sizeof(pkt_buf));
>  
> -			packet_add(pkt, l2_len, (char *)eh);
> +			packet_add(pkt, l2len, (char *)eh);
>  			if (dhcp(c, pkt))
>  				continue;
>  		}
> @@ -711,7 +711,7 @@ resume:
>  #undef L4_SET
>  
>  append:
> -		packet_add((struct pool *)&seq->p, l4_len, l4h);
> +		packet_add((struct pool *)&seq->p, l4len, l4h);
>  	}
>  
>  	for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) {
> @@ -763,7 +763,7 @@ static int tap6_handler(struct ctx *c, const struct pool *in,
>  	i = 0;
>  resume:
>  	for (seq_count = 0, seq = NULL; i < in->count; i++) {
> -		size_t l4_len, plen, check;
> +		size_t l4len, plen, check;
>  		struct in6_addr *saddr, *daddr;
>  		const struct ethhdr *eh;
>  		const struct udphdr *uh;
> @@ -786,7 +786,7 @@ resume:
>  		if (plen != check)
>  			continue;
>  
> -		if (!(l4h = ipv6_l4hdr(in, i, sizeof(*eh), &proto, &l4_len)))
> +		if (!(l4h = ipv6_l4hdr(in, i, sizeof(*eh), &proto, &l4len)))
>  			continue;
>  
>  		if (IN6_IS_ADDR_LOOPBACK(saddr) || IN6_IS_ADDR_LOOPBACK(daddr)) {
> @@ -814,7 +814,7 @@ resume:
>  			if (c->no_icmp)
>  				continue;
>  
> -			if (l4_len < sizeof(struct icmp6hdr))
> +			if (l4len < sizeof(struct icmp6hdr))
>  				continue;
>  
>  			if (ndp(c, (struct icmp6hdr *)l4h, saddr))
> @@ -822,20 +822,20 @@ resume:
>  
>  			tap_packet_debug(NULL, ip6h, NULL, proto, NULL, 1);
>  
> -			packet_add(pkt, l4_len, l4h);
> +			packet_add(pkt, l4len, l4h);
>  			icmp_tap_handler(c, PIF_TAP, AF_INET6,
>  					 saddr, daddr, pkt, now);
>  			continue;
>  		}
>  
> -		if (l4_len < sizeof(*uh))
> +		if (l4len < sizeof(*uh))
>  			continue;
>  		uh = (struct udphdr *)l4h;
>  
>  		if (proto == IPPROTO_UDP) {
>  			PACKET_POOL_P(pkt, 1, in->buf, sizeof(pkt_buf));
>  
> -			packet_add(pkt, l4_len, l4h);
> +			packet_add(pkt, l4len, l4h);
>  
>  			if (dhcpv6(c, pkt, saddr, daddr))
>  				continue;
> @@ -886,7 +886,7 @@ resume:
>  #undef L4_SET
>  
>  append:
> -		packet_add((struct pool *)&seq->p, l4_len, l4h);
> +		packet_add((struct pool *)&seq->p, l4len, l4h);
>  	}
>  
>  	for (j = 0, seq = tap6_l4; j < seq_count; j++, seq++) {
> diff --git a/tcp.c b/tcp.c
> index 24f99cdf..23d408f2 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -891,13 +891,13 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s)
>   */
>  static void tcp_update_check_tcp4(const struct iphdr *iph, struct tcphdr *th)
>  {
> -	uint16_t tlen = ntohs(iph->tot_len) - sizeof(struct iphdr);
> +	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 };
> -	uint32_t sum = proto_ipv4_header_psum(tlen, IPPROTO_TCP, saddr, daddr);
> +	uint32_t sum = proto_ipv4_header_psum(l4len, IPPROTO_TCP, saddr, daddr);
>  
>  	th->check = 0;
> -	th->check = csum(th, tlen, sum);
> +	th->check = csum(th, l4len, sum);
>  }
>  
>  /**
> @@ -907,12 +907,12 @@ static void tcp_update_check_tcp4(const struct iphdr *iph, struct tcphdr *th)
>   */
>  static void tcp_update_check_tcp6(struct ipv6hdr *ip6h, struct tcphdr *th)
>  {
> -	uint16_t payload_len = ntohs(ip6h->payload_len);
> -	uint32_t sum = proto_ipv6_header_psum(payload_len, IPPROTO_TCP,
> +	uint16_t l4len = ntohs(ip6h->payload_len);
> +	uint32_t sum = proto_ipv6_header_psum(l4len, IPPROTO_TCP,
>  					      &ip6h->saddr, &ip6h->daddr);
>  
>  	th->check = 0;
> -	th->check = csum(th, payload_len, sum);
> +	th->check = csum(th, l4len, sum);
>  }
>  
>  /**
> @@ -1349,12 +1349,13 @@ static size_t tcp_fill_headers4(const struct ctx *c,
>  				size_t plen, const uint16_t *check,
>  				uint32_t seq)
>  {
> -	size_t ip_len = plen + sizeof(struct iphdr) + sizeof(struct tcphdr);
>  	const struct in_addr *a4 = inany_v4(&conn->faddr);
> +	size_t l4len = plen + sizeof(*th);
> +	size_t l3len = l4len + sizeof(*iph);
>  
>  	ASSERT(a4);
>  
> -	iph->tot_len = htons(ip_len);
> +	iph->tot_len = htons(l3len);
>  	iph->saddr = a4->s_addr;
>  	iph->daddr = c->ip4.addr_seen.s_addr;
>  
> @@ -1366,7 +1367,7 @@ static size_t tcp_fill_headers4(const struct ctx *c,
>  
>  	tcp_update_check_tcp4(iph, th);
>  
> -	return ip_len;
> +	return l3len;
>  }
>  
>  /**
> @@ -1386,9 +1387,10 @@ static size_t tcp_fill_headers6(const struct ctx *c,
>  				struct ipv6hdr *ip6h, struct tcphdr *th,
>  				size_t plen, uint32_t seq)
>  {
> -	size_t ip_len = plen + sizeof(struct ipv6hdr) + sizeof(struct tcphdr);
> +	size_t l4len = plen + sizeof(*th);
> +	size_t l3len = l4len + sizeof(*ip6h);
>  
> -	ip6h->payload_len = htons(plen + sizeof(struct tcphdr));
> +	ip6h->payload_len = htons(l4len);
>  	ip6h->saddr = conn->faddr.a6;
>  	if (IN6_IS_ADDR_LINKLOCAL(&ip6h->saddr))
>  		ip6h->daddr = c->ip6.addr_ll_seen;
> @@ -1407,7 +1409,7 @@ static size_t tcp_fill_headers6(const struct ctx *c,
>  
>  	tcp_update_check_tcp6(ip6h, th);
>  
> -	return ip_len;
> +	return l3len;
>  }
>  
>  /**
> @@ -1427,21 +1429,21 @@ static size_t tcp_l2_buf_fill_headers(const struct ctx *c,
>  				      const uint16_t *check, uint32_t seq)
>  {
>  	const struct in_addr *a4 = inany_v4(&conn->faddr);
> -	size_t ip_len, tlen;
> +	size_t l3len, l4len;
>  
>  	if (a4) {
> -		ip_len = tcp_fill_headers4(c, conn, iov[TCP_IOV_IP].iov_base,
> +		l3len = tcp_fill_headers4(c, conn, iov[TCP_IOV_IP].iov_base,
>  					   iov[TCP_IOV_PAYLOAD].iov_base, plen,
>  					   check, seq);
> -		tlen = ip_len - sizeof(struct iphdr);
> +		l4len = l3len - sizeof(struct iphdr);
>  	} else {
> -		ip_len = tcp_fill_headers6(c, conn, iov[TCP_IOV_IP].iov_base,
> +		l3len = tcp_fill_headers6(c, conn, iov[TCP_IOV_IP].iov_base,
>  					   iov[TCP_IOV_PAYLOAD].iov_base, plen,
>  					   seq);
> -		tlen = ip_len - sizeof(struct ipv6hdr);
> +		l4len = l3len - sizeof(struct ipv6hdr);
>  	}
>  
> -	return tlen;
> +	return l4len;
>  }
>  
>  /**
> @@ -1578,7 +1580,7 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
>  	size_t optlen = 0;
>  	struct tcphdr *th;
>  	struct iovec *iov;
> -	size_t ip_len;
> +	size_t l4len;
>  	char *data;
>  
>  	if (SEQ_GE(conn->seq_ack_to_tap, conn->seq_from_tap) &&
> @@ -1658,11 +1660,11 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
>  	th->syn = !!(flags & SYN);
>  	th->fin = !!(flags & FIN);
>  
> -	ip_len = tcp_l2_buf_fill_headers(c, conn, iov, optlen, NULL,
> -					 conn->seq_to_tap);
> -	iov[TCP_IOV_PAYLOAD].iov_len = ip_len;
> +	l4len = tcp_l2_buf_fill_headers(c, conn, iov, optlen, NULL,
> +					conn->seq_to_tap);
> +	iov[TCP_IOV_PAYLOAD].iov_len = l4len;
>  
> -	*(uint32_t *)iov[TCP_IOV_VLEN].iov_base = htonl(vnet_len + ip_len);
> +	*(uint32_t *)iov[TCP_IOV_VLEN].iov_base = htonl(vnet_len + l4len);

I was completely puzzled by this until I reached 7/7. Okay, this works
because vnet_len includes everything that's not in l4len.

There's one bit of confusion left after this series, though. This series
actually highlights that. I'm not sure if it's convenient to fix that
here as well:

		l4len = tcp_l2_buf_fill_headers(c, conn, iov, plen,
		check, seq);

This fills in Layer-2 buffers, and returns Layer-3 payload length (i.e.
payload plus size of Layer-4 headers). I didn't really think about it
when the variable was ip_len, but now it's apparent.

Further on:

                iov[TCP_IOV_PAYLOAD].iov_len = l4len;

...so l4len is the length of the payload plus Layer-4 header, but we
use that to set the buffer for TCP_IOV_PAYLOAD...

-- 
Stefano