From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202504 header.b=Ux6p06mu; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 48FD25A0272 for ; Mon, 14 Apr 2025 05:22:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202504; t=1744600912; bh=8krN1GO6WKZgo/Q7rqCT6ZrxiCsOuvMG/pK873nOM6Q=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ux6p06muhvbRlj0IIbpvGcHy2KCQMGe/j/qWGFDkXTObB2aC8SL9KMkmwnRIUttDA 2qcen3nFfPQaJnfdvs4xPExrmsyMZaN53ZxIkliUC8oWO+gLFAUyP95hsWX8iuJu+S azfo5rQN5dqUWrnm8Tw4Su6FZDfb+nY8dv3NCoWhIAkQbF5IIMedIQsg90EQa3ztqs I4t0a8FVKzzg7x7E3iZ8riC8v6sHunnp8n8qlWz/VUCJDInvhmjNOWvtoxba3lRYfP HIapjTi6D7kzOqI1amIMVASHI8Xd0RQ3jl6JR53/Co3I6xUsS6X9UQkxvdwearAMRp DmoT2EtMGh9aw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZbXdX0QzYz4x1V; Mon, 14 Apr 2025 13:21:52 +1000 (AEST) Date: Mon, 14 Apr 2025 13:14:24 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v2 09/20] icmp: Convert to iov_tail Message-ID: References: <20250411131031.1398006-1-lvivier@redhat.com> <20250411131031.1398006-10-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gh/I4f0BQGvTWtI2" Content-Disposition: inline In-Reply-To: <20250411131031.1398006-10-lvivier@redhat.com> Message-ID-Hash: 7V5JMRYAUINPU55Q4TIDVK3ZJQ3V74E6 X-Message-ID-Hash: 7V5JMRYAUINPU55Q4TIDVK3ZJQ3V74E6 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --gh/I4f0BQGvTWtI2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Apr 11, 2025 at 03:10:19PM +0200, Laurent Vivier wrote: > Use packet_base() and extract headers using IOV_PEEK_HEADER() > rather than packet_get(). >=20 > Introduce iov_tail_msghdr() to convert iov_tail to msghdr > Signed-off-by: Laurent Vivier > --- > icmp.c | 32 +++++++++++++++++--------------- > iov.c | 21 +++++++++++++++++++++ > iov.h | 2 ++ > 3 files changed, 40 insertions(+), 15 deletions(-) >=20 > diff --git a/icmp.c b/icmp.c > index 7e2b3423a8d1..64ad738b6809 100644 > --- a/icmp.c > +++ b/icmp.c > @@ -241,24 +241,25 @@ int icmp_tap_handler(const struct ctx *c, uint8_t p= if, 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; > =20 > (void)saddr; > ASSERT(pif =3D=3D PIF_TAP); > =20 > + if (!packet_base(p, 0, &data)) > + return -1; > + > if (af =3D=3D AF_INET) { > const struct icmphdr *ih; > + struct icmphdr ihc; > =20 > - if (!(pkt =3D packet_get(p, 0, 0, sizeof(*ih), &dlen))) > - return 1; > - > - ih =3D (struct icmphdr *)pkt; > - l4len =3D dlen + sizeof(*ih); > + ih =3D IOV_PEEK_HEADER(&data, ihc); > + if (!ih) > + return -1; You've replaced a 'return 1' with a 'return -1' which doesn't seem correct. > =20 > if (ih->type !=3D ICMP_ECHO) > return 1; > @@ -268,12 +269,11 @@ int icmp_tap_handler(const struct ctx *c, uint8_t p= if, sa_family_t af, > seq =3D ntohs(ih->un.echo.sequence); > } else if (af =3D=3D AF_INET6) { > const struct icmp6hdr *ih; > + struct icmp6hdr ihc; > =20 > - if (!(pkt =3D packet_get(p, 0, 0, sizeof(*ih), &dlen))) > - return 1; > - > - ih =3D (struct icmp6hdr *)pkt; > - l4len =3D dlen + sizeof(*ih); > + ih =3D IOV_PEEK_HEADER(&data, ihc); > + if (!ih) > + return -1; Same here. > =20 > if (ih->icmp6_type !=3D ICMPV6_ECHO_REQUEST) > return 1; > @@ -298,8 +298,10 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pi= f, sa_family_t af, > ASSERT(flow_proto[pingf->f.type] =3D=3D proto); > pingf->ts =3D now->tv_sec; > =20 > - 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); > + > + pif_sockaddr(c, &sa, &msh.msg_namelen, PIF_HOST, &tgt->eaddr, 0); > + 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 98d5fecbdc8f..9e3786e6efe8 100644 > --- a/iov.c > +++ b/iov.c > @@ -210,6 +210,27 @@ int iov_slice(struct iovec *dst_iov, size_t dst_iov_= cnt, > return j; > } > =20 > +/** > + * 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 > + */ > +void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail, > + void *msg_name) > +{ > + iov_tail_prune(tail); > + > + ASSERT(tail->off =3D=3D 0); > + > + msh->msg_name =3D msg_name; This doesn't initialise msg_namelen. I see that you do it outside =66rom pif_sockaddr(), but that makes it extremely easy to misuse this function. Not immediately sure how to make an interface that's both safe and convenient here. If it helps, it's probably reasonable to assume here that you'll always use a union sockaddr_inany for the address. > + msh->msg_iov =3D (struct iovec *)tail->iov; > + msh->msg_iovlen =3D tail->cnt; > + msh->msg_control =3D NULL; > + msh->msg_controllen =3D 0; > + msh->msg_flags =3D 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 b412a96b1090..acba2ea4240b 100644 > --- a/iov.h > +++ b/iov.h > @@ -83,6 +83,8 @@ struct iov_tail { > #define IOV_TAIL_FROM_BUF(buf_, len_, off_) \ > IOV_TAIL((&(const struct iovec){ .iov_base =3D (buf_), .iov_len =3D (le= n_) }), 1, (off_)) > =20 > +void iov_tail_msghdr(struct msghdr *msh, struct iov_tail *tail, > + void *msg_name); > 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); --=20 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 --gh/I4f0BQGvTWtI2 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmf8fY8ACgkQzQJF27ox 2Gfn/w//Z1zn9oJqfwWAaBtpzdcA6MPtYLwN0+7BuRtk86wLN1IJf4EeC4a9WQ6v bnzrtVJk0Xy6LmIs+rF2dYHb6eqmuyqEAPTBxo+QjP/WBsWdLi0Knf91JRreWuVu 33XKlvYl+f0aNInWlVflL/sAsDsK43AH4niIzEtDqpEK+AhB9Qkqa+YnVqF/yIJx OUhn94xOkQ1umK7Yqn4GkEYPqk0V/2t4albs3mtdLiFte1J/vEh6sOTSYSzkXKrI Gar8yenuQmiYKdzo406lmn74Fscir0DxZHTgE76wp0U8ZnlvwTSL0BhzhWpfIOnn ZlC0WRv/e0zmSSUp1fE+slcpfiNTayZNDDO7QSC49Nb6QyNK5aScz/wsayo9aQgV yrSABWhSA+HVDuGL8UoFlHEvMa84rUlqxEsiu+0tVvdk84Sn9xyQ8UVDQK+l6uzy GkG8ccib9horNaWu+V4MSKE4qVqPq0mukfR9DrLektDxJrlskqbtkFCf28O44GsO itLFcoHGjlKRnTXZdFEsBIPBQ5BxLEt34YbQ3bgPGqFpwyjlJWPgFANdTyhf9eL+ KIZ4+7YRC5DPPwQbBY403zbJP8bsSM7J8X8zoVsKjhb2ot3ujbt+ayx0+AOS5Pc7 Jdf5yFf/kI7/fKuqiWKslkwDGL8q6/7OWrRYe/h4gGSrl9EZUT0= =E/mK -----END PGP SIGNATURE----- --gh/I4f0BQGvTWtI2--