netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: netdev@vger.kernel.org, makita.toshiaki@lab.ntt.co.jp,
	Eric Dumazet <eric.dumazet@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	ihor.solodrai@linux.dev, toshiaki.makita1@gmail.com,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@cloudflare.com
Subject: Re: [PATCH net V1 2/3] veth: stop and start all TX queue in netdev down/up
Date: Fri, 24 Oct 2025 17:54:02 -0700	[thread overview]
Message-ID: <20251024175402.397c05a9@kernel.org> (raw)
In-Reply-To: <176123157775.2281302.5972243809904783041.stgit@firesoul>

On Thu, 23 Oct 2025 16:59:37 +0200 Jesper Dangaard Brouer wrote:
> The veth driver started manipulating TXQ states in commit
> dc82a33297fc ("veth: apply qdisc backpressure on full ptr_ring
> to reduce TX drops").
> 
> Other drivers manipulating TXQ states takes care of stopping
> and starting TXQs in NDOs.  Thus, adding this to veth .ndo_open
> and .ndo_stop.

Kinda, but taking a device up or down resets the qdisc, IIRC.

So stopping the qdisc for real drivers is mostly a way to make sure
that there's nothing entering the xmit handler as the driver dismantles
its state.

I'm not sure if this is an official rule, but I'm under the impression
that stopping the queues or carrier loss (and
netif_tx_stop_all_queues(peer) in close() is stopping peer's Tx queue
on carrier loss) is inadvisable as it may lead to old packets getting
transmitted when carrier comes back.

IOW based on the commit msg - I'm not sure this patch is needed..

  reply	other threads:[~2025-10-25  0:54 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-23 14:59 [PATCH net V1 0/3] veth: Fix TXQ stall race condition and add recovery Jesper Dangaard Brouer
2025-10-23 14:59 ` [PATCH net V1 1/3] veth: enable dev_watchdog for detecting stalled TXQs Jesper Dangaard Brouer
2025-10-24 13:39   ` Toke Høiland-Jørgensen
2025-10-27 11:41     ` Jesper Dangaard Brouer
2025-10-27 14:09       ` Toke Høiland-Jørgensen
2025-10-27 16:18         ` Jesper Dangaard Brouer
2025-10-23 14:59 ` [PATCH net V1 2/3] veth: stop and start all TX queue in netdev down/up Jesper Dangaard Brouer
2025-10-25  0:54   ` Jakub Kicinski [this message]
2025-10-27 10:33     ` Jesper Dangaard Brouer
2025-10-23 14:59 ` [PATCH net V1 3/3] veth: more robust handing of race to avoid txq getting stuck Jesper Dangaard Brouer
2025-10-24 14:33   ` Toke Høiland-Jørgensen
2025-10-27 12:19     ` Jesper Dangaard Brouer
2025-10-27 14:12       ` Toke Høiland-Jørgensen
2025-10-27 19:22   ` Jesper Dangaard Brouer

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=20251024175402.397c05a9@kernel.org \
    --to=kuba@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=hawk@kernel.org \
    --cc=ihor.solodrai@linux.dev \
    --cc=kernel-team@cloudflare.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=makita.toshiaki@lab.ntt.co.jp \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=toshiaki.makita1@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).