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=202510 header.b=LJbpV8SY; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id C31065A0274 for ; Wed, 26 Nov 2025 04:17:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1764127020; bh=0hQE7mNToWvDrSvumHJDzCyvMmE+bu/dUS2oBtDidhE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LJbpV8SYBU4ANW4oRd6phRdOW/+zJSEQbI2vdp70FWYYyUwDPg8xUm2Epxgu+r+c4 bhr0HEPPpf2ws281Ev1jqyIWzVCUHxXiFugIPKPZlYnYV6996Pt82ihb8qhlkYwYOy wnOY4SKlHGzS4X4JzGXB+pNri3MbJA/Lo7d+dmfEIC3Ro8xN2d7KyZ4zQxh8Bg7/dP KRta08JMoCCjOGbMqsGdTY5Xy/hjpDCFF/Kxkd47Z3KhvCzN3uHT+OUhFIsaunoE/q X8UEU/07FrEWGdBWI5cCbIYiJ5AgIl+ry6qA3glS0Riu/sEE0sjMow8H1z4LjElHzX ou5z54v3/2tHQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dGPqc0xpfz4wDT; Wed, 26 Nov 2025 14:17:00 +1100 (AEDT) Date: Wed, 26 Nov 2025 14:16:49 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v2 4/6] tap: Add queue pair parameter throughout the packet processing path Message-ID: References: <20251121165902.1014964-1-lvivier@redhat.com> <20251121165902.1014964-5-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="HuNUkT4glbuBlE53" Content-Disposition: inline In-Reply-To: <20251121165902.1014964-5-lvivier@redhat.com> Message-ID-Hash: OH6NHDSUUTLDACKECSWFG35WUYT3FXPD X-Message-ID-Hash: OH6NHDSUUTLDACKECSWFG35WUYT3FXPD 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: --HuNUkT4glbuBlE53 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Nit: It's not very obvious to me what the salient difference is between "throughout the network stack" (previous patch) and "throughout the packet processing path" (this one). On Fri, Nov 21, 2025 at 05:59:00PM +0100, Laurent Vivier wrote: > For vhost-user multiqueue support, we need to maintain separate packet > pools for each queue pair, as packets from different queues should be > processed independently. >=20 > Previously, we had single global packet pools (pool_tap4 and pool_tap6) > for IPv4 and IPv6 packets. This worked fine for single-queue scenarios > (passt and pasta), but doesn't support multiple concurrent receive queues. >=20 > Convert these to arrays of pools, one per queue pair, indexed by the > queue pair number. The queue pair is simply the virtqueue index divided > by 2 (since each queue pair consists of one RX and one TX virtqueue). >=20 > Add a qpair parameter throughout the packet processing pipeline: > - tap4_handler() and tap6_handler(): specify which queue's packets to pro= cess > - tap_flush_pools(): specify which queue's pools to flush > - tap_handler(): specify which queue to handle > - tap_add_packet(): specify which queue's pool to add the packet to >=20 > For passt and pasta (single queue), all calls use qpair=3D0. For vhost-us= er > multiqueue, the queue pair is derived from the virtqueue index (index / 2) > in vu_handle_tx(). >=20 > The pool initialization in tap_sock_update_pool() is updated to initialize > all queue pairs' pools from the same underlying buffer. >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson One nit below. > --- > tap.c | 107 ++++++++++++++++++++++++++++++---------------------- > tap.h | 7 ++-- > vu_common.c | 6 +-- > 3 files changed, 68 insertions(+), 52 deletions(-) >=20 > diff --git a/tap.c b/tap.c > index a842104687b7..529acecc9851 100644 > --- a/tap.c > +++ b/tap.c > @@ -94,9 +94,13 @@ CHECK_FRAME_LEN(L2_MAX_LEN_VU); > DIV_ROUND_UP(sizeof(pkt_buf), \ > ETH_HLEN + sizeof(struct ipv6hdr) + sizeof(struct udphdr)) > =20 > -/* IPv4 (plus ARP) and IPv6 message batches from tap/guest to IP handler= s */ > -static PACKET_POOL_NOINIT(pool_tap4, TAP_MSGS_IP4); > -static PACKET_POOL_NOINIT(pool_tap6, TAP_MSGS_IP6); > +/* IPv4 (plus ARP) and IPv6 message batches from tap/guest to IP handlers > + * One pool per queue pair for multiqueue support > + */ > +static PACKET_POOL_DECL(pool_tap4, TAP_MSGS_IP4) pool_tap4_storage[VHOST= _USER_MAX_VQS / 2]; > +static struct pool *pool_tap4[VHOST_USER_MAX_VQS / 2]; > +static PACKET_POOL_DECL(pool_tap6, TAP_MSGS_IP6) pool_tap6_storage[VHOST= _USER_MAX_VQS / 2]; > +static struct pool *pool_tap6[VHOST_USER_MAX_VQS / 2]; > =20 > #define TAP_SEQS 128 /* Different L4 tuples in one batch */ > #define FRAGMENT_MSG_RATE 10 /* # seconds between fragment warnings */ > @@ -703,21 +707,22 @@ static bool tap4_is_fragment(const struct iphdr *ip= h, > /** > * tap4_handler() - IPv4 and ARP packet handler for tap file descriptor > * @c: Execution context > + * @qpair: Queue pair > * @now: Current timestamp > * > * Return: count of packets consumed by handlers > */ > -static int tap4_handler(struct ctx *c, const struct timespec *now) > +static int tap4_handler(struct ctx *c, unsigned int qpair, const struct = timespec *now) > { > unsigned int i, j, seq_count; > struct tap4_l4_t *seq; > =20 > - if (!c->ifi4 || !pool_tap4->count) > - return pool_tap4->count; > + if (!c->ifi4 || !pool_tap4[qpair]->count) > + return pool_tap4[qpair]->count; > =20 > i =3D 0; > resume: > - for (seq_count =3D 0, seq =3D NULL; i < pool_tap4->count; i++) { > + for (seq_count =3D 0, seq =3D NULL; i < pool_tap4[qpair]->count; i++) { > size_t l3len, hlen, l4len; > struct ethhdr eh_storage; > struct iphdr iph_storage; > @@ -727,7 +732,7 @@ resume: > struct iov_tail data; > struct iphdr *iph; > =20 > - if (!packet_get(pool_tap4, i, &data)) > + if (!packet_get(pool_tap4[qpair], i, &data)) > continue; > =20 > eh =3D IOV_PEEK_HEADER(&data, eh_storage); > @@ -794,8 +799,8 @@ resume: > if (iph->protocol =3D=3D IPPROTO_UDP) { > struct iov_tail eh_data; > =20 > - packet_get(pool_tap4, i, &eh_data); > - if (dhcp(c, 0, &eh_data)) > + packet_get(pool_tap4[qpair], i, &eh_data); > + if (dhcp(c, qpair, &eh_data)) > continue; > } > =20 > @@ -825,7 +830,7 @@ resume: > goto append; > =20 > if (seq_count =3D=3D TAP_SEQS) > - break; /* Resume after flushing if i < pool_tap4->count */ > + break; /* Resume after flushing if i < pool_tap4[qpair]->count */ Nit: overlong line > =20 > for (seq =3D tap4_l4 + seq_count - 1; seq >=3D tap4_l4; seq--) { > if (L4_MATCH(iph, uh, seq)) { > @@ -871,30 +876,31 @@ append: > } > } > =20 > - if (i < pool_tap4->count) > + if (i < pool_tap4[qpair]->count) > goto resume; > =20 > - return pool_tap4->count; > + return pool_tap4[qpair]->count; > } > =20 > /** > * tap6_handler() - IPv6 packet handler for tap file descriptor > * @c: Execution context > + * @qpair: Queue pair > * @now: Current timestamp > * > * Return: count of packets consumed by handlers > */ > -static int tap6_handler(struct ctx *c, const struct timespec *now) > +static int tap6_handler(struct ctx *c, unsigned int qpair, const struct = timespec *now) > { > unsigned int i, j, seq_count =3D 0; > struct tap6_l4_t *seq; > =20 > - if (!c->ifi6 || !pool_tap6->count) > - return pool_tap6->count; > + if (!c->ifi6 || !pool_tap6[qpair]->count) > + return pool_tap6[qpair]->count; > =20 > i =3D 0; > resume: > - for (seq_count =3D 0, seq =3D NULL; i < pool_tap6->count; i++) { > + for (seq_count =3D 0, seq =3D NULL; i < pool_tap6[qpair]->count; i++) { > size_t l4len, plen, check; > struct in6_addr *saddr, *daddr; > struct ipv6hdr ip6h_storage; > @@ -906,7 +912,7 @@ resume: > struct ipv6hdr *ip6h; > uint8_t proto; > =20 > - if (!packet_get(pool_tap6, i, &data)) > + if (!packet_get(pool_tap6[qpair], i, &data)) > return -1; > =20 > eh =3D IOV_REMOVE_HEADER(&data, eh_storage); > @@ -1014,7 +1020,7 @@ resume: > goto append; > =20 > if (seq_count =3D=3D TAP_SEQS) > - break; /* Resume after flushing if i < pool_tap6->count */ > + break; /* Resume after flushing if i < pool_tap6[qpair]->count */ > =20 > for (seq =3D tap6_l4 + seq_count - 1; seq >=3D tap6_l4; seq--) { > if (L4_MATCH(ip6h, proto, uh, seq)) { > @@ -1061,39 +1067,42 @@ append: > } > } > =20 > - if (i < pool_tap6->count) > + if (i < pool_tap6[qpair]->count) > goto resume; > =20 > - return pool_tap6->count; > + return pool_tap6[qpair]->count; > } > =20 > /** > - * tap_flush_pools() - Flush both IPv4 and IPv6 packet pools > + * tap_flush_pools() - Flush both IPv4 and IPv6 packet pools for a given= qpair > */ > -void tap_flush_pools(void) > +void tap_flush_pools(unsigned int qpair) > { > - pool_flush(pool_tap4); > - pool_flush(pool_tap6); > + pool_flush(pool_tap4[qpair]); > + pool_flush(pool_tap6[qpair]); > } > =20 > /** > * tap_handler() - IPv4/IPv6 and ARP packet handler for tap file descrip= tor > * @c: Execution context > + * @qpair: Queue pair > * @now: Current timestamp > */ > -void tap_handler(struct ctx *c, const struct timespec *now) > +void tap_handler(struct ctx *c, unsigned int qpair, const struct timespe= c *now) > { > - tap4_handler(c, now); > - tap6_handler(c, now); > + ASSERT(qpair < VHOST_USER_MAX_VQS / 2); > + tap4_handler(c, qpair, now); > + tap6_handler(c, qpair, now); > } > =20 > /** > * tap_add_packet() - Queue/capture packet, update notion of guest MAC a= ddress > * @c: Execution context > + * @qpair: Queue pair > * @data: Packet to add to the pool > * @now: Current timestamp > */ > -void tap_add_packet(struct ctx *c, struct iov_tail *data, > +void tap_add_packet(struct ctx *c, unsigned int qpair, struct iov_tail *= data, > const struct timespec *now) > { > struct ethhdr eh_storage; > @@ -1114,21 +1123,23 @@ void tap_add_packet(struct ctx *c, struct iov_tai= l *data, > proto_update_l2_buf(c->guest_mac); > } > =20 > + ASSERT(qpair < VHOST_USER_MAX_VQS / 2); > + > switch (ntohs(eh->h_proto)) { > case ETH_P_ARP: > case ETH_P_IP: > - if (!pool_can_fit(pool_tap4, data)) { > - tap4_handler(c, now); > - pool_flush(pool_tap4); > + if (!pool_can_fit(pool_tap4[qpair], data)) { > + tap4_handler(c, qpair, now); > + pool_flush(pool_tap4[qpair]); > } > - packet_add(pool_tap4, data); > + packet_add(pool_tap4[qpair], data); > break; > case ETH_P_IPV6: > - if (!pool_can_fit(pool_tap6, data)) { > - tap6_handler(c, now); > - pool_flush(pool_tap6); > + if (!pool_can_fit(pool_tap6[qpair], data)) { > + tap6_handler(c, qpair, now); > + pool_flush(pool_tap6[qpair]); > } > - packet_add(pool_tap6, data); > + packet_add(pool_tap6[qpair], data); > break; > default: > break; > @@ -1168,7 +1179,7 @@ static void tap_passt_input(struct ctx *c, const st= ruct timespec *now) > ssize_t n; > char *p; > =20 > - tap_flush_pools(); > + tap_flush_pools(0); > =20 > if (partial_len) { > /* We have a partial frame from an earlier pass. Move it to the > @@ -1212,7 +1223,7 @@ static void tap_passt_input(struct ctx *c, const st= ruct timespec *now) > n -=3D sizeof(uint32_t); > =20 > data =3D IOV_TAIL_FROM_BUF(p, l2len, 0); > - tap_add_packet(c, &data, now); > + tap_add_packet(c, 0, &data, now); > =20 > p +=3D l2len; > n -=3D l2len; > @@ -1221,7 +1232,7 @@ static void tap_passt_input(struct ctx *c, const st= ruct timespec *now) > partial_len =3D n; > partial_frame =3D p; > =20 > - tap_handler(c, now); > + tap_handler(c, 0, now); > } > =20 > /** > @@ -1251,7 +1262,7 @@ static void tap_pasta_input(struct ctx *c, const st= ruct timespec *now) > { > ssize_t n, len; > =20 > - tap_flush_pools(); > + tap_flush_pools(0); > =20 > for (n =3D 0; > n <=3D (ssize_t)(sizeof(pkt_buf) - L2_MAX_LEN_PASTA); > @@ -1280,10 +1291,10 @@ static void tap_pasta_input(struct ctx *c, const = struct timespec *now) > continue; > =20 > data =3D IOV_TAIL_FROM_BUF(pkt_buf + n, len, 0); > - tap_add_packet(c, &data, now); > + tap_add_packet(c, 0, &data, now); > } > =20 > - tap_handler(c, now); > + tap_handler(c, 0, now); > } > =20 > /** > @@ -1487,10 +1498,14 @@ static void tap_sock_tun_init(struct ctx *c) > */ > static void tap_sock_update_pool(void *base, size_t size) > { > - int i; > + unsigned int i; > =20 > - pool_tap4_storage =3D PACKET_INIT(pool_tap4, TAP_MSGS_IP4, base, size); > - pool_tap6_storage =3D PACKET_INIT(pool_tap6, TAP_MSGS_IP6, base, size); > + for (i =3D 0; i < VHOST_USER_MAX_VQS / 2; i++) { > + pool_tap4_storage[i] =3D PACKET_INIT(pool_tap4, TAP_MSGS_IP4, base, si= ze); > + pool_tap4[i] =3D (struct pool *)&pool_tap4_storage[i]; > + pool_tap6_storage[i] =3D PACKET_INIT(pool_tap6, TAP_MSGS_IP6, base, si= ze); > + pool_tap6[i] =3D (struct pool *)&pool_tap6_storage[i]; > + } > =20 > for (i =3D 0; i < TAP_SEQS; i++) { > tap4_l4[i].p =3D PACKET_INIT(pool_l4, UIO_MAXIOV, base, size); > diff --git a/tap.h b/tap.h > index 92d3e5446991..f10c2a212a51 100644 > --- a/tap.h > +++ b/tap.h > @@ -118,8 +118,9 @@ void tap_handler_passt(struct ctx *c, uint32_t events, > int tap_sock_unix_open(char *sock_path); > void tap_sock_reset(struct ctx *c); > 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, struct iov_tail *data, > +void tap_flush_pools(unsigned int qpair); > +void tap_handler(struct ctx *c, unsigned int qpair, > + const struct timespec *now); > +void tap_add_packet(struct ctx *c, unsigned int qpair, struct iov_tail *= data, > const struct timespec *now); > #endif /* TAP_H */ > diff --git a/vu_common.c b/vu_common.c > index 040ad067ffbf..63ead85a4674 100644 > --- a/vu_common.c > +++ b/vu_common.c > @@ -170,7 +170,7 @@ static void vu_handle_tx(struct vu_dev *vdev, int ind= ex, > =20 > ASSERT(VHOST_USER_IS_QUEUE_TX(index)); > =20 > - tap_flush_pools(); > + tap_flush_pools(index / 2); > =20 > count =3D 0; > out_sg_count =3D 0; > @@ -196,11 +196,11 @@ static void vu_handle_tx(struct vu_dev *vdev, int i= ndex, > =20 > 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, now); > + tap_add_packet(vdev->context, index / 2, &data, now); > =20 > count++; > } > - tap_handler(vdev->context, now); > + tap_handler(vdev->context, index / 2, now); > =20 > if (count) { > int i; > --=20 > 2.51.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 --HuNUkT4glbuBlE53 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmkmcSEACgkQzQJF27ox 2GeE/g/8CjIi8RzWTGDcQjCGStsPbeYzoFDa4/o/Dolf6vLGOJfupZkMNL09SCg7 EIdnxuAc9pWSTtgaMdMWdNpDM/yjJ3QUemq35CRiikonpc744DUIzy4br5sqEbSJ xF4PscEfoINPSuJXTHqhyVbSr8MXSheUHvdHU9y14eoqcIIaOzsWR1hXCv1RMIJp klb0REZzSkfQIC9jMgnhR/9ceOklHVmXq8yIRkKO8JTBVfDCzWJzBt8ADcTld0Fk XWjgMlK+aIlIYl2CpibGdbE4wqe5hNMbJWOcikRKMDiZ+LCWhr1wuaWyZezDXWxI KR5uuFBu9E0KLSd1JMq9kOA9knmNov6mXqkmt7yrbGV6vNTW12WwDr23O5Zl2SJV A23/B8JlUEfy2zA0ZkofjXpdu7yewVQcZDOiifqunClBWLGr1ZMmiQxCNlYVMuAB z5RVUAx9o8n3qwdasgbuhrsgYIv7eHSsIYwKR8+EDWQkOMJ+Ei77HVAkXVokkR3K p9cvt3JaLJ98G3oJx/Wqhdku1vHDMPo/Bb4xiK7eo/Ct6DJeE78YcfqNAm1jry+L YhpXSNz2tJP3Wk90BcUQ7rtRBfs74Y6WAzKUpTGj68LEJZIqflEoQEDoQcn987R8 ooFiNa6GqHJHf7+BcbgVJjKmjrjkBdcJIGhsmHRvLE9AuSP8B0E= =GsIJ -----END PGP SIGNATURE----- --HuNUkT4glbuBlE53--