Re: [PATCH 2/2] net/tap: drain queue FD in Rx interrupt mode

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]