Re: [PATCH v8 09/30] icmp: Convert to iov_tail

On Tue, Aug 05, 2025 at 05:46:07PM +0200, Laurent Vivier wrote:

Use packet_data() and extract headers using IOV_PEEK_HEADER() rather than packet_get().

Introduce iov_tail_msghdr() to convert iov_tail to msghdr

Signed-off-by: Laurent Vivier --- icmp.c | 25 ++++++++++++++----------- iov.c | 23 +++++++++++++++++++++++ iov.h | 2 ++ 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/icmp.c b/icmp.c index 95f38c1e2a3a..fdfc857b5ae8 100644 --- a/icmp.c +++ b/icmp.c @@ -241,25 +241,27 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, struct icmp_ping_flow *pingf; const struct flowside *tgt; union sockaddr_inany sa; - size_t dlen, l4len; + struct iov_tail data; + struct msghdr msh; uint16_t id, seq; union flow *flow; uint8_t proto; socklen_t sl; - void *pkt;

(void)saddr; ASSERT(pif == PIF_TAP);

+ if (!packet_data(p, 0, &data)) + return -1; + if (af == AF_INET) { + struct icmphdr ih_storage; const struct icmphdr *ih;

- if (!(pkt = packet_get(p, 0, 0, sizeof(*ih), &dlen))) + ih = IOV_PEEK_HEADER(&data, ih_storage); + if (!ih) return 1;

- ih = (struct icmphdr *)pkt; - l4len = dlen + sizeof(*ih); - if (ih->type != ICMP_ECHO) return 1;

@@ -267,14 +269,13 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, id = ntohs(ih->un.echo.id); seq = ntohs(ih->un.echo.sequence); } else if (af == AF_INET6) { + struct icmp6hdr ih_storage; const struct icmp6hdr *ih;

- if (!(pkt = packet_get(p, 0, 0, sizeof(*ih), &dlen))) + ih = IOV_PEEK_HEADER(&data, ih_storage); + if (!ih) return 1;

- ih = (struct icmp6hdr *)pkt; - l4len = dlen + sizeof(*ih); - if (ih->icmp6_type != ICMPV6_ECHO_REQUEST) return 1;

@@ -298,8 +299,10 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, ASSERT(flow_proto[pingf->f.type] == proto); pingf->ts = now->tv_sec;

+

Bogus extra blank line.

pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0); - if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) { + iov_tail_msghdr(&msh, &data, &sa, sl); + if (sendmsg(pingf->sock, &msh, MSG_NOSIGNAL) < 0) { flow_dbg_perror(pingf, "failed to relay request to socket"); } else { flow_dbg(pingf, diff --git a/iov.c b/iov.c index 97e4ea733540..9d99beb32532 100644 --- a/iov.c +++ b/iov.c @@ -158,6 +158,29 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt) return len; }

+/** + * iov_tail_msghdr - Initialize a msghdr from an IOV tail structure + * @msh: msghdr to initialize + * @tail: iov_tail to use to set msg_iov and msg_iovlen + * @msg_name: Pointer to set to msg_name + * @msg_namelen: Size of @msg_name + */ +void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail, + void *msg_name, socklen_t msg_namelen) +{ + iov_tail_prune(tail); + + ASSERT(tail->off == 0);

Oof, this is a pretty nasty non-obvious constraint on calling this function. The whole point of iov_tails is the offset, but this function won't work in that case.

+ msh->msg_name = msg_name; + msh->msg_namelen = msg_namelen; + msh->msg_iov = (struct iovec *)tail->iov; + msh->msg_iovlen = tail->cnt; + msh->msg_control = NULL; + msh->msg_controllen = 0; + msh->msg_flags = 0; +} + /** * iov_tail_prune() - Remove any unneeded buffers from an IOV tail * @tail: IO vector tail (modified) diff --git a/iov.h b/iov.h index ccdb690ef3f1..75c3b07a87e3 100644 --- a/iov.h +++ b/iov.h @@ -82,6 +82,8 @@ struct iov_tail { 1, \ (off_))

+void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail, + void *msg_name, socklen_t msg_namelen); bool iov_tail_prune(struct iov_tail *tail); size_t iov_tail_size(struct iov_tail *tail); bool iov_tail_drop(struct iov_tail *tail, size_t len);

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