From patchwork Thu Apr 17 13:55:09 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jesper Dangaard Brouer X-Patchwork-Id: 14055623 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 09EF524EF72; Thu, 17 Apr 2025 13:55:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744898114; cv=none; b=OuGzjqdyOx13L8Gly/fh7jBf0mQroAuH28rubCpB+PRNgl1RmUZlVd4E8kY4MSDfsDRowPMsWHYYNZrFJP+xlbMHy9R+fah/2JL1y4UqBoQJhtz0RLtsQ/CiEhPfWK1Mbpd2wvJ1wc+ONoUxWWufMSu7yzgk03vJQ/sGNv8PdG4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744898114; c=relaxed/simple; bh=DaXbVItV4vEdJ1PxdCzd1lAzo6REI3kB5/yRt2cUJ1Q=; h=Subject:From:To:Cc:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=nT9uY2mT999W9sjbIjahKVvTEAldvgItDxgs8mCaKdaaRIu7FfnqhA56mO/RZoXJ9XJrViJQm8zQkJcW6tlrs4qkhlIb4Wddho1XGEvkm7nSLn7nziqGuPyCuqw5BY5mGtIy0U7nWIsamazJKZTV29C1cJrYyVWmzAisioIes/I= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lTrZinNx; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="lTrZinNx" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5A5FDC4CEEA; Thu, 17 Apr 2025 13:55:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744898113; bh=DaXbVItV4vEdJ1PxdCzd1lAzo6REI3kB5/yRt2cUJ1Q=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=lTrZinNxVD0pK3FP/9ZXEsvueppmmYKT6Axqz6Was9MxSmrYrK/6X/pUMOPbv2jO0 4A+WSRfX9ZhvZbHLinkWHch6E14ByOmz6KsI/uh8IEPWyGz/wS7/dwcyZFHzf1aMbc RYsfiDXU/K52ECeYWYvbSbRYO5vtzXLXoGxLq7cmWhj/vANDSIBrsLVFTBXTCJUVvx OaBKnY1jANSBoH+Zo3Vxpbb1t0o/ESBfBVv/PRXtYALSTJcCqhIMg0tC1Zn+Df/l2k Is4mcjMkvjyp654UAwb2C0I7WXTuWOwmoCkMeyW//k7w4OisjcolTQ2xhx1Ezk6xkB EaXIOhJglxpmA== Subject: [PATCH net-next V5 1/2] net: sched: generalize check for no-queue qdisc on TX queue From: Jesper Dangaard Brouer To: netdev@vger.kernel.org, Jakub Kicinski Cc: Jesper Dangaard Brouer , bpf@vger.kernel.org, tom@herbertland.com, Eric Dumazet , "David S. Miller" , Paolo Abeni , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , dsahern@kernel.org, makita.toshiaki@lab.ntt.co.jp, kernel-team@cloudflare.com, phil@nwl.cc Date: Thu, 17 Apr 2025 15:55:09 +0200 Message-ID: <174489810897.355490.12073714761899580608.stgit@firesoul> In-Reply-To: <174489803410.355490.13216831426556849084.stgit@firesoul> References: <174489803410.355490.13216831426556849084.stgit@firesoul> User-Agent: StGit/1.5 Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org The "noqueue" qdisc can either be directly attached, or get default attached if net_device priv_flags has IFF_NO_QUEUE. In both cases, the allocated Qdisc structure gets it's enqueue function pointer reset to NULL by noqueue_init() via noqueue_qdisc_ops. This is a common case for software virtual net_devices. For these devices with no-queue, the transmission path in __dev_queue_xmit() will bypass the qdisc layer. Directly invoking device drivers ndo_start_xmit (via dev_hard_start_xmit). In this mode the device driver is not allowed to ask for packets to be queued (either via returning NETDEV_TX_BUSY or stopping the TXQ). The simplest and most reliable way to identify this no-queue case is by checking if enqueue == NULL. The vrf driver currently open-codes this check (!qdisc->enqueue). While functionally correct, this low-level detail is better encapsulated in a dedicated helper for clarity and long-term maintainability. To make this behavior more explicit and reusable, this patch introduce a new helper: qdisc_txq_has_no_queue(). Helper will also be used by the veth driver in the next patch, which introduces optional qdisc-based backpressure. This is a non-functional change. Reviewed-by: David Ahern Reviewed-by: Toke Høiland-Jørgensen Signed-off-by: Jesper Dangaard Brouer --- drivers/net/vrf.c | 4 +--- include/net/sch_generic.h | 8 ++++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c index 7168b33adadb..9a4beea6ee0c 100644 --- a/drivers/net/vrf.c +++ b/drivers/net/vrf.c @@ -343,15 +343,13 @@ static int vrf_ifindex_lookup_by_table_id(struct net *net, u32 table_id) static bool qdisc_tx_is_default(const struct net_device *dev) { struct netdev_queue *txq; - struct Qdisc *qdisc; if (dev->num_tx_queues > 1) return false; txq = netdev_get_tx_queue(dev, 0); - qdisc = rcu_access_pointer(txq->qdisc); - return !qdisc->enqueue; + return qdisc_txq_has_no_queue(txq); } /* Local traffic destined to local address. Reinsert the packet to rx diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index d48c657191cd..b6c177f7141c 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -803,6 +803,14 @@ static inline bool qdisc_tx_changing(const struct net_device *dev) return false; } +/* "noqueue" qdisc identified by not having any enqueue, see noqueue_init() */ +static inline bool qdisc_txq_has_no_queue(const struct netdev_queue *txq) +{ + struct Qdisc *qdisc = rcu_access_pointer(txq->qdisc); + + return qdisc->enqueue == NULL; +} + /* Is the device using the noop qdisc on all queues? */ static inline bool qdisc_tx_is_noop(const struct net_device *dev) { From patchwork Thu Apr 17 13:55:15 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jesper Dangaard Brouer X-Patchwork-Id: 14055624 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4496D250C05; Thu, 17 Apr 2025 13:55:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744898120; cv=none; b=s+4f4tEPYnnMLdd3x8ctu0Z0p6O6B2/fCcgwuLtSp4qC4nabjV+/IbWra60RmginvsHxpviOpIY22Ztq9ukUi2UzbBUhYzIlybGqo/TsV0FLjvc2MOlQj8Dix6Cx2aCYtp3Cd3+l29GuFIkGKpXzRqAx3i9iHbkjPU6NAiZBssQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744898120; c=relaxed/simple; bh=UZKqP3+llQ3QOmeT2lY8PQ4zeXE9rf8nCh9ro5bpbvM=; h=Subject:From:To:Cc:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=F0KiiFtrf18jLm9so1Hy9jPmz62RAa34ywqpFg6dODpu9eSMUq/4cEctegBt0wdZGBvzCb3zMDQ7L+lXJ9yI2SaUFYV0ppbPCqxDdGNJYhKXjmbMKYLZzx2ATNO+DNLc8hucyP+S5NbhiHbx0pA8zU2AgXzLDwaU5Jd4l9uqSN0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=AS8S5WRt; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="AS8S5WRt" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 53774C4CEE4; Thu, 17 Apr 2025 13:55:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1744898119; bh=UZKqP3+llQ3QOmeT2lY8PQ4zeXE9rf8nCh9ro5bpbvM=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=AS8S5WRta1v0mmBnURA1c2CaZ8hWnrGT5VBjw1PPq5hQanBTjr/oVS9ZV3pH11YzD D4iOpaKImuBnrX6/PpbuNsrr4VCeU+4vtbIVSZU/WZU/nDWfP28SKaJTkcEWGGCUes QTUefpV2qqgkVJz0wfMOPEq6LKlNlUxrGkOsanVeFaOlxQH1waq1be+GRB+vLTnmx3 PTwaJn27X7jl/7+u/T4woUZpQFBmlhUSk+2UrEl/2xnU7w26IpZLo/WnGEGmcGJMWX u7CtzOQ66MDh2Kmn+ssZ1SqmSdTgnqoBQThKXr3GxJCFiOj5PVfNuJCjAyZ+HlEs/C Vt4DNitGO31yg== Subject: [PATCH net-next V5 2/2] veth: apply qdisc backpressure on full ptr_ring to reduce TX drops From: Jesper Dangaard Brouer To: netdev@vger.kernel.org, Jakub Kicinski Cc: Jesper Dangaard Brouer , bpf@vger.kernel.org, tom@herbertland.com, Eric Dumazet , "David S. Miller" , Paolo Abeni , =?utf-8?q?Toke_H=C3=B8iland-J=C3=B8rgensen?= , dsahern@kernel.org, makita.toshiaki@lab.ntt.co.jp, kernel-team@cloudflare.com, phil@nwl.cc Date: Thu, 17 Apr 2025 15:55:15 +0200 Message-ID: <174489811513.355490.8155513147018728621.stgit@firesoul> In-Reply-To: <174489803410.355490.13216831426556849084.stgit@firesoul> References: <174489803410.355490.13216831426556849084.stgit@firesoul> User-Agent: StGit/1.5 Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org In production, we're seeing TX drops on veth devices when the ptr_ring fills up. This can occur when NAPI mode is enabled, though it's relatively rare. However, with threaded NAPI - which we use in production - the drops become significantly more frequent. The underlying issue is that with threaded NAPI, the consumer often runs on a different CPU than the producer. This increases the likelihood of the ring filling up before the consumer gets scheduled, especially under load, leading to drops in veth_xmit() (ndo_start_xmit()). This patch introduces backpressure by returning NETDEV_TX_BUSY when the ring is full, signaling the qdisc layer to requeue the packet. The txq (netdev queue) is stopped in this condition and restarted once veth_poll() drains entries from the ring, ensuring coordination between NAPI and qdisc. Backpressure is only enabled when a qdisc is attached. Without a qdisc, the driver retains its original behavior - dropping packets immediately when the ring is full. This avoids unexpected behavior changes in setups without a configured qdisc. With a qdisc in place (e.g. fq, sfq) this allows Active Queue Management (AQM) to fairly schedule packets across flows and reduce collateral damage from elephant flows. A known limitation of this approach is that the full ring sits in front of the qdisc layer, effectively forming a FIFO buffer that introduces base latency. While AQM still improves fairness and mitigates flow dominance, the latency impact is measurable. In hardware drivers, this issue is typically addressed using BQL (Byte Queue Limits), which tracks in-flight bytes needed based on physical link rate. However, for virtual drivers like veth, there is no fixed bandwidth constraint - the bottleneck is CPU availability and the scheduler's ability to run the NAPI thread. It is unclear how effective BQL would be in this context. This patch serves as a first step toward addressing TX drops. Future work may explore adapting a BQL-like mechanism to better suit virtual devices like veth. Reported-by: Yan Zhai Signed-off-by: Jesper Dangaard Brouer --- drivers/net/veth.c | 55 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/drivers/net/veth.c b/drivers/net/veth.c index 7bb53961c0ea..55be225c4e20 100644 --- a/drivers/net/veth.c +++ b/drivers/net/veth.c @@ -307,12 +307,10 @@ static void __veth_xdp_flush(struct veth_rq *rq) static int veth_xdp_rx(struct veth_rq *rq, struct sk_buff *skb) { - if (unlikely(ptr_ring_produce(&rq->xdp_ring, skb))) { - dev_kfree_skb_any(skb); - return NET_RX_DROP; - } + if (unlikely(ptr_ring_produce(&rq->xdp_ring, skb))) + return NETDEV_TX_BUSY; /* signal qdisc layer */ - return NET_RX_SUCCESS; + return NET_RX_SUCCESS; /* same as NETDEV_TX_OK */ } static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb, @@ -346,11 +344,11 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev) { struct veth_priv *rcv_priv, *priv = netdev_priv(dev); struct veth_rq *rq = NULL; - int ret = NETDEV_TX_OK; + struct netdev_queue *txq; struct net_device *rcv; int length = skb->len; bool use_napi = false; - int rxq; + int ret, rxq; rcu_read_lock(); rcv = rcu_dereference(priv->peer); @@ -373,17 +371,43 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev) } skb_tx_timestamp(skb); - if (likely(veth_forward_skb(rcv, skb, rq, use_napi) == NET_RX_SUCCESS)) { + + ret = veth_forward_skb(rcv, skb, rq, use_napi); + switch (ret) { + case NET_RX_SUCCESS: /* same as NETDEV_TX_OK */ if (!use_napi) dev_sw_netstats_tx_add(dev, 1, length); else __veth_xdp_flush(rq); - } else { + break; + case NETDEV_TX_BUSY: + /* If a qdisc is attached to our virtual device, returning + * NETDEV_TX_BUSY is allowed. + */ + txq = netdev_get_tx_queue(dev, rxq); + + if (qdisc_txq_has_no_queue(txq)) { + dev_kfree_skb_any(skb); + goto drop; + } + netif_tx_stop_queue(txq); + /* Restore Eth hdr pulled by dev_forward_skb/eth_type_trans */ + __skb_push(skb, ETH_HLEN); + if (use_napi) + __veth_xdp_flush(rq); + /* Cancel TXQ stop for very unlikely race */ + if (unlikely(__ptr_ring_empty(&rq->xdp_ring))) + netif_tx_wake_queue(txq); + break; + case NET_RX_DROP: /* same as NET_XMIT_DROP */ drop: atomic64_inc(&priv->dropped); ret = NET_XMIT_DROP; + break; + default: + net_crit_ratelimited("%s(%s): Invalid return code(%d)", + __func__, dev->name, ret); } - rcu_read_unlock(); return ret; @@ -874,9 +898,17 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget, struct veth_xdp_tx_bq *bq, struct veth_stats *stats) { + struct veth_priv *priv = netdev_priv(rq->dev); + int queue_idx = rq->xdp_rxq.queue_index; + struct netdev_queue *peer_txq; + struct net_device *peer_dev; int i, done = 0, n_xdpf = 0; void *xdpf[VETH_XDP_BATCH]; + /* NAPI functions as RCU section */ + peer_dev = rcu_dereference_check(priv->peer, rcu_read_lock_bh_held()); + peer_txq = netdev_get_tx_queue(peer_dev, queue_idx); + for (i = 0; i < budget; i++) { void *ptr = __ptr_ring_consume(&rq->xdp_ring); @@ -925,6 +957,9 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget, rq->stats.vs.xdp_packets += done; u64_stats_update_end(&rq->stats.syncp); + if (unlikely(netif_tx_queue_stopped(peer_txq))) + netif_tx_wake_queue(peer_txq); + return done; }