public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Jon Maloy <jmaloy@redhat.com>
Cc: passt-dev@passt.top, sbrivio@redhat.com, lvivier@redhat.com,
	dgibson@redhat.com
Subject: Re: [PATCH 2/4] tcp: update ip address in l2 tap queues on the fly
Date: Mon, 9 Sep 2024 11:08:26 +1000	[thread overview]
Message-ID: <Zt5Kiqe-y6ovHmzG@zatzit.fritz.box> (raw)
In-Reply-To: <20240906213427.1915806-3-jmaloy@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4531 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2024-09-09  1:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-06 21:34 [PATCH 0/4] tcp: unify IPv4 and IPv6 l2 tap queues Jon Maloy
2024-09-06 21:34 ` [PATCH 1/4] tcp: create unified struct for IPv4 and IPv6 header Jon Maloy
2024-09-09  1:04   ` David Gibson
2024-09-06 21:34 ` [PATCH 2/4] tcp: update ip address in l2 tap queues on the fly Jon Maloy
2024-09-09  1:08   ` David Gibson [this message]
2024-09-06 21:34 ` [PATCH 3/4] tcp: unify l2 TCPv4 and TCPv6 queues and structures Jon Maloy
2024-09-09  1:17   ` David Gibson
2024-09-06 21:34 ` [PATCH 4/4] tcp: change prefix tcp4_ to tcp_ where applicable Jon Maloy
2024-09-09  8:31 ` [PATCH 0/4] tcp: unify IPv4 and IPv6 l2 tap queues Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zt5Kiqe-y6ovHmzG@zatzit.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=dgibson@redhat.com \
    --cc=jmaloy@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).