net: Provide a PREEMPT_RT specific check for netdev_queue::_xmit_lock

After acquiring netdev_queue::_xmit_lock the number of the CPU owning
the lock is recorded in netdev_queue::xmit_lock_owner. This works as
long as the BH context is not preemptible.

On PREEMPT_RT the softirq context is preemptible and without the
softirq-lock it is possible to have multiple user in __dev_queue_xmit()
submitting a skb on the same CPU. This is fine in general but this means
also that the current CPU is recorded as netdev_queue::xmit_lock_owner.
This in turn leads to the recursion alert and the skb is dropped.

Instead checking the for CPU number, that owns the lock, PREEMPT_RT can
check if the lockowner matches the current task.

Add netif_tx_owned() which returns true if the current context owns the
lock by comparing the provided CPU number with the recorded number. This
resembles the current check by negating the condition (the current check
returns true if the lock is not owned).
On PREEMPT_RT use rt_mutex_owner() to return the lock owner and compare
the current task against it.
Use the new helper in __dev_queue_xmit() and netif_local_xmit_active()
which provides a similar check.
Update comments regarding pairing READ_ONCE().

Reported-by: Bert Karwatzki <spasswolf@web.de>
Closes: https://lore.kernel.org/all/20260216134333.412332-1-spasswolf@web.de
Fixes: 3253cb49cbad4 ("softirq: Allow to drop the softirq-BKL lock on PREEMPT_RT")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reported-by: Bert Karwatzki <spasswolf@web.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://patch.msgid.link/20260302162631.uGUyIqDT@linutronix.de
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d4e6e00bb90a72c58c73a545c551f16dd668fbe7..67e25f6d15a4728b6e6bcb0d448fb60b4b89fca9 100644 (file)
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4711,7 +4711,7 @@ static inline u32 netif_msg_init(int debug_value, int default_msg_enable_bits)
 static inline void __netif_tx_lock(struct netdev_queue *txq, int cpu)
 {
        spin_lock(&txq->_xmit_lock);
-       /* Pairs with READ_ONCE() in __dev_queue_xmit() */
+       /* Pairs with READ_ONCE() in netif_tx_owned() */
        WRITE_ONCE(txq->xmit_lock_owner, cpu);
 }
 
@@ -4729,7 +4729,7 @@ static inline void __netif_tx_release(struct netdev_queue *txq)
 static inline void __netif_tx_lock_bh(struct netdev_queue *txq)
 {
        spin_lock_bh(&txq->_xmit_lock);
-       /* Pairs with READ_ONCE() in __dev_queue_xmit() */
+       /* Pairs with READ_ONCE() in netif_tx_owned() */
        WRITE_ONCE(txq->xmit_lock_owner, smp_processor_id());
 }
 
@@ -4738,7 +4738,7 @@ static inline bool __netif_tx_trylock(struct netdev_queue *txq)
        bool ok = spin_trylock(&txq->_xmit_lock);
 
        if (likely(ok)) {
-               /* Pairs with READ_ONCE() in __dev_queue_xmit() */
+               /* Pairs with READ_ONCE() in netif_tx_owned() */
                WRITE_ONCE(txq->xmit_lock_owner, smp_processor_id());
        }
        return ok;
@@ -4746,14 +4746,14 @@ static inline bool __netif_tx_trylock(struct netdev_queue *txq)
 
 static inline void __netif_tx_unlock(struct netdev_queue *txq)
 {
-       /* Pairs with READ_ONCE() in __dev_queue_xmit() */
+       /* Pairs with READ_ONCE() in netif_tx_owned() */
        WRITE_ONCE(txq->xmit_lock_owner, -1);
        spin_unlock(&txq->_xmit_lock);
 }
 
 static inline void __netif_tx_unlock_bh(struct netdev_queue *txq)
 {
-       /* Pairs with READ_ONCE() in __dev_queue_xmit() */
+       /* Pairs with READ_ONCE() in netif_tx_owned() */
        WRITE_ONCE(txq->xmit_lock_owner, -1);
        spin_unlock_bh(&txq->_xmit_lock);
 }
@@ -4846,6 +4846,23 @@ static inline void netif_tx_disable(struct net_device *dev)
        local_bh_enable();
 }
 
+#ifndef CONFIG_PREEMPT_RT
+static inline bool netif_tx_owned(struct netdev_queue *txq, unsigned int cpu)
+{
+       /* Other cpus might concurrently change txq->xmit_lock_owner
+        * to -1 or to their cpu id, but not to our id.
+        */
+       return READ_ONCE(txq->xmit_lock_owner) == cpu;
+}
+
+#else
+static inline bool netif_tx_owned(struct netdev_queue *txq, unsigned int cpu)
+{
+       return rt_mutex_owner(&txq->_xmit_lock.lock) == current;
+}
+
+#endif
+
 static inline void netif_addr_lock(struct net_device *dev)
 {
        unsigned char nest_level = 0;

diff --git a/net/core/dev.c b/net/core/dev.c
index 20610a192ec7a1d23257797056e8e6e3132bab1b..14a83f2035b9338850647798206dc257aefb8409 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4818,10 +4818,7 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
        if (dev->flags & IFF_UP) {
                int cpu = smp_processor_id(); /* ok because BHs are off */
 
-               /* Other cpus might concurrently change txq->xmit_lock_owner
-                * to -1 or to their cpu id, but not to our id.
-                */
-               if (READ_ONCE(txq->xmit_lock_owner) != cpu) {
+               if (!netif_tx_owned(txq, cpu)) {
                        bool is_list = false;
 
                        if (dev_xmit_recursion())

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index a8558a52884f9a9be7388748ddef0c6c38eda7f0..cd74beffd209ca7b6bd692aa43ca490534aa73f8 100644 (file)
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -132,7 +132,7 @@ static int netif_local_xmit_active(struct net_device *dev)
        for (i = 0; i < dev->num_tx_queues; i++) {
                struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
 
-               if (READ_ONCE(txq->xmit_lock_owner) == smp_processor_id())
+               if (netif_tx_owned(txq, smp_processor_id()))
                        return 1;
        }