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=202608 header.b=qv0Fhtdp; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id CE4485A0619 for ; Mon, 03 Aug 2026 05:46:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202608; t=1785728795; bh=fNs0uHwiCRnnWqMHyOY20gHekNNTjS5hyKtbciU0enc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qv0Fhtdp3V8TpULjg2MDknx/5HmScHY5c/NHSR6dBYaJiWM6z4UHu86hwipePrJFW pcCwAA8v2vmSQDJ+Nvld+4wZTyDrZo5hIZ8fd+mXA/L52Jf10xVXXJWykGxgtYSeX+ vjpdnu61Xva7NFYHexTzN/2Icff5TaIT8aNyS+JxnV1ZZo6EV+Fk6pW6nDVR/uU/Dx fOzfgI70vNH5LUQhvL0WC774MVcAv86bt5djtvLiFMGJaeEsOYDx8Hd16bYHw4iSFq JyZTGcEeBljTT6EIiiLWERWFD1THT3Tib8qBk/Gkgy2mB7gj3JkrGGJm4TWZj8Q0b8 nTIuodF+UMQeA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4hD2fM2M66z4wJT; Mon, 03 Aug 2026 13:46:35 +1000 (AEST) Date: Mon, 3 Aug 2026 13:46:20 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v6 12/12] flow: Derive epoll fd from queue pair, removing epollid field Message-ID: References: <20260731161617.3550626-1-lvivier@redhat.com> <20260731161617.3550626-13-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="nCoaMHxa9XDogfwm" Content-Disposition: inline In-Reply-To: <20260731161617.3550626-13-lvivier@redhat.com> Message-ID-Hash: IJ7YDQW4I52TTV7EBUMX7B63M3LVNR7D X-Message-ID-Hash: IJ7YDQW4I52TTV7EBUMX7B63M3LVNR7D 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: --nCoaMHxa9XDogfwm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 31, 2026 at 06:16:17PM +0200, Laurent Vivier wrote: > Since each queue pair maps to exactly one epoll instance, the epoll > file descriptor can be looked up directly from the flow's qpair > field. This makes the separate epollid field and its indirection > through epoll_id_to_fd[] redundant. >=20 > Replace epoll_id_to_fd[] with qpair_to_fd[], derived from the qpair > assigned at flow creation. Remove flow_epollid_set(), > flow_epollid_register(), flow_qp()/FLOW_QP(), and the epollid field > from flow_common. flow_epollfd() now simply returns > qpair_to_fd[f->qpair]. >=20 > flow_init() now takes the execution context to initialise > qpair_to_fd[] from c->epollfd. >=20 > Signed-off-by: Laurent Vivier A few minor comments, otherwise LGTM. > --- > flow.c | 55 +++++++++------------------------------------------- > flow.h | 20 +++++-------------- > icmp.c | 1 - > passt.c | 3 +-- > tcp.c | 3 --- > tcp_splice.c | 1 - > udp_flow.c | 1 - > 7 files changed, 15 insertions(+), 69 deletions(-) >=20 > diff --git a/flow.c b/flow.c > index 40b2e68be0ab..1efa9b966991 100644 > --- a/flow.c > +++ b/flow.c > @@ -130,7 +130,7 @@ static_assert(ARRAY_SIZE(flow_epoll) =3D=3D FLOW_NUM_= TYPES, > unsigned flow_first_free; > union flow flowtab[FLOW_MAX]; > static const union flow *flow_new_entry; /* =3D NULL */ > -static int epoll_id_to_fd[EPOLLFD_ID_SIZE]; > +int qpair_to_fd[FLOW_QPAIR_SIZE]; Nit: I'd suggest qpair_to_epollfd[], since there could bve a bunch of fds related to a qpair one way or another. > =20 > /* Hash table to index it */ > #define FLOW_HASH_LOAD 70 /* % */ > @@ -364,19 +364,7 @@ static void flow_set_state(struct flow_common *f, en= um flow_state state) > */ > int flow_epollfd(const struct flow_common *f) > { > - return epoll_id_to_fd[f->epollid]; > -} > - > -/** > - * flow_epollid_set() - Associate a flow with an epoll id > - * @f: Flow to update > - * @epollid: epoll id to associate with this flow > - */ > -void flow_epollid_set(struct flow_common *f, int epollid) > -{ > - assert(epollid < EPOLLFD_ID_SIZE); > - > - f->epollid =3D epollid; > + return qpair_to_fd[f->qpair]; > } > =20 > /** > @@ -405,31 +393,6 @@ int flow_epoll_set(const struct flow_common *f, int = command, uint32_t events, > return epoll_ctl(flow_epollfd(f), command, fd, &ev); > } > =20 > -/** > - * flow_epollid_register() - Initialize the epoll id -> fd mapping > - * @epollid: epoll id to associate to > - * @epollfd: epoll file descriptor for this epoll id > - */ > -void flow_epollid_register(int epollid, int epollfd) > -{ > - assert(epollid < EPOLLFD_ID_SIZE); > - > - epoll_id_to_fd[epollid] =3D epollfd; > -} > - > -/** > - * flow_qp() - Get the queue pair for a flow > - * @f: Flow to query (may be NULL) > - * > - * Return: queue pair number for the flow, or 0 if flow is NULL or has no > - * valid queue pair assignment > - */ > -/* cppcheck-suppress unusedFunction */ > -unsigned int flow_qp(const struct flow_common *f) > -{ > - return f->qpair; > -} > - > /** > * flow_setqp() - Set queue pair assignment for a flow > * @f: Flow to update > @@ -437,13 +400,9 @@ unsigned int flow_qp(const struct flow_common *f) > */ > static void flow_setqp(struct flow_common *f, unsigned int qpair) > { > - assert(qpair < FLOW_QPAIR_MAX); > - > - if (f->qpair =3D=3D qpair) > - return; > + assert(qpair < FLOW_QPAIR_SIZE); > =20 > - flow_trace((union flow *)f, "updating queue pair from %d to %d", > - f->qpair, qpair); > + flow_trace((union flow *)f, "setting queue pair to %d", qpair); > =20 > f->qpair =3D qpair; > } > @@ -1299,8 +1258,9 @@ int flow_migrate_target(struct ctx *c, const struct= migrate_stage *stage, > =20 > /** > * flow_init() - Initialise flow related data structures > + * @c: Execution context > */ > -void flow_init(void) > +void flow_init(const struct ctx *c) > { > unsigned b; > =20 > @@ -1310,4 +1270,7 @@ void flow_init(void) > =20 > for (b =3D 0; b < FLOW_HASH_SIZE; b++) > flow_hashtab[b] =3D FLOW_SIDX_NONE; > + > + for (b =3D 0; b < FLOW_QPAIR_SIZE; b++) > + qpair_to_fd[b] =3D c->epollfd; > } > diff --git a/flow.h b/flow.h > index b6f980b4f826..028d42bccc0c 100644 > --- a/flow.h > +++ b/flow.h > @@ -157,6 +157,8 @@ struct flowside { > in_port_t eport; > }; > =20 > +extern int qpair_to_fd[]; Does this need to be extern? Looks like everything outside flow.h accesses it indirectly via flow_epollfd()? > + > /** > * flowside_eq() - Check if two flowsides are equal > * @left, @right: Flowsides to compare > @@ -203,19 +205,12 @@ struct flow_common { > =20 > uint8_t tap_omac[6]; > =20 > -#define EPOLLFD_ID_BITS 8 > - unsigned int epollid:EPOLLFD_ID_BITS; > #define FLOW_QPAIR_BITS 5 > unsigned int qpair:FLOW_QPAIR_BITS; > }; > =20 > -#define EPOLLFD_ID_DEFAULT 0 > -#define EPOLLFD_ID_SIZE (1 << EPOLLFD_ID_BITS) > - > -#define FLOW_QPAIR_NUM (1 << FLOW_QPAIR_BITS) > -#define FLOW_QPAIR_MAX (FLOW_QPAIR_NUM - 1) > - > -static_assert(VHOST_USER_MAX_VQS <=3D FLOW_QPAIR_MAX * 2); > +#define FLOW_QPAIR_SIZE (1 << FLOW_QPAIR_BITS) > +static_assert(VHOST_USER_MAX_VQS <=3D FLOW_QPAIR_SIZE * 2); > =20 > #define FLOW_INDEX_BITS 17 /* 128k - 1 */ > #define FLOW_MAX MAX_FROM_BITS(FLOW_INDEX_BITS) > @@ -271,15 +266,10 @@ flow_sidx_t flow_lookup_sa(const struct ctx *c, uin= t8_t proto, uint8_t pif, > =20 > union flow; > =20 > -void flow_init(void); > +void flow_init(const struct ctx *c); > int flow_epollfd(const struct flow_common *f); > -void flow_epollid_set(struct flow_common *f, int epollid); > int flow_epoll_set(const struct flow_common *f, int command, uint32_t ev= ents, > int fd, unsigned int sidei); > -void flow_epollid_register(int epollid, int epollfd); > -unsigned int flow_qp(const struct flow_common *f); > -#define FLOW_QP(flow_) \ > - (flow_qp(&(flow_)->f)) > =20 > void flow_defer_handler(const struct ctx *c, const struct timespec *now, > unsigned int qpair); > diff --git a/icmp.c b/icmp.c > index e75d66d168be..46c5d925600a 100644 > --- a/icmp.c > +++ b/icmp.c > @@ -219,7 +219,6 @@ static struct icmp_ping_flow *icmp_ping_new(const str= uct ctx *c, > if (pingf->sock > FD_REF_MAX) > goto cancel; > =20 > - flow_epollid_set(&pingf->f, EPOLLFD_ID_DEFAULT); > if (flow_epoll_set(&pingf->f, EPOLL_CTL_ADD, EPOLLIN, pingf->sock, > TGTSIDE) < 0) { > close(pingf->sock); > diff --git a/passt.c b/passt.c > index 55708d96fb06..bc9f1575ef8c 100644 > --- a/passt.c > +++ b/passt.c > @@ -382,7 +382,6 @@ int main(int argc, char **argv) > c->epollfd =3D epoll_create1(EPOLL_CLOEXEC); > if (c->epollfd =3D=3D -1) > die_perror("Failed to create epoll file descriptor"); > - flow_epollid_register(EPOLLFD_ID_DEFAULT, c->epollfd); > =20 > if (getrlimit(RLIMIT_NOFILE, &limit)) > die_perror("Failed to get maximum value of open files limit"); > @@ -405,7 +404,7 @@ int main(int argc, char **argv) > if (clock_gettime(CLOCK_MONOTONIC, &now)) > die_perror("Failed to get CLOCK_MONOTONIC time"); > =20 > - flow_init(); > + flow_init(c); > fwd_scan_ports_init(c); > =20 > if ((!c->no_udp && udp_init(c)) || (!c->no_tcp && tcp_init(c))) > diff --git a/tcp.c b/tcp.c > index 39f732f8bea3..e80a64e38f24 100644 > --- a/tcp.c <> +++ b/tcp.c > @@ -1764,7 +1764,6 @@ static void tcp_conn_from_tap(const struct ctx *c, = unsigned int qpair, > =20 > conn->sock =3D s; > conn->timer =3D -1; > - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, TGTSIDE) < 0) { > flow_perror_ratelimit(flow, now, "Can't register with epoll"); > goto cancel; > @@ -2578,7 +2577,6 @@ static void tcp_tap_conn_from_sock(const struct ctx= *c, union flow *flow, > conn->timer =3D -1; > conn->ws_to_tap =3D conn->ws_from_tap =3D 0; > =20 > - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, s, INISIDE) < 0) { > flow_perror_ratelimit(flow, now, "Can't register with epoll"); > conn_flag(c, conn, CLOSING, now); > @@ -3876,7 +3874,6 @@ int tcp_flow_migrate_target(struct ctx *c, int fd) > goto out; > } > =20 > - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->sock, > !TAPSIDE(conn))) > goto out; /* tcp_flow_migrate_target_ext() will clean this up */ > diff --git a/tcp_splice.c b/tcp_splice.c > index 4b01f1aa3602..8617e62cea9a 100644 > --- a/tcp_splice.c > +++ b/tcp_splice.c > @@ -387,7 +387,6 @@ static int tcp_splice_connect(const struct ctx *c, st= ruct tcp_splice_conn *conn, > =20 > pif_sockaddr(c, &sa, tgtpif, &tgt->eaddr, tgt->eport); > =20 > - flow_epollid_set(&conn->f, EPOLLFD_ID_DEFAULT); > if (flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->s[0], 0) || > flow_epoll_set(&conn->f, EPOLL_CTL_ADD, 0, conn->s[1], 1)) { > int ret =3D -errno; > diff --git a/udp_flow.c b/udp_flow.c > index 2578d640a3da..8e985df8398d 100644 > --- a/udp_flow.c > +++ b/udp_flow.c > @@ -154,7 +154,6 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, = union flow *flow, > uflow->ttl[INISIDE] =3D uflow->ttl[TGTSIDE] =3D 0; > uflow->activity[INISIDE] =3D 1; > uflow->activity[TGTSIDE] =3D 0; > - flow_epollid_set(&uflow->f, EPOLLFD_ID_DEFAULT); > =20 > flow_foreach_sidei(sidei) { > if (pif_is_socket(uflow->f.pif[sidei])) > --=20 > 2.54.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 --nCoaMHxa9XDogfwm Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpwDwIACgkQzQJF27ox 2Gf70g//UwWMWCcT/vluK7ab6cH8x6M5CFQ1xdNYw8GNqt82QttBSZbUhCKhSP48 1yI3dFhbBslsGLvMAwoYl8fspYbba/o/pSx7IQh+LBetIU2wI2YaqWr6lK6VMlO8 hP96GNUvlfhydvRJ+Du9EyWSE54aD/YX8xTfpWyDdlYhzMdVApMR7ZW0hCwmYJ47 HHhRJSaYOCBqY472lXi6DCv1Te6sw7XZHIImr939DIGV1L8wNeWONq1KOLH/6FR2 OTEBpsSe+28g4j7wpyWGdWVbpVg8HOO3mJPGrPjjHRWQjlCSGj6gYiO7b4VJ99cI sYIaJXkmcyrvEvgf+IIfr+fQ/sVuL8syORjdUCfvrwR8ofDPH4sxjkct3hnIxOle V5ppSVZLaPP2WKKLtgsSidIYxcVPlspdRmzVzzg7nENYUHYjtYiOzvzxc8ir3ww4 GERomv956APCMoKo6kdoYx0jueL+MJk/+ElVk7X/fIhGXMjySQnSrSSGmk7Wdw2T CaFBSy9M2nv0BnguT4vgaFQ+GTO4V6mU9p7NvlJw4NYw12yCZHYYHWbyRfpm/pPf 5D3kggLS8RREXPDfsMqjFEKGLTp1QepTgwqAiglD/FkWaAH/kMciAyctMhUvKSju XxCM1xCawOavZkBLp30k/aGpLzaZuIBCy51Ephr1HHzA6FyYgr4= =2w9r -----END PGP SIGNATURE----- --nCoaMHxa9XDogfwm--