On Fri, Jul 17, 2026 at 2:34 PM Stephen Hemminger
<[email protected]> 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 <[email protected]> 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: [email protected]
>>
>> Signed-off-by: Maxime Leroy <[email protected]>
>> ---
>> 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
[email protected]
