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
next prev parent 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).