On Fri, Sep 06, 2024 at 05:34:25PM -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 now need to initialize the complete ip header on the fly.
> 
> We do this here.

Comments from 1/4 relevant here too.

> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>  tcp.c     | 14 +++++++++-----
>  tcp_buf.c |  6 ++++--
>  tcp_buf.h |  2 ++
>  3 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 77c62f0..006e503 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -920,6 +920,7 @@ static size_t tcp_fill_headers4(const struct tcp_tap_conn *conn,
>  
>  	ASSERT(src4 && dst4);
>  
> +	*iph = tcp_payload_ip4;
>  	iph->tot_len = htons(l3len);
>  	iph->saddr = src4->s_addr;
>  	iph->daddr = dst4->s_addr;
> @@ -956,6 +957,7 @@ static size_t tcp_fill_headers6(const struct tcp_tap_conn *conn,
>  	const struct flowside *tapside = TAPFLOW(conn);
>  	size_t l4len = dlen + sizeof(*th);
>  
> +	*ip6h = tcp_payload_ip6;
>  	ip6h->payload_len = htons(l4len);
>  	ip6h->saddr = tapside->oaddr.a6;
>  	ip6h->daddr = tapside->eaddr.a6;
> @@ -995,16 +997,18 @@ size_t tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
>  	const struct in_addr *a4 = inany_v4(&tapside->oaddr);
>  
>  	if (a4) {
> +		iov[TCP_IOV_IP].iov_len = sizeof(struct iphdr);
>  		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);
> +	} else {
> +		iov[TCP_IOV_IP].iov_len = sizeof(struct ipv6hdr);
> +		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);

Personally I marginally prefer this formatting, but one of the static
checkers is going to whinge about an else clause after a 'return' in
the if clause.

>  	}
> -
> -	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);
>  }
>  
>  /**
> diff --git a/tcp_buf.c b/tcp_buf.c
> index 1af4786..6e6549f 100644
> --- a/tcp_buf.c
> +++ b/tcp_buf.c
> @@ -84,6 +84,7 @@ static struct ethhdr		tcp4_eth_src;
>  
>  static struct tap_hdr		tcp4_payload_tap_hdr[TCP_FRAMES_MEM];
>  /* IPv4 headers */
> +struct iphdr		tcp_payload_ip4;
>  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];
> @@ -107,6 +108,7 @@ static struct ethhdr		tcp6_eth_src;
>  
>  static struct tap_hdr		tcp6_payload_tap_hdr[TCP_FRAMES_MEM];
>  /* IPv6 headers */
> +struct ipv6hdr		tcp_payload_ip6;
>  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];
> @@ -154,9 +156,9 @@ void tcp_sock4_iov_init(const struct ctx *c)
>  	int i;
>  
>  	tcp4_eth_src.h_proto = htons_constant(ETH_P_IP);
> +	tcp_payload_ip4 = iph;
>  
>  	for (i = 0; i < ARRAY_SIZE(tcp4_payload); i++) {
> -		tcp4_payload_ip[i].ip4 = iph;
>  		tcp4_payload[i].th.doff = sizeof(struct tcphdr) / 4;
>  		tcp4_payload[i].th.ack = 1;
>  	}
> @@ -200,9 +202,9 @@ void tcp_sock6_iov_init(const struct ctx *c)
>  	int i;
>  
>  	tcp6_eth_src.h_proto = htons_constant(ETH_P_IPV6);
> +	tcp_payload_ip6 = ip6;
>  
>  	for (i = 0; i < ARRAY_SIZE(tcp6_payload); i++) {
> -		tcp6_payload_ip[i].ip6 = ip6;
>  		tcp6_payload[i].th.doff = sizeof(struct tcphdr) / 4;
>  		tcp6_payload[i].th.ack = 1;
>  	}
> diff --git a/tcp_buf.h b/tcp_buf.h
> index 3db4c56..d3d0d7f 100644
> --- a/tcp_buf.h
> +++ b/tcp_buf.h
> @@ -13,4 +13,6 @@ void tcp_payload_flush(struct ctx *c);
>  int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn);
>  int tcp_buf_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags);
>  
> +extern struct iphdr		tcp_payload_ip4;
> +extern struct ipv6hdr		tcp_payload_ip6;
>  #endif  /*TCP_BUF_H */

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