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

[Stephen Hemminger]

the coding style against bool is more of a keenel thing.

On Mon, Jul 20, 2026, 10:33 Maxime Leroy <maxime at leroys.fr> wrote:

> On Fri, Jul 17, 2026 at 2:34 PM Stephen Hemminger
> <stephen at networkplumber.org> 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 <maxime at leroys.fr> 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: stable at dpdk.org
> >>
> >> Signed-off-by: Maxime Leroy <maxime at leroys.fr>
> >> ---
> >>  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
> maxime at leroys.fr
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20260720/01b65e53/attachment.htm>