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=202602 header.b=AfZf0F1X; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id D59705A026D for ; Fri, 10 Apr 2026 09:18:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1775805479; bh=6HiYKbCx1RAt4mXMi+xyDkF8+gOUw5rqr+K0KWRoacM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=AfZf0F1XxoG7Aod84O3wSjMuMdHZ43BwldAl7Nk0COh69QnxHpT6SNLuufRHW3xLT 7h4p2MlqWdTKgh+ZAIJr0mco/2lTJw0JAWzsDOxXzeeQM5m6rDnkLLh6BKbtfxwaBP l01GdVzppn2khOhOa5wk6GPsXXq52lcpAfEbrL0E76W7EmDin7D073Ftheo8JCnCDb vFpE5GJGpWM3U2z578uL8uvvU4Bx5hSxAcWkvrZDDJmFweDNVbfsZCtkYS+1gTSaOn TnPSUR3aMzVvmGjwVfbvGVUHIuU+Hc0j3NOrYmMGFZsz8xSYRCBRAkY3VvAim2HaBH sxjpEGGMH0f8Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fsSnM5JJFz4wLn; Fri, 10 Apr 2026 17:17:59 +1000 (AEST) Date: Fri, 10 Apr 2026 17:17:55 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v2 07/10] pcap: Pass explicit L2 length to pcap_iov() Message-ID: References: <20260403163811.3209635-1-lvivier@redhat.com> <20260403163811.3209635-8-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="6+Jus82/nJl0g9XD" Content-Disposition: inline In-Reply-To: <20260403163811.3209635-8-lvivier@redhat.com> Message-ID-Hash: DBKW4J2UD2KN3FXEHU7DPOGD2EOLS22P X-Message-ID-Hash: DBKW4J2UD2KN3FXEHU7DPOGD2EOLS22P 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: --6+Jus82/nJl0g9XD Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Apr 03, 2026 at 06:38:08PM +0200, Laurent Vivier wrote: > With vhost-user multibuffer frames, the iov can be larger than the > actual L2 frame. The previous approach of computing L2 length as > iov_size() - offset would overcount and write extra bytes into the > pcap file. >=20 > Pass the L2 frame length explicitly to pcap_frame() and pcap_iov(), > and write exactly that many bytes instead of the full iov remainder. >=20 > Signed-off-by: Laurent Vivier > --- > pcap.c | 41 +++++++++++++++++++++++++++++++---------- > pcap.h | 2 +- > tap.c | 3 ++- > tcp_vu.c | 14 ++++++++------ > udp_vu.c | 4 +++- > vu_common.c | 2 +- > 6 files changed, 46 insertions(+), 20 deletions(-) >=20 > diff --git a/pcap.c b/pcap.c > index a026f17e7974..c2a6910bec02 100644 > --- a/pcap.c > +++ b/pcap.c > @@ -52,22 +52,39 @@ struct pcap_pkthdr { > * @iov: IO vector containing frame (with L2 headers and tap headers) > * @iovcnt: Number of buffers (@iov entries) in frame > * @offset: Byte offset of the L2 headers within @iov > + * @l2len: Length of L2 frame data to capture > * @now: Timestamp > */ > static void pcap_frame(const struct iovec *iov, size_t iovcnt, > - size_t offset, const struct timespec *now) > + size_t offset, size_t l2len, const struct timespec *now) > { > - size_t l2len =3D iov_size(iov, iovcnt) - offset; > struct pcap_pkthdr h =3D { > .tv_sec =3D now->tv_sec, > .tv_usec =3D DIV_ROUND_CLOSEST(now->tv_nsec, 1000), > .caplen =3D l2len, > .len =3D l2len > }; > + size_t i; > =20 > - if (write_all_buf(pcap_fd, &h, sizeof(h)) < 0 || > - write_remainder(pcap_fd, iov, iovcnt, offset) < 0) > - debug_perror("Cannot log packet, length %zu", l2len); > + if (write_all_buf(pcap_fd, &h, sizeof(h)) < 0) { > + debug_perror("Cannot log packet, packet header error"); > + return; > + } > + Hm, would it be cleaner to push a length parameter into write_remainder(), rather than open coding the loop here? We might be able to make at least partial use of writev() in that case. > + for (i =3D iov_skip_bytes(iov, iovcnt, offset, &offset); > + i < iovcnt && l2len; i++) { > + size_t l =3D MIN(l2len, iov[i].iov_len - offset); > + int n; > + > + n =3D write_all_buf(pcap_fd, (char *)iov[i].iov_base + offset, l); > + if (n < 0) { > + debug_perror("Cannot log packet, length %zu", l); > + return; > + } > + > + offset =3D 0; > + l2len -=3D l; > + } > } > =20 > /** > @@ -87,7 +104,7 @@ void pcap(const char *pkt, size_t l2len) > if (clock_gettime(CLOCK_REALTIME, &now)) > err_perror("Failed to get CLOCK_REALTIME time"); > =20 > - pcap_frame(&iov, 1, 0, &now); > + pcap_frame(&iov, 1, 0, l2len, &now); > } > =20 > /** > @@ -109,8 +126,11 @@ void pcap_multiple(const struct iovec *iov, size_t f= rame_parts, unsigned int n, > if (clock_gettime(CLOCK_REALTIME, &now)) > err_perror("Failed to get CLOCK_REALTIME time"); > =20 > - for (i =3D 0; i < n; i++) > - pcap_frame(iov + i * frame_parts, frame_parts, offset, &now); > + for (i =3D 0; i < n; i++) { > + pcap_frame(iov + i * frame_parts, frame_parts, offset, > + iov_size(iov + i * frame_parts, frame_parts) - offset, > + &now); > + } > } > =20 > /** > @@ -120,8 +140,9 @@ void pcap_multiple(const struct iovec *iov, size_t fr= ame_parts, unsigned int n, > * containing packet data to write, including L2 header > * @iovcnt: Number of buffers (@iov entries) > * @offset: Offset of the L2 frame within the full data length > + * @l2len: Length of L2 frame data to capture > */ > -void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset) > +void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset, siz= e_t l2len) > { > struct timespec now =3D { 0 }; > =20 > @@ -131,7 +152,7 @@ void pcap_iov(const struct iovec *iov, size_t iovcnt,= size_t offset) > if (clock_gettime(CLOCK_REALTIME, &now)) > err_perror("Failed to get CLOCK_REALTIME time"); > =20 > - pcap_frame(iov, iovcnt, offset, &now); > + pcap_frame(iov, iovcnt, offset, l2len, &now); > } > =20 > /** > diff --git a/pcap.h b/pcap.h > index dface5df4ee6..c171257cbd73 100644 > --- a/pcap.h > +++ b/pcap.h > @@ -13,7 +13,7 @@ extern int pcap_fd; > void pcap(const char *pkt, size_t l2len); > void pcap_multiple(const struct iovec *iov, size_t frame_parts, unsigned= int n, > size_t offset); > -void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset); > +void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset, siz= e_t l2len); > void pcap_init(struct ctx *c); > =20 > #endif /* PCAP_H */ > diff --git a/tap.c b/tap.c > index b61199dd699d..a4cc00b93d82 100644 > --- a/tap.c > +++ b/tap.c > @@ -1102,10 +1102,11 @@ void tap_handler(struct ctx *c, const struct time= spec *now) > void tap_add_packet(struct ctx *c, struct iov_tail *data, > const struct timespec *now) > { > + size_t l2len =3D iov_tail_size(data); > struct ethhdr eh_storage; > const struct ethhdr *eh; > =20 > - pcap_iov(data->iov, data->cnt, data->off); > + pcap_iov(data->iov, data->cnt, data->off, l2len); > =20 > eh =3D IOV_PEEK_HEADER(data, eh_storage); > if (!eh) > diff --git a/tcp_vu.c b/tcp_vu.c > index 0cd01190d612..7b7ea9c789b1 100644 > --- a/tcp_vu.c > +++ b/tcp_vu.c > @@ -128,7 +128,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_= tap_conn *conn, int flags) > return ret; > } > =20 > - iov_truncate(&flags_iov[0], 1, hdrlen + optlen); > + l2len =3D hdrlen + optlen - VNET_HLEN; > + iov_truncate(&flags_iov[0], 1, l2len + VNET_HLEN); > payload =3D IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen); > =20 > if (flags & KEEPALIVE) > @@ -137,13 +138,12 @@ int tcp_vu_send_flag(const struct ctx *c, struct tc= p_tap_conn *conn, int flags) > tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload, > NULL, seq, !*c->pcap); > =20 > - l2len =3D optlen + hdrlen - VNET_HLEN; > vu_pad(&flags_elem[0].in_sg[0], l2len); > =20 > vu_flush(vdev, vq, flags_elem, 1); > =20 > if (*c->pcap) > - pcap_iov(&flags_elem[0].in_sg[0], 1, VNET_HLEN); > + pcap_iov(&flags_elem[0].in_sg[0], 1, VNET_HLEN, l2len); > =20 > if (flags & DUP_ACK) { > elem_cnt =3D vu_collect(vdev, vq, &flags_elem[1], 1, > @@ -158,8 +158,10 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp= _tap_conn *conn, int flags) > =20 > vu_flush(vdev, vq, &flags_elem[1], 1); > =20 > - if (*c->pcap) > - pcap_iov(&flags_elem[1].in_sg[0], 1, VNET_HLEN); > + if (*c->pcap) { > + pcap_iov(&flags_elem[1].in_sg[0], 1, VNET_HLEN, > + l2len); > + } > } > } > vu_queue_notify(vdev, vq); > @@ -464,7 +466,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct= tcp_tap_conn *conn) > vu_flush(vdev, vq, &elem[head[i]], buf_cnt); > =20 > if (*c->pcap) > - pcap_iov(iov, buf_cnt, VNET_HLEN); > + pcap_iov(iov, buf_cnt, VNET_HLEN, l2len); > =20 > conn->seq_to_tap +=3D dlen; > } > diff --git a/udp_vu.c b/udp_vu.c > index 5421a7d71a19..81491afa7e6a 100644 > --- a/udp_vu.c > +++ b/udp_vu.c > @@ -185,6 +185,7 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, i= nt n, flow_sidx_t tosidx) > static struct iovec iov_vu[VIRTQUEUE_MAX_SIZE]; > struct vu_dev *vdev =3D c->vdev; > struct vu_virtq *vq =3D &vdev->vq[VHOST_USER_RX_QUEUE]; > + size_t hdrlen =3D udp_vu_hdrlen(v6); > int i; > =20 > assert(!c->no_udp); > @@ -230,7 +231,8 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, i= nt n, flow_sidx_t tosidx) > size_t l4len =3D udp_vu_prepare(c, iov_vu, toside, dlen); > if (*c->pcap) { > udp_vu_csum(toside, iov_vu, iov_cnt, l4len); > - pcap_iov(iov_vu, iov_cnt, VNET_HLEN); > + pcap_iov(iov_vu, iov_cnt, VNET_HLEN, > + hdrlen + dlen - VNET_HLEN); > } > vu_flush(vdev, vq, elem, elem_used); > vu_queue_notify(vdev, vq); > diff --git a/vu_common.c b/vu_common.c > index 57949ca32309..f254cb67ec78 100644 > --- a/vu_common.c > +++ b/vu_common.c > @@ -268,7 +268,7 @@ int vu_send_single(const struct ctx *c, const void *b= uf, size_t size) > iov_from_buf(in_sg, in_total, VNET_HLEN, buf, total); > =20 > if (*c->pcap) > - pcap_iov(in_sg, in_total, VNET_HLEN); > + pcap_iov(in_sg, in_total, VNET_HLEN, size); > =20 > vu_flush(vdev, vq, elem, elem_cnt); > vu_queue_notify(vdev, vq); > --=20 > 2.53.0 >=20 --=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 --6+Jus82/nJl0g9XD Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmnYpCIACgkQzQJF27ox 2GeEww//c7oCNdr4TtmMA4lT2uT2s4dR1MUCm5hh35X42uKKB+h1mvkR2uaedMn2 XHQt3jITzIcIjc8ItzE7E5HtUwXJcwKHTZUrAYFSbJJ3bd6MVh6Va1uE3WSFi/KP HOsjNuHKl2C8OGjjbq+ZHQWh3bGP7dpNuz/NrxQJGz+12fHKQuOttzn1//FXP4xU 5fBo+m/fLxCeZBYPdz3/pvF9QDM2CE0SJvoRBTDofSm28Q7bW2BzfsID22FFK8sw 7VaoQpopa/4yUN414wnYHacdAxiAokDhmwFH4WT4GbJvkM6qbFtlLa+LLOeDspdE 5L1E6ic4xZFcFmwee3JMx/WBg5oE36RFko17Ng7bs3I0/h+8HD8q6DZA7pU22Y/I ZqkyQ1ofsWaYNJgblnntmvrE7OkV1/SqNznDJcAPt47wMGD0J2jNVIu6jT+ID2h3 mFjQn/QqSFrhvrqOb/+izK3RYqFBHB5uX57cFDlhPwEcNFIockl/KA6rkELnlepm qVWp+JVUAOHXGcIK8yjNEKtY9FpGbG8xxroIgfm/8FhfAcR7B8ZyMakJSxr67utz hJ78y5CByiLORs3SeOdVvDMyXz/Vh04pFtkzkc9p+r1C1luE7QRdSg97roLKls7N d+xwRJPHHdXZN5DR/c3Bmvc/4tATEr+It6QPvxbuSBg0C0o7Ma0= =VLfC -----END PGP SIGNATURE----- --6+Jus82/nJl0g9XD--