From 9ed7de1afad3ebc08a3ce379bee0bbbbd9702414 Mon Sep 17 00:00:00 2001 From: Robert Lubos Date: Fri, 10 Nov 2023 10:23:34 +0100 Subject: [PATCH] [nrf fromlist] net: iface: Introduce TX mutex locking A recent iface lock removal in ed17320c3d2e43c76b09dc359d3b371e9d23732d exposed issues with concurrent access on TX to drivers that are not re-entrant. Reverting that commit does not really solve the problem, as it would still exist if multiple Traffic Class queues are in use. Therefore, introduce a separate mutex for TX data path, protecting the L2/driver from concurrent transfers from several threads. Upstream PR: https://github.com/zephyrproject-rtos/zephyr/pull/65049 Signed-off-by: Robert Lubos --- include/zephyr/net/net_if.h | 29 +++++++++++++++++++++++++++++ subsys/net/ip/net_if.c | 3 +++ subsys/net/l2/ethernet/arp.c | 2 ++ 3 files changed, 34 insertions(+) diff --git a/include/zephyr/net/net_if.h b/include/zephyr/net/net_if.h index b06bf74de03..92a59ad3b73 100644 --- a/include/zephyr/net/net_if.h +++ b/include/zephyr/net/net_if.h @@ -212,6 +212,9 @@ enum net_if_flag { /** IPv6 Multicast Listener Discovery disabled. */ NET_IF_IPV6_NO_MLD, + /** Mutex locking on TX data path disabled on the interface. */ + NET_IF_NO_TX_LOCK, + /** @cond INTERNAL_HIDDEN */ /* Total number of flags - must be at the end of the enum */ NET_IF_NUM_FLAGS @@ -613,6 +616,7 @@ struct net_if { #endif struct k_mutex lock; + struct k_mutex tx_lock; }; static inline void net_if_lock(struct net_if *iface) @@ -629,6 +633,31 @@ static inline void net_if_unlock(struct net_if *iface) k_mutex_unlock(&iface->lock); } +static inline bool net_if_flag_is_set(struct net_if *iface, + enum net_if_flag value); + +static inline void net_if_tx_lock(struct net_if *iface) +{ + NET_ASSERT(iface); + + if (net_if_flag_is_set(iface, NET_IF_NO_TX_LOCK)) { + return; + } + + (void)k_mutex_lock(&iface->tx_lock, K_FOREVER); +} + +static inline void net_if_tx_unlock(struct net_if *iface) +{ + NET_ASSERT(iface); + + if (net_if_flag_is_set(iface, NET_IF_NO_TX_LOCK)) { + return; + } + + k_mutex_unlock(&iface->tx_lock); +} + /** * @brief Set a value in network interface flags * diff --git a/subsys/net/ip/net_if.c b/subsys/net/ip/net_if.c index 035e18da74f..026eaa88b48 100644 --- a/subsys/net/ip/net_if.c +++ b/subsys/net/ip/net_if.c @@ -265,7 +265,9 @@ static bool net_if_tx(struct net_if *iface, struct net_pkt *pkt) } } + net_if_tx_lock(iface); status = net_if_l2(iface)->send(iface, pkt); + net_if_tx_unlock(iface); if (IS_ENABLED(CONFIG_NET_PKT_TXTIME_STATS)) { uint32_t end_tick = k_cycle_get_32(); @@ -437,6 +439,7 @@ static inline void init_iface(struct net_if *iface) #endif k_mutex_init(&iface->lock); + k_mutex_init(&iface->tx_lock); api->init(iface); } diff --git a/subsys/net/l2/ethernet/arp.c b/subsys/net/l2/ethernet/arp.c index 35a2ec63a64..39a06ed64a0 100644 --- a/subsys/net/l2/ethernet/arp.c +++ b/subsys/net/l2/ethernet/arp.c @@ -534,7 +534,9 @@ static void arp_update(struct net_if *iface, * the pkt are not counted twice and the packet filter * callbacks are only called once. */ + net_if_tx_lock(iface); ret = net_if_l2(iface)->send(iface, pkt); + net_if_tx_unlock(iface); if (ret < 0) { net_pkt_unref(pkt); }