All of lore.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Shuah Khan <shuah@kernel.org>, Simon Horman <horms@kernel.org>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-kselftest@vger.kernel.org, bpf@vger.kernel.org,
	kernel-team@meta.com
Subject: Re: [PATCH net-next v4 3/3] selftests: net: add netpoll basic functionality test
Date: Tue, 8 Jul 2025 08:25:45 -0700	[thread overview]
Message-ID: <aG04ece5RWJCpMmA@gmail.com> (raw)
In-Reply-To: <686c88e0283_29b0d29422@willemb.c.googlers.com.notmuch>

Hello Willem,

On Mon, Jul 07, 2025 at 10:56:32PM -0400, Willem de Bruijn wrote:
> > +def test_netpoll(cfg: NetDrvEpEnv) -> None:

<snip>

> > +        bpftrace_stop()
> 
> One risk with stateful tests is that the state is not reset if the
> test exists (or crashes) before reaching the cleanup logic. There
> are ways around it. Jakub added defer for this purpose, for one.

Agree. Jakub suggested about "defer" a while ago, but I didn't realize
we had a "defer" helper in our test framework. For a second I though
Jakub was talking calling the stop later, my bad.

Now that you raised it, I found that we have "defer" as a helper, and we
definitely should use it. How about something as simple as:

	diff --git a/tools/testing/selftests/drivers/net/netpoll_basic.py b/tools/testing/selftests/drivers/net/netpoll_basic.py
	index 398ac959151b3..6017b71f154b2 100755
	--- a/tools/testing/selftests/drivers/net/netpoll_basic.py
	+++ b/tools/testing/selftests/drivers/net/netpoll_basic.py
	@@ -27,6 +27,7 @@ from typing import Optional

	from lib.py import (
	bpftrace,
	+    defer,
	ip,
	ethtool,
	GenerateTraffic,
	@@ -251,6 +252,7 @@ def do_netpoll_flush_monitored(cfg: NetDrvEpEnv, ifname: str, target_name: str)
	# Start bpftrace in parallel, so, it is watching
	# netpoll_poll_dev() while we are sending netconsole messages
	bpftrace_start()
	+    defer(bpftrace_stop)

	do_netpoll_flush(cfg, ifname, target_name)

	@@ -338,7 +340,6 @@ def test_netpoll(cfg: NetDrvEpEnv) -> None:
		# Revert RX/TX queues
		ethtool_set_rx_tx_queue(ifname, original_queues[0], original_queues[1])
		netcons_delete_target(target_name)

Thanks for th heads-up and review,
--breno

      reply	other threads:[~2025-07-08 15:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-02 11:20 [PATCH net-next v4 0/3] selftest: net: Add selftest for netpoll Breno Leitao
2025-07-02 11:21 ` [PATCH net-next v4 1/3] selftests: drv-net: add helper/wrapper for bpftrace Breno Leitao
2025-07-08 13:42   ` Paolo Abeni
2025-07-08 14:01     ` Breno Leitao
2025-07-02 11:21 ` [PATCH net-next v4 2/3] selftests: drv-net: Strip '@' prefix from bpftrace map keys Breno Leitao
2025-07-02 11:21 ` [PATCH net-next v4 3/3] selftests: net: add netpoll basic functionality test Breno Leitao
2025-07-08  2:56   ` Willem de Bruijn
2025-07-08 15:25     ` Breno Leitao [this message]

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=aG04ece5RWJCpMmA@gmail.com \
    --to=leitao@debian.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    --cc=willemdebruijn.kernel@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 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.