<div dir="auto">the coding style against bool is more of a keenel thing. </div> <div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Mon, Jul 20, 2026, 10:33 Maxime Leroy <<a href="mailto:maxime@leroys.fr">maxime@leroys.fr</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Jul 17, 2026 at 2:34 PM Stephen Hemminger <<a href="mailto:stephen@networkplumber.org" target="_blank" rel="noreferrer">stephen@networkplumber.org</a>> wrote: > > Don't use int as a boolean. Use bool. Ideally find unused pad hole for it Agreed int-as-a-boolean is not great. But bool in a structure is discouraged by coding_style.rst: "Uses of bool in structures are not preferred as is wastes space and it's also not clear as to what type size the bool is." (Ref: LKML) and that very LKML reference is Linus recommending the opposite of bool for struct members: "please don't use bool in structures at all. [...] Use bool mainly as a return type from functions [...]. [...] just make sure the base type is unsigned [...] you can specify the base type as you wish [...] for packing." The current tap PMD follows that too: the existing flags (persist, flow_init, flow_isolate) are all int, and stdbool.h is not even included in the headers. So for v2 I'll use uint8_t for intr_mode and intr_mode_set: fixed 1-byte size, unambiguous representation, and it fits the existing padding so neither struct grows (pmd_internals stays 144 bytes, rx_queue stays 80). That matches both the guide and the referenced LKML guidance. I can also add a third patch at the end of the series converting the existing int flags (persist, flow_init, flow_isolate) to uint8_t, to make the driver consistent > > On Fri, Jul 17, 2026, 11:59 AM Maxime Leroy <<a href="mailto:maxime@leroys.fr" target="_blank" rel="noreferrer">maxime@leroys.fr</a>> wrote: >> >> The tap Rx path is driven by a SIGIO trigger: pmd_rx_burst() returns >> without reading the queue fd unless tap_trigger, bumped by the O_ASYNC >> signal handler, has advanced. That avoids a readv() on every empty poll. >> >> In Rx interrupt mode the application blocks on the queue fd through epoll >> and polls only after a wakeup. The epoll wakeup and the trigger are >> distinct signals, so the burst can return 0 right after a wakeup because >> the trigger has not advanced, leaving the fd readable with no further >> edge: traffic stalls. >> >> When the port is configured for Rx interrupts, drain the fd >> unconditionally in the burst and do not arm the SIGIO trigger on the data >> queue fd. A data queue fd can be closed and recreated on a later setup, >> so the mode must stay constant to keep every fd on the same SIGIO policy: >> it is fixed at configure time and a later change is rejected. Close and >> reopen the port to switch modes. >> >> Fixes: 4870a8cdd968 ("net/tap: support Rx interrupt") >> Cc: <a href="mailto:stable@dpdk.org" target="_blank" rel="noreferrer">stable@dpdk.org</a> >> >> Signed-off-by: Maxime Leroy <<a href="mailto:maxime@leroys.fr" target="_blank" rel="noreferrer">maxime@leroys.fr</a>> >> --- >> drivers/net/tap/rte_eth_tap.c | 18 +++++++++++++++++- >> drivers/net/tap/rte_eth_tap.h | 2 ++ >> 2 files changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c >> index 99ede19e49..ea9ef76335 100644 >> --- a/drivers/net/tap/rte_eth_tap.c >> +++ b/drivers/net/tap/rte_eth_tap.c >> @@ -254,6 +254,10 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive, int persistent) >> goto error; >> } >> >> + /* interrupt mode wakes through epoll on the data queue fd, not the SIGIO trigger */ >> + if (pmd->intr_mode) >> + return fd; >> + >> /* Find a free realtime signal */ >> for (signo = SIGRTMIN + 1; signo < SIGRTMAX; signo++) { >> struct sigaction sa; >> @@ -477,7 +481,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) >> unsigned long num_rx_bytes = 0; >> uint32_t trigger = tap_trigger; >> >> - if (trigger == rxq->trigger_seen) >> + if (!rxq->intr_mode && trigger == rxq->trigger_seen) >> return 0; >> >> process_private = rte_eth_devices[rxq->in_port].process_private; >> @@ -937,6 +941,16 @@ tap_dev_configure(struct rte_eth_dev *dev) >> struct pmd_internals *pmd = dev->data->dev_private; >> int intr_mode = !!dev->data->dev_conf.intr_conf.rxq; >> >> + /* The queue fd is created once and its SIGIO trigger is armed for poll >> + * mode only; the interrupt mode cannot be toggled on an existing port. >> + */ >> + if (pmd->intr_mode_set && pmd->intr_mode != intr_mode) { >> + TAP_LOG(ERR, >> + "%s: Rx interrupt mode is fixed after configure, close and reopen the port to change it", >> + dev->device->name); >> + return -ENOTSUP; >> + } >> + >> if (dev->data->nb_rx_queues != dev->data->nb_tx_queues) { >> TAP_LOG(ERR, >> "%s: number of rx queues %d must be equal to number of tx queues %d", >> @@ -947,6 +961,7 @@ tap_dev_configure(struct rte_eth_dev *dev) >> } >> >> pmd->intr_mode = intr_mode; >> + pmd->intr_mode_set = 1; >> >> TAP_LOG(INFO, "%s: %s: TX configured queues number: %u", >> dev->device->name, pmd->name, dev->data->nb_tx_queues); >> @@ -1620,6 +1635,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev, >> rxq->queue_id = rx_queue_id; >> rxq->max_rx_segs = max_rx_segs; >> rxq->rxmode = &dev->data->dev_conf.rxmode; >> + rxq->intr_mode = internals->intr_mode; >> >> dev->data->rx_queues[rx_queue_id] = rxq; >> int fd = tap_setup_queue(dev, rx_queue_id, 1); >> diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h >> index 3180719c34..abe22aac9f 100644 >> --- a/drivers/net/tap/rte_eth_tap.h >> +++ b/drivers/net/tap/rte_eth_tap.h >> @@ -50,6 +50,7 @@ struct rx_queue { >> uint16_t queue_id; /* queue ID*/ >> struct queue_stats stats; /* Stats for this RX queue */ >> uint16_t max_rx_segs; /* max scatter segments per packet */ >> + uint16_t intr_mode; /* 1 when Rx queue interrupts are used */ >> struct rte_eth_rxmode *rxmode; /* RX features */ >> struct rte_mbuf *pool; /* mbufs pool for this queue */ >> struct tun_pi pi; /* packet info for iovecs */ >> @@ -94,6 +95,7 @@ struct pmd_internals { >> >> struct rte_intr_handle *intr_handle; /* LSC interrupt handle. */ >> int intr_mode; /* Rx queue interrupt mode */ >> + int intr_mode_set; /* intr_mode locked after configure */ >> int ka_fd; /* keep-alive file descriptor */ >> struct rte_mempool *gso_ctx_mp; /* Mempool for GSO packets */ >> }; >> -- >> 2.43.0 >> -- ------------------------------- Maxime Leroy <a href="mailto:maxime@leroys.fr" target="_blank" rel="noreferrer">maxime@leroys.fr</a> </blockquote></div>