public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top, Laurent Vivier <lvivier@redhat.com>
Subject: Re: [PATCH 6/7] iov: Helper macro to construct iovs covering existing variables or fields
Date: Tue, 30 Apr 2024 20:47:46 +0200	[thread overview]
Message-ID: <20240430204746.6441f86b@elisabeth> (raw)
In-Reply-To: <20240429070933.1366881-7-david@gibson.dropbear.id.au>

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

> Laurent's recent changes mean we use IO vectors much more heavily in the
> TCP code.  In many of those cases, and few others around the code base,
> individual iovs of these vectors are constructed to exactly cover existing
> variables or fields.  We can make initializing such iovs shorter and
> clearer with a macro for the purpose.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  iov.h |  3 +++
>  tap.c |  3 +--
>  tcp.c | 24 +++++++++---------------
>  udp.c |  7 +++----
>  4 files changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/iov.h b/iov.h
> index 6058af77..5668ca5f 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -18,6 +18,9 @@
>  #include <unistd.h>
>  #include <string.h>
>  
> +#define IOV_OF_LVALUE(lval) \
> +	(struct iovec){ .iov_base = &(lval), .iov_len = sizeof(lval) }

I'm not sure if IOV_OF_LVALUE() is much clearer than IOV_FROM() or
IOV_OF(). No strong preference though.

> +
>  size_t iov_skip_bytes(const struct iovec *iov, size_t n,
>  		      size_t skip, size_t *offset);
>  size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
> diff --git a/tap.c b/tap.c
> index 6a08cca6..5db7b169 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -79,8 +79,7 @@ void tap_send_single(const struct ctx *c, const void *data, size_t len)
>  	size_t iovcnt = 0;
>  
>  	if (c->mode == MODE_PASST) {
> -		iov[iovcnt].iov_base = &vnet_len;
> -		iov[iovcnt].iov_len = sizeof(vnet_len);
> +		iov[iovcnt] = IOV_OF_LVALUE(vnet_len);
>  		iovcnt++;
>  	}
>  
> diff --git a/tcp.c b/tcp.c
> index ad409fc5..27c06958 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -290,6 +290,7 @@
>  
>  #include "checksum.h"
>  #include "util.h"
> +#include "iov.h"
>  #include "ip.h"
>  #include "passt.h"
>  #include "tap.h"
> @@ -954,10 +955,8 @@ static void tcp_sock4_iov_init(const struct ctx *c)
>  		iov = tcp4_l2_iov[i];
>  
>  		iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp4_payload_tap_hdr[i]);
> -		iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src;
> -		iov[TCP_IOV_ETH].iov_len = sizeof(tcp4_eth_src);
> -		iov[TCP_IOV_IP].iov_base = &tcp4_payload_ip[i];
> -		iov[TCP_IOV_IP].iov_len = sizeof(tcp4_payload_ip[i]);
> +		iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp4_eth_src);
> +		iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_payload_ip[i]);
>  		iov[TCP_IOV_PAYLOAD].iov_base = &tcp4_payload[i];
>  	}
>  
> @@ -966,9 +965,8 @@ static void tcp_sock4_iov_init(const struct ctx *c)
>  
>  		iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp4_flags_tap_hdr[i]);
>  		iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src;
> -		iov[TCP_IOV_ETH].iov_len = sizeof(tcp4_eth_src);
> -		iov[TCP_IOV_IP].iov_base = &tcp4_flags_ip[i];
> -		iov[TCP_IOV_IP].iov_len = sizeof(tcp4_flags_ip[i]);
> +		iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp4_eth_src);
> +		iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_flags_ip[i]);
>  		iov[TCP_IOV_PAYLOAD].iov_base = &tcp4_flags[i];
>  	}
>  }
> @@ -1001,10 +999,8 @@ static void tcp_sock6_iov_init(const struct ctx *c)
>  		iov = tcp6_l2_iov[i];
>  
>  		iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp6_payload_tap_hdr[i]);
> -		iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src;
> -		iov[TCP_IOV_ETH].iov_len = sizeof(tcp6_eth_src);
> -		iov[TCP_IOV_IP].iov_base = &tcp6_payload_ip[i];
> -		iov[TCP_IOV_IP].iov_len = sizeof(tcp6_payload_ip[i]);
> +		iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp6_eth_src);
> +		iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[i]);
>  		iov[TCP_IOV_PAYLOAD].iov_base = &tcp6_payload[i];
>  	}
>  
> @@ -1012,10 +1008,8 @@ static void tcp_sock6_iov_init(const struct ctx *c)
>  		iov = tcp6_l2_flags_iov[i];
>  
>  		iov[TCP_IOV_TAP] = tap_hdr_iov(c, &tcp6_flags_tap_hdr[i]);
> -		iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src;
> -		iov[TCP_IOV_ETH].iov_len = sizeof(tcp6_eth_src);
> -		iov[TCP_IOV_IP].iov_base = &tcp6_flags_ip[i];
> -		iov[TCP_IOV_IP].iov_len = sizeof(tcp6_flags_ip[i]);
> +		iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp6_eth_src);
> +		iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_flags_ip[i]);
>  		iov[TCP_IOV_PAYLOAD].iov_base = &tcp6_flags[i];
>  	}
>  }
> diff --git a/udp.c b/udp.c
> index 1a4c02f4..545212c5 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -113,6 +113,7 @@
>  
>  #include "checksum.h"
>  #include "util.h"
> +#include "iov.h"
>  #include "ip.h"
>  #include "siphash.h"
>  #include "inany.h"
> @@ -315,8 +316,7 @@ static void udp_sock4_iov_init_one(const struct ctx *c, size_t i)
>  		.iph = L2_BUF_IP4_INIT(IPPROTO_UDP)
>  	};
>  
> -	siov->iov_base	= buf->data;
> -	siov->iov_len	= sizeof(buf->data);
> +	*siov		= IOV_OF_LVALUE(buf->data);
>  
>  	mh->msg_name	= &buf->s_in;
>  	mh->msg_namelen	= sizeof(buf->s_in);
> @@ -343,8 +343,7 @@ static void udp_sock6_iov_init_one(const struct ctx *c, size_t i)
>  		.ip6h = L2_BUF_IP6_INIT(IPPROTO_UDP)
>  	};
>  
> -	siov->iov_base	= buf->data;
> -	siov->iov_len	= sizeof(buf->data);
> +	*siov		 = IOV_OF_LVALUE(buf->data);

Extra whitespace between tabs and =.

-- 
Stefano


  reply	other threads:[~2024-04-30 18:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-29  7:09 [PATCH 0/7] Small improvements to IOV handling David Gibson
2024-04-29  7:09 ` [PATCH 1/7] checksum: Use proto_ipv6_header_psum() for ICMPv6 as well David Gibson
2024-04-29  7:09 ` [PATCH 2/7] tap: Split tap specific and L2 (ethernet) headers David Gibson
2024-04-30 18:46   ` Stefano Brivio
2024-04-30 23:53     ` David Gibson
2024-04-29  7:09 ` [PATCH 3/7] treewide: Standardise variable names for various packet lengths David Gibson
2024-04-30 18:46   ` Stefano Brivio
2024-05-01  0:05     ` David Gibson
2024-04-29  7:09 ` [PATCH 4/7] tcp: Simplify packet length calculation when preparing headers David Gibson
2024-04-29  7:09 ` [PATCH 5/7] tap, tcp: (Re-)abstract TAP specific header handling David Gibson
2024-04-30 18:47   ` Stefano Brivio
2024-05-01  0:06     ` David Gibson
2024-04-29  7:09 ` [PATCH 6/7] iov: Helper macro to construct iovs covering existing variables or fields David Gibson
2024-04-30 18:47   ` Stefano Brivio [this message]
2024-05-01  0:09     ` David Gibson
2024-04-29  7:09 ` [PATCH 7/7] tcp: Update tap specific header too in tcp_fill_headers[46]() David Gibson
2024-04-30 18:48   ` Stefano Brivio
2024-05-01  0:10     ` David Gibson

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=20240430204746.6441f86b@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    /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).