[PATCH net-next] tun: don't steer a flow to a queue index that no longer exists

[PATCH net-next] tun: don't steer a flow to a queue index that no longer exists

[Umang Pokhriyal]

A multi-queue tun device keeps a flow table that maps each flow to the
index of the queue it last used, so that packets sent back to the flow
go to the same queue. When a queue is removed, the last queue moves
into its index. An entry that still holds the last queue's old index
then holds an index that no longer exists. Packets of that flow are
sent to queue 0 with a "selects TX queue" warning, until the flow sends
again or its entry expires after a few seconds.

Commit 4cc0a3f89707 ("tun: delete the flow entries of the moved queue
on detach") deletes those entries when the queue is removed. As the
Sashiko review linked below points out, a send running at the same
moment can still write the old index back right after the delete.

Rather than taking a lock on every send to close that window, check the
index when an entry is used. If the entry holds an index that no longer
exists, choose the queue by hash, as is done for a flow with no entry.
The index is read only once, since a concurrent send can change it.

The race has not been seen in practice. The fix was tested in a guest
with stale entries left in place. Flows that all went to queue 0 with
the warning are now spread over the remaining queues.

Link: https://lore.kernel.org/netdev/179415734502.994134.10139593761656069984@kernel.org/
Signed-off-by: Umang Pokhriyal <umangpokhriyall@gmail.com>
---
 drivers/net/tun.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index dcec7311e6..5367975ff8 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -471,13 +471,15 @@ static u16 tun_automq_select_queue(struct tun_struct *tun, struct sk_buff *skb)
 	txq = __skb_get_hash_symmetric(skb);
 	e = tun_flow_find(&tun->flows[tun_hashfn(txq)], txq);
 	if (e) {
+		u32 queue_index = READ_ONCE(e->queue_index);
+
 		tun_flow_save_rps_rxhash(e, txq);
-		txq = e->queue_index;
-	} else {
-		txq = reciprocal_scale(txq, numqueues);
+		/* A racing update can leave the old index of a moved queue */
+		if (likely(queue_index < numqueues))
+			return queue_index;
 	}
 
-	return txq;
+	return reciprocal_scale(txq, numqueues);
 }
 
 static u16 tun_ebpf_select_queue(struct tun_struct *tun, struct sk_buff *skb)
-- 
2.53.0