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=202502 header.b=aDifuXhb; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 560C55A0008 for ; Thu, 03 Apr 2025 07:38:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1743658684; bh=7eF6FvYa3sK3S/TJlqBzqROjByejUYL7PQo6pUpKI7Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=aDifuXhbgoxIsnOnoZvUQCXWnf0TigkAtx+YEV8P7gkh5IM0CBQNFB+Prw4P2Bizu +0Jnkh6ci7FZ4BMwoZInMjIgmz+8B/OHCkOyttasSquVBsoXCSSqQpqiERgSLlJX3s DsB149RgfbDGmReH55fi3ycOo/9tVXP5ahshpH65yvLy+U7RO/7FePHcp5uFIjeODI T8MRM+mesc8OE3svQzKF7s277ANfsW2BimfRuTNVKqNHX5OWveTv8RJVSjan0HLW5i PdHLdCcT3+caOWT0Pax0mNqN7Gxtcx9sWCJYHvRBZRBqL0PM9Em6Y2vt7LSp0WVIev jrdc5F/lt3whw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZSr9m518nz4wcr; Thu, 3 Apr 2025 16:38:04 +1100 (AEDT) Date: Thu, 3 Apr 2025 15:50:53 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 03/18] tap: Use iov_tail with tap_add_packet() Message-ID: References: <20250402172343.858187-1-lvivier@redhat.com> <20250402172343.858187-4-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="owu8IPtUG14Y/+Ik" Content-Disposition: inline In-Reply-To: <20250402172343.858187-4-lvivier@redhat.com> Message-ID-Hash: OKA5SYQU43QRW2DS4TWHQZU3M5O3Z7BE X-Message-ID-Hash: OKA5SYQU43QRW2DS4TWHQZU3M5O3Z7BE 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: --owu8IPtUG14Y/+Ik Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 02, 2025 at 07:23:28PM +0200, Laurent Vivier wrote: > Use IOV_PEEK_HEADER() to get the ethernet header from the iovec. >=20 > Move the workaround about multiple iovec array from vu_handle_tx() to > tap_add_packet(). Removing the offset out of the iovec array should > reduce the iovec count to 1. >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson > --- > iov.c | 1 - > pcap.c | 1 + > tap.c | 30 +++++++++++++++++++++--------- > tap.h | 2 +- > vu_common.c | 25 +++++-------------------- > 5 files changed, 28 insertions(+), 31 deletions(-) >=20 > diff --git a/iov.c b/iov.c > index d96fc2ab594b..508fb6da91fb 100644 > --- a/iov.c > +++ b/iov.c > @@ -238,7 +238,6 @@ size_t iov_tail_size(struct iov_tail *tail) > * > * Returns: true if the tail still contains any bytes, otherwise false > */ > -/* cppcheck-suppress unusedFunction */ > bool iov_drop(struct iov_tail *tail, size_t len) > { > tail->off =3D tail->off + len; > diff --git a/pcap.c b/pcap.c > index e95aa6fe29a6..404043a27e22 100644 > --- a/pcap.c > +++ b/pcap.c > @@ -76,6 +76,7 @@ static void pcap_frame(const struct iovec *iov, size_t = iovcnt, > * @pkt: Pointer to data buffer, including L2 headers > * @l2len: L2 frame length > */ > +/* cppcheck-suppress unusedFunction */ > void pcap(const char *pkt, size_t l2len) > { > struct iovec iov =3D { (char *)pkt, l2len }; > diff --git a/tap.c b/tap.c > index 182a1151f139..ab3effe80f89 100644 > --- a/tap.c > +++ b/tap.c > @@ -1040,29 +1040,36 @@ void tap_handler(struct ctx *c, const struct time= spec *now) > /** > * tap_add_packet() - Queue/capture packet, update notion of guest MAC a= ddress > * @c: Execution context > - * @l2len: Total L2 packet length > - * @p: Packet buffer > + * @data: Packet to add to the pool > */ > -void tap_add_packet(struct ctx *c, ssize_t l2len, char *p) > +void tap_add_packet(struct ctx *c, struct iov_tail *data) > { > const struct ethhdr *eh; > + struct ethhdr ehc; > =20 > - pcap(p, l2len); > + pcap_iov(data->iov, data->cnt, data->off); > =20 > - eh =3D (struct ethhdr *)p; > + eh =3D IOV_PEEK_HEADER(data, ehc); > + if (!eh) > + return; > =20 > if (memcmp(c->guest_mac, eh->h_source, ETH_ALEN)) { > memcpy(c->guest_mac, eh->h_source, ETH_ALEN); > proto_update_l2_buf(c->guest_mac, NULL); > } > =20 > + iov_tail_prune(data); > + ASSERT(data->cnt =3D=3D 1); /* packet_add() doesn't support iovec */ > + > switch (ntohs(eh->h_proto)) { > case ETH_P_ARP: > case ETH_P_IP: > - packet_add(pool_tap4, l2len, p); > + packet_add(pool_tap4, data->iov[0].iov_len - data->off, > + (char *)data->iov[0].iov_base + data->off); > break; > case ETH_P_IPV6: > - packet_add(pool_tap6, l2len, p); > + packet_add(pool_tap6, data->iov[0].iov_len - data->off, > + (char *)data->iov[0].iov_base + data->off); > break; > default: > break; > @@ -1128,6 +1135,7 @@ static void tap_passt_input(struct ctx *c, const st= ruct timespec *now) > =20 > while (n >=3D (ssize_t)sizeof(uint32_t)) { > uint32_t l2len =3D ntohl_unaligned(p); > + struct iov_tail data; > =20 > if (l2len < sizeof(struct ethhdr) || l2len > L2_MAX_LEN_PASST) { > err("Bad frame size from guest, resetting connection"); > @@ -1142,7 +1150,8 @@ static void tap_passt_input(struct ctx *c, const st= ruct timespec *now) > p +=3D sizeof(uint32_t); > n -=3D sizeof(uint32_t); > =20 > - tap_add_packet(c, l2len, p); > + data =3D IOV_TAIL_FROM_BUF(p, l2len, 0); > + tap_add_packet(c, &data); > =20 > p +=3D l2len; > n -=3D l2len; > @@ -1186,6 +1195,8 @@ static void tap_pasta_input(struct ctx *c, const st= ruct timespec *now) > for (n =3D 0; > n <=3D (ssize_t)(sizeof(pkt_buf) - L2_MAX_LEN_PASTA); > n +=3D len) { > + struct iov_tail data; > + > len =3D read(c->fd_tap, pkt_buf + n, L2_MAX_LEN_PASTA); > =20 > if (len =3D=3D 0) { > @@ -1207,7 +1218,8 @@ static void tap_pasta_input(struct ctx *c, const st= ruct timespec *now) > len > (ssize_t)L2_MAX_LEN_PASTA) > continue; > =20 > - tap_add_packet(c, len, pkt_buf + n); > + data =3D IOV_TAIL_FROM_BUF(pkt_buf + n, len, 0); > + tap_add_packet(c, &data); > } > =20 > tap_handler(c, now); > diff --git a/tap.h b/tap.h > index dd39fd896f4a..5034acd8ac46 100644 > --- a/tap.h > +++ b/tap.h > @@ -119,6 +119,6 @@ void tap_sock_update_pool(void *base, size_t size); > void tap_backend_init(struct ctx *c); > void tap_flush_pools(void); > void tap_handler(struct ctx *c, const struct timespec *now); > -void tap_add_packet(struct ctx *c, ssize_t l2len, char *p); > +void tap_add_packet(struct ctx *c, struct iov_tail *data); > =20 > #endif /* TAP_H */ > diff --git a/vu_common.c b/vu_common.c > index 686a09b28c8e..e446fc4f2054 100644 > --- a/vu_common.c > +++ b/vu_common.c > @@ -159,7 +159,6 @@ static void vu_handle_tx(struct vu_dev *vdev, int ind= ex, > struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE]; > struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; > struct vu_virtq *vq =3D &vdev->vq[index]; > - int hdrlen =3D sizeof(struct virtio_net_hdr_mrg_rxbuf); > int out_sg_count; > int count; > =20 > @@ -172,6 +171,7 @@ static void vu_handle_tx(struct vu_dev *vdev, int ind= ex, > while (count < VIRTQUEUE_MAX_SIZE && > out_sg_count + VU_MAX_TX_BUFFER_NB <=3D VIRTQUEUE_MAX_SIZE) { > int ret; > + struct iov_tail data; > =20 > elem[count].out_num =3D VU_MAX_TX_BUFFER_NB; > elem[count].out_sg =3D &out_sg[out_sg_count]; > @@ -187,25 +187,10 @@ static void vu_handle_tx(struct vu_dev *vdev, int i= ndex, > warn("virtio-net transmit queue contains no out buffers"); > break; > } > - if (elem[count].out_num =3D=3D 1) { > - tap_add_packet(vdev->context, > - elem[count].out_sg[0].iov_len - hdrlen, > - (char *)elem[count].out_sg[0].iov_base + > - hdrlen); > - } else { > - /* vnet header can be in a separate iovec */ > - if (elem[count].out_num !=3D 2) { > - debug("virtio-net transmit queue contains more than one buffer ([%d]= : %u)", > - count, elem[count].out_num); > - } else if (elem[count].out_sg[0].iov_len !=3D (size_t)hdrlen) { > - debug("virtio-net transmit queue entry not aligned on hdrlen ([%d]: = %d !=3D %zu)", > - count, hdrlen, elem[count].out_sg[0].iov_len); > - } else { > - tap_add_packet(vdev->context, > - elem[count].out_sg[1].iov_len, > - (char *)elem[count].out_sg[1].iov_base); > - } > - } > + > + data =3D IOV_TAIL(elem[count].out_sg, elem[count].out_num, 0); > + if (IOV_DROP_HEADER(&data, struct virtio_net_hdr_mrg_rxbuf)) > + tap_add_packet(vdev->context, &data); > =20 > count++; > } --=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 --owu8IPtUG14Y/+Ik Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmfuE50ACgkQzQJF27ox 2Gfv6Q//QRQbJpOSigcOS2DW/ImswI9nl8CouD3IGMHbiyLpMrNK8pdu5Hhzuktw 6wqp+acZ20y5ll0HL5ohyQW3cjueZOAaj7qOQ9NnrwGRbYmR+XuObNTAerDPUf9s cSnYrp3wkowucGbLat7DaRxi0XBeBNnMAZTtT8X+e5Jqv8on0dKmRc+np2AFwaTJ oaR6aNi1r+t8/NBKjwiNC6FUy8bs+an4bSOVInEsxLBpUjU9cjYl96vE03KsI2+l nQ/qLClWLM7M5F/LKIspk8EOFRTYMREnH3k35ZrQnl41ACAUMFCp9Wjvq1YhOpDe uSDBVGZu5mhuKP33isLTRJy3l5IOqwpYO0n9bnJxFaJw4siMgXWYhw/BnZnEhgx9 WYBRgoAHkJb0Af0LOnSC7y+vA/AR4AI5MynZtIGgUdOfwWbh1sowEan+rysOxM4z Rn8ymoY+vVVEIbgFTQe3CDC4RRmLQGHDi/kvfCAHC+M/R3zo1oqZ/8KpediSbfyv hkWFArY0QGiTo+JTO2hxtQTP+oEhp7+urdEkVd50DUgmWPZyl+ICEsgBRgC2a+eg BB3+JrWjcI23FSiCVC5P0TIqcORBI0QjxPZ3ovPLoAFSeDXWYW62TLUTbrKEOm6C tMA0U3VYrmwzxzbu+6J3vnrRUMS40h3YWn2EAZGmtBfmUZ7NOx4= =8LtI -----END PGP SIGNATURE----- --owu8IPtUG14Y/+Ik--