From: Jakub Kicinski <kuba@kernel.org>
To: Breno Leitao <leitao@debian.org>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-team@meta.com, dw@davidwei.uk
Subject: Re: [PATCH net-next v2] netdevsim: implement peer queue flow control
Date: Tue, 8 Jul 2025 18:27:18 -0700 [thread overview]
Message-ID: <20250708182718.29c4ae45@kernel.org> (raw)
In-Reply-To: <20250703-netdev_flow_control-v2-1-ab00341c9cc1@debian.org>
On Thu, 03 Jul 2025 06:09:31 -0700 Breno Leitao wrote:
> +static int nsim_napi_rx(struct net_device *dev, struct nsim_rq *rq,
> + struct sk_buff *skb)
> {
> if (skb_queue_len(&rq->skb_queue) > NSIM_RING_SIZE) {
> + nsim_stop_peer_tx_queue(dev, rq, skb_get_queue_mapping(skb));
> dev_kfree_skb_any(skb);
> return NET_RX_DROP;
> }
we should probably add:
if (skb_queue_len(&rq->skb_queue) > NSIM_RING_SIZE)
nsim_stop_tx_queue(dev, rq, skb_get_queue_mapping(skb));
after enqueuing the skb, so that we stop the queue before any drops
happen
> @@ -51,7 +109,7 @@ static int nsim_napi_rx(struct nsim_rq *rq, struct sk_buff *skb)
> static int nsim_forward_skb(struct net_device *dev, struct sk_buff *skb,
> struct nsim_rq *rq)
> {
> - return __dev_forward_skb(dev, skb) ?: nsim_napi_rx(rq, skb);
> + return __dev_forward_skb(dev, skb) ?: nsim_napi_rx(dev, rq, skb);
> }
>
> static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
nsim_start_xmit() has both dev and peer_dev, pass them all the way to
nsim_stop_peer_tx_queue() so that you don't have to try to dereference
the peer again.
> + if (dev->real_num_tx_queues != peer_dev->num_rx_queues)
given that we compare real_num_tx_queues I think we should also kick
the queues in nsim_set_channels(), like we do in unlink_device_store()
--
pw-bot: cr
next prev parent reply other threads:[~2025-07-09 1:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-03 13:09 [PATCH net-next v2] netdevsim: implement peer queue flow control Breno Leitao
2025-07-09 1:27 ` Jakub Kicinski [this message]
2025-07-09 10:34 ` Breno Leitao
2025-07-09 21:36 ` Jakub Kicinski
2025-07-11 16:29 ` Breno Leitao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250708182718.29c4ae45@kernel.org \
--to=kuba@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=dw@davidwei.uk \
--cc=edumazet@google.com \
--cc=kernel-team@meta.com \
--cc=leitao@debian.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.