[PATCH v3 3/4] net/tap: drain queue FD in Rx interrupt mode

[Maxime Leroy]

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 does not busy poll: it waits on the
queue fd through the Rx interrupt API and bursts only after a wakeup. The
trigger is process wide and the signal is sent to the process, since the
fd owner is set with F_SETOWN and getpid(), so it is not necessarily
handled by the thread that was woken for that queue. The burst can
therefore run before the trigger has advanced and return 0 while the fd is
readable. The registration done by rte_intr_rx_ctl() is edge triggered
(EPOLLET), so no further wakeup comes for data that is already queued: the
packet waits for the next arrival, and the last one waits forever.

Seen with grout in interrupt mode: the echo reply to the first ping stays
in the fd until the second request creates a new edge, so the first ping
times out and the second burst returns both replies.

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 | 20 +++++++++++++++++++-
 drivers/net/tap/rte_eth_tap.h |  2 ++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 770d75f0c7..f846ce1fda 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -254,6 +254,12 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive, int persistent)
 		goto error;
 	}
 
+	/* data queue fds in interrupt mode wake through epoll, not the SIGIO
+	 * trigger; the keep-alive fd always keeps SIGIO
+	 */
+	if (!is_keepalive && pmd->intr_mode)
+		return fd;
+
 	/* Find a free realtime signal */
 	for (signo = SIGRTMIN + 1; signo < SIGRTMAX; signo++) {
 		struct sigaction sa;
@@ -477,7 +483,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 +943,16 @@ tap_dev_configure(struct rte_eth_dev *dev)
 	struct pmd_internals *pmd = dev->data->dev_private;
 	bool 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 +963,7 @@ tap_dev_configure(struct rte_eth_dev *dev)
 	}
 
 	pmd->intr_mode = intr_mode;
+	pmd->intr_mode_set = true;
 
 	TAP_LOG(INFO, "%s: %s: TX configured queues number: %u",
 		dev->device->name, pmd->name, dev->data->nb_tx_queues);
@@ -1620,6 +1637,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 df9a8f8a7a..24fd055df9 100644
--- a/drivers/net/tap/rte_eth_tap.h
+++ b/drivers/net/tap/rte_eth_tap.h
@@ -51,6 +51,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 */
+	bool intr_mode;                 /* Rx queue interrupt mode */
 	struct rte_eth_rxmode *rxmode;  /* RX features */
 	struct rte_mbuf *pool;          /* mbufs pool for this queue */
 	struct tun_pi pi;               /* packet info for iovecs */
@@ -95,6 +96,7 @@ struct pmd_internals {
 
 	struct rte_intr_handle *intr_handle;         /* LSC interrupt handle. */
 	bool intr_mode;                   /* Rx queue interrupt mode */
+	bool 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