All of lore.kernel.org
 help / color / mirror / Atom feed
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>, Shuah Khan <shuah@kernel.org>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-kselftest@vger.kernel.org,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	gustavold@gmail.com
Subject: Re: [PATCH net-next] selftests: net: add netpoll basic functionality test
Date: Tue, 24 Jun 2025 09:38:24 -0700	[thread overview]
Message-ID: <20250624093824.4c0dd380@kernel.org> (raw)
In-Reply-To: <aFq1z0BS6RCUCNwa@gmail.com>

On Tue, 24 Jun 2025 15:27:27 +0100 Breno Leitao wrote:
> > > +    try:
> > > +        for key, value in config_data.items():
> > > +            if DEBUG:
> > > +                ksft_pr(f"Setting {key} to {value}")
> > > +            with open(
> > > +                f"{NETCONSOLE_CONFIGFS_PATH}/{target_name}/{key}",  
> > 
> > Could be personal preference but I think that using temp variable to
> > store the argument looks better than breaking out the function call
> > over 5 lines..  
> 
> I was not able to get what you mean here, sorry.
> 
> We have config_data, which is a dictionary that stores the netconsole
> keys (as in configfs) and their value, which will be set in the code below.
> 
> What would this temp variable look like, and how it would look like?

		path = f"{NETCONSOLE_CONFIGFS_PATH}/{target_name}/{key}"
		with open(path, "r", encoding="utf-8") as f:
			...

> > > +def test_netpoll(cfg: NetDrvEpEnv, netdevnl: NetdevFamily) -> None:
> > > +    """
> > > +    Test netpoll by sending traffic to the interface and then sending
> > > +    netconsole messages to trigger a poll
> > > +    """
> > > +
> > > +    target_name = generate_random_netcons_name()
> > > +    ifname = cfg.dev["ifname"]
> > > +    traffic = None
> > > +
> > > +    try:
> > > +        set_single_rx_tx_queue(ifname)
> > > +        traffic = GenerateTraffic(cfg)
> > > +        check_traffic_flowing(cfg, netdevnl)  
> > 
> > Any reason to perform this check? GenerateTraffic() already waits for
> > traffic to ramp up. Do we need to adjust the logic there, or make some
> > methods public?  
> 
> Not really. I can just remove this code, in fact, given
> GenerateTraffic() already waits for the code. Or, I can add under DEBUG.

Let's not put functional changes under DEBUG, just prints.
It could make it so that the test fails without DEBUG and passes with.

> As we discussed in the RFC thread, I will add support for bpftrace in
> the v2.

      reply	other threads:[~2025-06-24 16:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-20  9:38 [PATCH net-next] selftests: net: add netpoll basic functionality test Breno Leitao
2025-06-24  1:30 ` Jakub Kicinski
2025-06-24 14:27   ` Breno Leitao
2025-06-24 16:38     ` Jakub Kicinski [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=20250624093824.4c0dd380@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gustavold@gmail.com \
    --cc=leitao@debian.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.