linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll
@ 2025-06-25 11:39 Breno Leitao
  2025-06-25 11:39 ` [PATCH net-next v2 1/4] selftests: drv-net: add helper/wrapper for bpftrace Breno Leitao
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Breno Leitao @ 2025-06-25 11:39 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman
  Cc: linux-kernel, netdev, linux-kselftest, Willem de Bruijn, bpf,
	gustavold, Breno Leitao

I am submitting a new selftest for the netpoll subsystem specifically
targeting the case where the RX is polling in the TX path, which is
a case that we don't have any test in the tree today. This is done when
netpoll_poll_dev() called, and this test creates a scenario when that is
probably.

The test does the following:

1) Configuring a single RX/TX queue to increase contention on the
   interface.
2) Generating background traffic to saturate the network, mimicking
   real-world congestion.
3) Sending netconsole messages to trigger netpoll polling and monitor
   its behavior.
4) Using dynamic netconsole targets via configfs, with the ability to
   delete and recreate targets during the test.
5) Running bpftrace in parallel to verify that netpoll_poll_dev() is
   called when expected. If it is called, then the test passes,
   otherwise the test is marked as skipped.

In order to achieve it, I stole Jakub's bpftrace helper from [1], and
did some small changes that I found useful to use the helper.

So, this patchset basically contains:

1) The code stolen from Jakub
2) Improvements on bpftrace() helper
3) The selftest itself

Link: https://lore.kernel.org/all/20250421222827.283737-22-kuba@kernel.org/ [1]

---
Changes in v1 (from RFC):
- Toggle the netconsole interfaces up and down after 5 iterations.
- Moved the traffic check under DEBUG (Willem de Bruijn).
- Bumped the iterations to 20 given it runs faster now.
- Link to the RFC: https://lore.kernel.org/r/20250612-netpoll_test-v1-1-4774fd95933f@debian.org

---
Changes in v2:
- Stole Jakub's helper to run bpftrace
- Removed the DEBUG option and moved logs to logging
- Change the code to have a higher chance of calling netpoll_poll_dev().
  In my current configuration, it is hitting multiple times during the
  test.
- Save and restore TX/RX queue size (Jakub)
- Link to v1: https://lore.kernel.org/r/20250620-netpoll_test-v1-1-5068832f72fc@debian.org

---
Breno Leitao (3):
      selftests: drv-net: Improve bpftrace utility error handling
      selftests: drv-net: Strip '@' prefix from bpftrace map keys
      selftests: net: add netpoll basic functionality test

Jakub Kicinski (1):
      selftests: drv-net: add helper/wrapper for bpftrace

 tools/testing/selftests/drivers/net/Makefile       |   1 +
 .../testing/selftests/drivers/net/netpoll_basic.py | 344 +++++++++++++++++++++
 tools/testing/selftests/net/lib/py/utils.py        |  38 +++
 3 files changed, 383 insertions(+)
---
base-commit: eb4c27edb4d8dbfbdcc7bc03e0394a0fab8af7d5
change-id: 20250612-netpoll_test-a1324d2057c8

Best regards,
--  
Breno Leitao <leitao@debian.org>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH net-next v2 1/4] selftests: drv-net: add helper/wrapper for bpftrace
  2025-06-25 11:39 [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Breno Leitao
@ 2025-06-25 11:39 ` Breno Leitao
  2025-06-25 22:12   ` Jakub Kicinski
  2025-06-25 11:39 ` [PATCH net-next v2 2/4] selftests: drv-net: Improve bpftrace utility error handling Breno Leitao
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Breno Leitao @ 2025-06-25 11:39 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman
  Cc: linux-kernel, netdev, linux-kselftest, Willem de Bruijn, bpf,
	gustavold, Breno Leitao

From: Jakub Kicinski <kuba@kernel.org>

bpftrace is very useful for low level driver testing. perf or trace-cmd
would also do for collecting data from tracepoints, but they require
much more post-processing.

Add a wrapper for running bpftrace and sanitizing its output.
bpftrace has JSON output, which is great, but it prints loose objects
and in a slightly inconvenient format. We have to read the objects
line by line, and while at it return them indexed by the map name.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/net/lib/py/utils.py | 33 +++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 34470d65d871a..760ccf6fccccc 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -185,6 +185,39 @@ def ethtool(args, json=None, ns=None, host=None):
     return tool('ethtool', args, json=json, ns=ns, host=host)
 
 
+def bpftrace(expr, json=None, ns=None, host=None, timeout=None):
+    """
+    Run bpftrace and return map data (if json=True).
+    The output of bpftrace is inconvenient, so the helper converts
+    to a dict indexed by map name, e.g.:
+     {
+       "@":     { ... },
+       "@map2": { ... },
+     }
+    """
+    cmd_arr = ['bpftrace']
+    # Throw in --quiet if json, otherwise the output has two objects
+    if json:
+        cmd_arr += ['-f', 'json', '-q']
+    if timeout:
+        expr += ' interval:s:' + str(timeout) + ' { exit(); }'
+    cmd_arr += ['-e', expr]
+    cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False)
+    if json:
+        # bpftrace prints objects as lines
+        ret = {}
+        for l in cmd_obj.stdout.split('\n'):
+            if not l.strip():
+                continue
+            one = _json.loads(l)
+            if one.get('type') != 'map':
+                continue
+            for k, v in one["data"].items():
+                ret[k] = v
+        return ret
+    return cmd_obj
+
+
 def rand_port(type=socket.SOCK_STREAM):
     """
     Get a random unprivileged port.

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next v2 2/4] selftests: drv-net: Improve bpftrace utility error handling
  2025-06-25 11:39 [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Breno Leitao
  2025-06-25 11:39 ` [PATCH net-next v2 1/4] selftests: drv-net: add helper/wrapper for bpftrace Breno Leitao
@ 2025-06-25 11:39 ` Breno Leitao
  2025-06-25 21:48   ` Jakub Kicinski
  2025-06-25 11:39 ` [PATCH net-next v2 3/4] selftests: drv-net: Strip '@' prefix from bpftrace map keys Breno Leitao
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Breno Leitao @ 2025-06-25 11:39 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman
  Cc: linux-kernel, netdev, linux-kselftest, Willem de Bruijn, bpf,
	gustavold, Breno Leitao

Enhance the bpftrace function to raise an exception if the underlying
bpftrace command returns a non-zero exit code. This helps detect and
surface errors from bpftrace execution in test scripts, improving
robustness and debugging capabilities.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/net/lib/py/utils.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 760ccf6fccccc..c4e26567ee6fb 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -203,6 +203,9 @@ def bpftrace(expr, json=None, ns=None, host=None, timeout=None):
         expr += ' interval:s:' + str(timeout) + ' { exit(); }'
     cmd_arr += ['-e', expr]
     cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False)
+    if cmd_obj.ret != 0:
+        raise Exception("Warning: bpftrace command returned a non-zero exit code.")
+
     if json:
         # bpftrace prints objects as lines
         ret = {}

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next v2 3/4] selftests: drv-net: Strip '@' prefix from bpftrace map keys
  2025-06-25 11:39 [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Breno Leitao
  2025-06-25 11:39 ` [PATCH net-next v2 1/4] selftests: drv-net: add helper/wrapper for bpftrace Breno Leitao
  2025-06-25 11:39 ` [PATCH net-next v2 2/4] selftests: drv-net: Improve bpftrace utility error handling Breno Leitao
@ 2025-06-25 11:39 ` Breno Leitao
  2025-06-25 22:07   ` Jakub Kicinski
  2025-06-25 11:39 ` [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test Breno Leitao
  2025-06-25 18:43 ` [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Simon Horman
  4 siblings, 1 reply; 19+ messages in thread
From: Breno Leitao @ 2025-06-25 11:39 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman
  Cc: linux-kernel, netdev, linux-kselftest, Willem de Bruijn, bpf,
	gustavold, Breno Leitao

The '@' prefix in bpftrace map keys is specific to bpftrace and can be
safely removed when processing results. This patch modifies the bpftrace
utility to strip the '@' from map keys before storing them in the result
dictionary, making the keys more consistent with Python conventions.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/net/lib/py/utils.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index c4e26567ee6fb..31d4d63621c5f 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -216,6 +216,8 @@ def bpftrace(expr, json=None, ns=None, host=None, timeout=None):
             if one.get('type') != 'map':
                 continue
             for k, v in one["data"].items():
+                if k.startswith('@'):
+                    k = k.lstrip('@')
                 ret[k] = v
         return ret
     return cmd_obj

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test
  2025-06-25 11:39 [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Breno Leitao
                   ` (2 preceding siblings ...)
  2025-06-25 11:39 ` [PATCH net-next v2 3/4] selftests: drv-net: Strip '@' prefix from bpftrace map keys Breno Leitao
@ 2025-06-25 11:39 ` Breno Leitao
  2025-06-25 22:09   ` Jakub Kicinski
                     ` (2 more replies)
  2025-06-25 18:43 ` [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Simon Horman
  4 siblings, 3 replies; 19+ messages in thread
From: Breno Leitao @ 2025-06-25 11:39 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman
  Cc: linux-kernel, netdev, linux-kselftest, Willem de Bruijn, bpf,
	gustavold, Breno Leitao

Add a basic selftest for the netpoll polling mechanism, specifically
targeting the netpoll poll() side.

The test creates a scenario where network transmission is running at
maximum speed, and netpoll needs to poll the NIC. This is achieved by:

  1. Configuring a single RX/TX queue to create contention
  2. Generating background traffic to saturate the interface
  3. Sending netconsole messages to trigger netpoll polling
  4. Using dynamic netconsole targets via configfs
  5. Delete and create new netconsole targets after some messages
  6. Start a bpftrace in parallel to make sure netpoll_poll_dev() is
     called
  7. If bpftrace exists and netpoll_poll_dev() was called, stop.

The test validates a critical netpoll code path by monitoring traffic
flow and ensuring netpoll_poll_dev() is called when the normal TX path
is blocked.

This addresses a gap in netpoll test coverage for a path that is
tricky for the network stack.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/drivers/net/Makefile       |   1 +
 .../testing/selftests/drivers/net/netpoll_basic.py | 344 +++++++++++++++++++++
 2 files changed, 345 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index bd309b2d39095..9bd84d6b542e5 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -16,6 +16,7 @@ TEST_PROGS := \
 	netcons_fragmented_msg.sh \
 	netcons_overflow.sh \
 	netcons_sysdata.sh \
+	netpoll_basic.py \
 	ping.py \
 	queues.py \
 	stats.py \
diff --git a/tools/testing/selftests/drivers/net/netpoll_basic.py b/tools/testing/selftests/drivers/net/netpoll_basic.py
new file mode 100755
index 0000000000000..dfb52d6793466
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netpoll_basic.py
@@ -0,0 +1,344 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+# Author: Breno Leitao <leitao@debian.org>
+"""
+ This test aims to evaluate the netpoll polling mechanism (as in
+ netpoll_poll_dev()). It presents a complex scenario where the network
+ attempts to send a packet but fails, prompting it to poll the NIC from within
+ the netpoll TX side.
+
+ This has been a crucial path in netpoll that was previously untested. Jakub
+ suggested using a single RX/TX queue, pushing traffic to the NIC, and then
+ sending netpoll messages (via netconsole) to trigger the poll.
+
+ In parallel, bpftrace is used to detect if netpoll_poll_dev() was called. If
+ so, the test passes, otherwise it will be skipped. This test is very dependent on
+ the driver and environment, given we are trying to trigger a tricky scenario.
+"""
+
+import errno
+import logging
+import os
+import random
+import string
+import threading
+import time
+
+from lib.py import (
+    bpftrace,
+    ethtool,
+    GenerateTraffic,
+    ksft_exit,
+    ksft_pr,
+    ksft_run,
+    KsftFailEx,
+    KsftSkipEx,
+    NetdevFamily,
+    NetDrvEpEnv,
+)
+
+# Configure logging
+logging.basicConfig(
+    level=logging.INFO,
+    format="%(asctime)s - %(levelname)s - %(message)s",
+)
+
+NETCONSOLE_CONFIGFS_PATH: str = "/sys/kernel/config/netconsole"
+NETCONS_REMOTE_PORT: int = 6666
+NETCONS_LOCAL_PORT: int = 1514
+# Max number of netcons messages to send. Each iteration will setup
+# netconsole and send 10 messages
+ITERATIONS: int = 20
+# MAPS contains the information coming from bpftrace
+# it will have only one key: @hits, which tells the number of times
+# netpoll_poll_dev() was called
+MAPS: dict[str, int] = {}
+# Thread to run bpftrace in parallel
+BPF_THREAD: threading.Thread = None
+# Time bpftrace will be running in parallel.
+BPFTRACE_TIMEOUT: int = 15
+
+
+def ethtool_read_rx_tx_queue(interface_name: str) -> tuple[int, int]:
+    """
+    Read the number of RX and TX queues using ethtool. This will be used
+    to restore it after the test
+    """
+    try:
+        ethtool_result = ethtool(f"-g {interface_name}").stdout
+        for line in ethtool_result.splitlines():
+            if line.startswith("RX:"):
+                rx_queue = int(line.split()[1])
+            if line.startswith("TX:"):
+                tx_queue = int(line.split()[1])
+    except IndexError as exception:
+        raise KsftSkipEx(
+            f"Failed to read RX/TX queues numbers: {exception}. Not going to mess with them."
+        ) from exception
+
+    return rx_queue, tx_queue
+
+
+def ethtool_set_rx_tx_queue(interface_name: str, rx_val: int, tx_val: int) -> None:
+    """Set the number of RX and TX queues to 1 using ethtool"""
+    try:
+        # This don't need to be reverted, since interfaces will be deleted after test
+        ethtool(f"-G {interface_name} rx {rx_val} tx {tx_val}")
+    except Exception as exception:
+        raise KsftSkipEx(
+            f"Failed to configure RX/TX queues: {exception}. Ethtool not available?"
+        ) from exception
+
+
+def netcons_generate_random_target_name() -> str:
+    """Generate a random target name starting with 'netcons'"""
+    random_suffix = "".join(random.choices(string.ascii_lowercase + string.digits, k=8))
+    return f"netcons_{random_suffix}"
+
+
+def netcons_create_target(
+    config_data: dict[str, str],
+    target_name: str,
+) -> None:
+    """Create a netconsole dynamic target against the interfaces"""
+    logging.debug("Using netconsole name: %s", target_name)
+    try:
+        os.makedirs(f"{NETCONSOLE_CONFIGFS_PATH}/{target_name}", exist_ok=True)
+        logging.debug(
+            "Created target directory: %s/%s", NETCONSOLE_CONFIGFS_PATH, target_name
+        )
+    except OSError as exception:
+        if exception.errno != errno.EEXIST:
+            raise KsftFailEx(
+                f"Failed to create netconsole target directory: {exception}"
+            ) from exception
+
+    try:
+        for key, value in config_data.items():
+            path = f"{NETCONSOLE_CONFIGFS_PATH}/{target_name}/{key}"
+            logging.debug("Writing %s to %s", key, path)
+            with open(path, "w", encoding="utf-8") as file:
+                # Always convert to string to write to file
+                file.write(str(value))
+                file.close()
+
+        # Read all configuration values for debugging purposes
+        for debug_key in config_data.keys():
+            with open(
+                f"{NETCONSOLE_CONFIGFS_PATH}/{target_name}/{debug_key}",
+                "r",
+                encoding="utf-8",
+            ) as file:
+                content = file.read()
+                logging.debug(
+                    "%s/%s/%s : %s",
+                    NETCONSOLE_CONFIGFS_PATH,
+                    target_name,
+                    debug_key,
+                    content,
+                )
+
+    except Exception as exception:
+        raise KsftFailEx(
+            f"Failed to configure netconsole target: {exception}"
+        ) from exception
+
+
+def netcons_configure_target(
+    cfg: NetDrvEpEnv, interface_name: str, target_name: str
+) -> None:
+    """Configure netconsole on the interface with the given target name"""
+    config_data = {
+        "extended": "1",
+        "dev_name": interface_name,
+        "local_port": NETCONS_LOCAL_PORT,
+        "remote_port": NETCONS_REMOTE_PORT,
+        "local_ip": cfg.addr_v["4"] if cfg.addr_ipver == "4" else cfg.addr_v["6"],
+        "remote_ip": (
+            cfg.remote_addr_v["4"] if cfg.addr_ipver == "4" else cfg.remote_addr_v["6"]
+        ),
+        "remote_mac": "00:00:00:00:00:00",  # Not important for this test
+        "enabled": "1",
+    }
+
+    netcons_create_target(config_data, target_name)
+    logging.debug(
+        "Created netconsole target: %s on interface %s", target_name, interface_name
+    )
+
+
+def netcons_delete_target(name: str) -> None:
+    """Delete a netconsole dynamic target"""
+    target_path = f"{NETCONSOLE_CONFIGFS_PATH}/{name}"
+    try:
+        if os.path.exists(target_path):
+            os.rmdir(target_path)
+    except OSError as exception:
+        raise KsftFailEx(
+            f"Failed to delete netconsole target: {exception}"
+        ) from exception
+
+
+def netcons_load_module() -> None:
+    """Try to load the netconsole module"""
+    os.system("modprobe netconsole")
+
+
+def bpftrace_call() -> None:
+    """Call bpftrace to find how many times netpoll_poll_dev() is called.
+    Output is saved in the global variable `maps`"""
+
+    # This is going to update the global variable, that will be seen by the
+    # main function
+    global MAPS  # pylint: disable=W0603
+
+    # This will be passed to bpftrace as in bpftrace -e "expr"
+    expr = "BEGIN{ @hits = 0;} kprobe:netpoll_poll_dev { @hits += 1; }"
+
+    MAPS = bpftrace(expr, timeout=BPFTRACE_TIMEOUT, json=True)
+    logging.debug("BPFtrace output: %s", MAPS)
+
+
+def bpftrace_start():
+    """Start a thread to call `call_bpf` in parallel for 2 seconds."""
+    global BPF_THREAD  # pylint: disable=W0603
+
+    BPF_THREAD = threading.Thread(target=bpftrace_call)
+    BPF_THREAD.start()
+    if not BPF_THREAD.is_alive():
+        raise KsftSkipEx("BPFtrace thread is not alive. Skipping test")
+
+
+def bpftrace_stop() -> None:
+    """Stop the bpftrace thread"""
+    if BPF_THREAD:
+        BPF_THREAD.join()
+
+
+def bpftrace_any_hit(join: bool) -> bool:
+    """Check if netpoll_poll_dev() was called by checking the global variable `maps`"""
+    if BPF_THREAD.is_alive():
+        if join:
+            # Wait for bpftrace to finish
+            BPF_THREAD.join()
+        else:
+            # bpftrace is still running, so, we will not check the result yet
+            return False
+
+    logging.debug("MAPS coming from bpftrace = %s", MAPS)
+    if "hits" not in MAPS.keys():
+        raise KsftFailEx(f"bpftrace failed to run!?: {MAPS}")
+
+    return MAPS["hits"] > 0
+
+
+def do_netpoll_flush_monitored(
+    cfg: NetDrvEpEnv, netdevnl: NetdevFamily, ifname: str, target_name: str
+) -> None:
+    """Print messages to the console, trying to trigger a netpoll poll"""
+    # Start bpftrace in parallel, so, it is watching
+    # netpoll_poll_dev() while we are sending netconsole messages
+    bpftrace_start()
+
+    do_netpoll_flush(cfg, netdevnl, ifname, target_name)
+
+    if bpftrace_any_hit(join=True):
+        ksft_pr("netpoll_poll_dev() was called. Success")
+        return
+
+    raise KsftSkipEx("netpoll_poll_dev() was not called. Skipping test")
+
+
+def do_netpoll_flush(
+    cfg: NetDrvEpEnv, netdevnl: NetdevFamily, ifname: str, target_name: str
+) -> None:
+    """Print messages to the console, trying to trigger a netpoll poll"""
+    netcons_configure_target(cfg, ifname, target_name)
+    retry = 0
+
+    for i in range(int(ITERATIONS)):
+        msg = f"netcons test #{i}"
+
+        with open("/dev/kmsg", "w", encoding="utf-8") as kmsg:
+            for j in range(10):
+                try:
+                    kmsg.write(f"{msg}-{j}\n")
+                except OSError as exception:
+                    # in some cases, kmsg can be busy, so, we will retry
+                    time.sleep(1)
+                    retry += 1
+                    if retry < 5:
+                        logging.info("Failed to write to kmsg. Retrying")
+                        # Just retry a few times
+                        continue
+                    raise KsftFailEx(
+                        f"Failed to write to kmsg: {exception}"
+                    ) from exception
+
+            if bpftrace_any_hit(join=False):
+                # Check if netpoll_poll_dev() was called, but do not wait for it
+                # to finish.
+                ksft_pr("netpoll_poll_dev() was called. Success")
+                return
+
+        # Every 5 iterations, toggle netconsole
+        netcons_delete_target(target_name)
+        netcons_configure_target(cfg, ifname, target_name)
+        # If we sleep here, we will have a better chance of triggering
+        # This number is based on a few tests I ran while developing this test
+        time.sleep(0.4)
+
+
+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 = netcons_generate_random_target_name()
+    ifname = cfg.dev["ifname"]
+    traffic = None
+    original_queues = ethtool_read_rx_tx_queue(ifname)
+
+    try:
+        # Set RX/TX queues to 1 to force congestion
+        ethtool_set_rx_tx_queue(ifname, 1, 1)
+
+        traffic = GenerateTraffic(cfg)
+        do_netpoll_flush_monitored(cfg, netdevnl, ifname, target_name)
+    finally:
+        if traffic:
+            traffic.stop()
+
+        # Revert RX/TX queues
+        ethtool_set_rx_tx_queue(ifname, original_queues[0], original_queues[1])
+        netcons_delete_target(target_name)
+        bpftrace_stop()
+
+
+def test_check_dependencies() -> None:
+    """Check if the dependencies are met"""
+    if not os.path.exists(NETCONSOLE_CONFIGFS_PATH):
+        raise KsftSkipEx(
+            f"Directory {NETCONSOLE_CONFIGFS_PATH} does not exist. CONFIG_NETCONSOLE_DYNAMIC might not be set."
+        )
+
+
+def main() -> None:
+    """Main function to run the test"""
+    netcons_load_module()
+    test_check_dependencies()
+    netdevnl = NetdevFamily()
+    with NetDrvEpEnv(__file__, nsim_test=True) as cfg:
+        ksft_run(
+            [test_netpoll],
+            args=(
+                cfg,
+                netdevnl,
+            ),
+        )
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll
  2025-06-25 11:39 [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Breno Leitao
                   ` (3 preceding siblings ...)
  2025-06-25 11:39 ` [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test Breno Leitao
@ 2025-06-25 18:43 ` Simon Horman
  2025-06-25 21:46   ` Jakub Kicinski
  4 siblings, 1 reply; 19+ messages in thread
From: Simon Horman @ 2025-06-25 18:43 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold, Stanislav Fomichev

+ Stan

On Wed, Jun 25, 2025 at 04:39:45AM -0700, Breno Leitao wrote:
> I am submitting a new selftest for the netpoll subsystem specifically
> targeting the case where the RX is polling in the TX path, which is
> a case that we don't have any test in the tree today. This is done when
> netpoll_poll_dev() called, and this test creates a scenario when that is
> probably.
> 
> The test does the following:
> 
> 1) Configuring a single RX/TX queue to increase contention on the
>    interface.
> 2) Generating background traffic to saturate the network, mimicking
>    real-world congestion.
> 3) Sending netconsole messages to trigger netpoll polling and monitor
>    its behavior.
> 4) Using dynamic netconsole targets via configfs, with the ability to
>    delete and recreate targets during the test.
> 5) Running bpftrace in parallel to verify that netpoll_poll_dev() is
>    called when expected. If it is called, then the test passes,
>    otherwise the test is marked as skipped.
> 
> In order to achieve it, I stole Jakub's bpftrace helper from [1], and
> did some small changes that I found useful to use the helper.
> 
> So, this patchset basically contains:
> 
> 1) The code stolen from Jakub
> 2) Improvements on bpftrace() helper
> 3) The selftest itself
> 
> Link: https://lore.kernel.org/all/20250421222827.283737-22-kuba@kernel.org/ [1]

Jakub, Stan, all,

It looks like bpftrace needs to be installed on the CI workers.
Currently they report something a lot like this:

# timeout set to 180
# selftests: drivers/net: netpoll_basic.py
# Exception in thread Thread-1:
# Traceback (most recent call last):
#   File "/usr/lib64/python3.9/threading.py", line 980, in _bootstrap_inner
#     self.run()
#   File "/usr/lib64/python3.9/threading.py", line 917, in run
#     self._target(*self._args, **self._kwargs)
#   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 198, in bpftrace_call
#     MAPS = bpftrace(expr, timeout=BPFTRACE_TIMEOUT, json=True)
#   File "/home/virtme/testing-17/tools/testing/selftests/net/lib/py/utils.py", line 205, in bpftrace
#     cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False)
#   File "/home/virtme/testing-17/tools/testing/selftests/net/lib/py/utils.py", line 60, in __init__
#     self.proc = subprocess.Popen(comm, shell=shell, stdout=subprocess.PIPE,
#   File "/usr/lib64/python3.9/subprocess.py", line 951, in __init__
#     self._execute_child(args, executable, preexec_fn, close_fds,
#   File "/usr/lib64/python3.9/subprocess.py", line 1837, in _execute_child
#     raise child_exception_type(errno_num, err_msg, err_filename)
# FileNotFoundError: [Errno 2] No such file or directory: 'bpftrace'
# TAP version 13
# 1..1
# # Exception| Traceback (most recent call last):
# # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/net/lib/py/ksft.py", line 243, in ksft_run
# # Exception|     case(*args)
# # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 308, in test_netpoll
# # Exception|     do_netpoll_flush_monitored(cfg, netdevnl, ifname, target_name)
# # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 243, in do_netpoll_flush_monitored
# # Exception|     do_netpoll_flush(cfg, netdevnl, ifname, target_name)
# # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 278, in do_netpoll_flush
# # Exception|     if bpftrace_any_hit(join=False):
# # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 230, in bpftrace_any_hit
# # Exception|     raise KsftFailEx(f"bpftrace failed to run!?: {MAPS}")
# # Exception| net.lib.py.ksft.KsftFailEx: bpftrace failed to run!?: {}
# not ok 1 netpoll_basic.test_netpoll
# # Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll
  2025-06-25 18:43 ` [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Simon Horman
@ 2025-06-25 21:46   ` Jakub Kicinski
  2025-06-26  8:17     ` Simon Horman
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2025-06-25 21:46 UTC (permalink / raw)
  To: Simon Horman
  Cc: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold, Stanislav Fomichev

On Wed, 25 Jun 2025 19:43:46 +0100 Simon Horman wrote:
> # # Exception| Traceback (most recent call last):
> # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/net/lib/py/ksft.py", line 243, in ksft_run
> # # Exception|     case(*args)
> # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 308, in test_netpoll
> # # Exception|     do_netpoll_flush_monitored(cfg, netdevnl, ifname, target_name)
> # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 243, in do_netpoll_flush_monitored
> # # Exception|     do_netpoll_flush(cfg, netdevnl, ifname, target_name)
> # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 278, in do_netpoll_flush
> # # Exception|     if bpftrace_any_hit(join=False):
> # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 230, in bpftrace_any_hit
> # # Exception|     raise KsftFailEx(f"bpftrace failed to run!?: {MAPS}")
> # # Exception| net.lib.py.ksft.KsftFailEx: bpftrace failed to run!?: {}

Looks like I haven't added bpftrace to the image before.
Fixed now.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 2/4] selftests: drv-net: Improve bpftrace utility error handling
  2025-06-25 11:39 ` [PATCH net-next v2 2/4] selftests: drv-net: Improve bpftrace utility error handling Breno Leitao
@ 2025-06-25 21:48   ` Jakub Kicinski
  2025-06-26 13:11     ` Breno Leitao
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2025-06-25 21:48 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Simon Horman, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Wed, 25 Jun 2025 04:39:47 -0700 Breno Leitao wrote:
>      cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False)
> +    if cmd_obj.ret != 0:
> +        raise Exception("Warning: bpftrace command returned a non-zero exit code.")

cmd should already raise CmdExitFailure, unless fail=False / fail=None
is specified.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 3/4] selftests: drv-net: Strip '@' prefix from bpftrace map keys
  2025-06-25 11:39 ` [PATCH net-next v2 3/4] selftests: drv-net: Strip '@' prefix from bpftrace map keys Breno Leitao
@ 2025-06-25 22:07   ` Jakub Kicinski
  2025-06-26 13:04     ` Breno Leitao
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2025-06-25 22:07 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Simon Horman, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Wed, 25 Jun 2025 04:39:48 -0700 Breno Leitao wrote:
> The '@' prefix in bpftrace map keys is specific to bpftrace and can be
> safely removed when processing results. This patch modifies the bpftrace
> utility to strip the '@' from map keys before storing them in the result
> dictionary, making the keys more consistent with Python conventions.

Make sense, tho, could you double check or ask Alastair if all outputs
are prefixed with @? Maybe there's some map type or other thingamajig
that doesn't have the prefix? 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test
  2025-06-25 11:39 ` [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test Breno Leitao
@ 2025-06-25 22:09   ` Jakub Kicinski
  2025-06-26 10:31     ` Breno Leitao
  2025-06-26  8:25   ` Simon Horman
  2025-06-26 16:31   ` Willem de Bruijn
  2 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2025-06-25 22:09 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Simon Horman, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Wed, 25 Jun 2025 04:39:49 -0700 Breno Leitao wrote:
> +    raise KsftSkipEx("netpoll_poll_dev() was not called. Skipping test")

Let's make this an Xfail. Looks like the condition doesn't trigger 
in VM testing :(

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 1/4] selftests: drv-net: add helper/wrapper for bpftrace
  2025-06-25 11:39 ` [PATCH net-next v2 1/4] selftests: drv-net: add helper/wrapper for bpftrace Breno Leitao
@ 2025-06-25 22:12   ` Jakub Kicinski
  0 siblings, 0 replies; 19+ messages in thread
From: Jakub Kicinski @ 2025-06-25 22:12 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Simon Horman, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Wed, 25 Jun 2025 04:39:46 -0700 Breno Leitao wrote:
> From: Jakub Kicinski <kuba@kernel.org>
> 
> bpftrace is very useful for low level driver testing. perf or trace-cmd
> would also do for collecting data from tracepoints, but they require
> much more post-processing.
> 
> Add a wrapper for running bpftrace and sanitizing its output.
> bpftrace has JSON output, which is great, but it prints loose objects
> and in a slightly inconvenient format. We have to read the objects
> line by line, and while at it return them indexed by the map name.

Could you squash this in? Otherwise pylint can't find it

diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py
index 9ed1d8f70524..98829a0f7a02 100644
--- a/tools/testing/selftests/drivers/net/lib/py/__init__.py
+++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py
@@ -14,7 +14,8 @@ KSFT_DIR = (Path(__file__).parent / "../../../..").resolve()
     from net.lib.py import EthtoolFamily, NetdevFamily, NetshaperFamily, \
         NlError, RtnlFamily
     from net.lib.py import CmdExitFailure
-    from net.lib.py import bkg, cmd, defer, ethtool, fd_read_timeout, ip, \
+    from net.lib.py import bkg, cmd, bpftrace, defer, ethtool, \
+        fd_read_timeout, ip, \
         rand_port, tool, wait_port_listen
     from net.lib.py import fd_read_timeout
     from net.lib.py import KsftSkipEx, KsftFailEx, KsftXfailEx

and with that I think it should be possible to make pylint clean on the
past patch?

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll
  2025-06-25 21:46   ` Jakub Kicinski
@ 2025-06-26  8:17     ` Simon Horman
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Horman @ 2025-06-26  8:17 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold, Stanislav Fomichev

On Wed, Jun 25, 2025 at 02:46:04PM -0700, Jakub Kicinski wrote:
> On Wed, 25 Jun 2025 19:43:46 +0100 Simon Horman wrote:
> > # # Exception| Traceback (most recent call last):
> > # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/net/lib/py/ksft.py", line 243, in ksft_run
> > # # Exception|     case(*args)
> > # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 308, in test_netpoll
> > # # Exception|     do_netpoll_flush_monitored(cfg, netdevnl, ifname, target_name)
> > # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 243, in do_netpoll_flush_monitored
> > # # Exception|     do_netpoll_flush(cfg, netdevnl, ifname, target_name)
> > # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 278, in do_netpoll_flush
> > # # Exception|     if bpftrace_any_hit(join=False):
> > # # Exception|   File "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./netpoll_basic.py", line 230, in bpftrace_any_hit
> > # # Exception|     raise KsftFailEx(f"bpftrace failed to run!?: {MAPS}")
> > # # Exception| net.lib.py.ksft.KsftFailEx: bpftrace failed to run!?: {}
> 
> Looks like I haven't added bpftrace to the image before.
> Fixed now.

Thanks, it looks good now.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test
  2025-06-25 11:39 ` [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test Breno Leitao
  2025-06-25 22:09   ` Jakub Kicinski
@ 2025-06-26  8:25   ` Simon Horman
  2025-06-27 15:18     ` Breno Leitao
  2025-06-26 16:31   ` Willem de Bruijn
  2 siblings, 1 reply; 19+ messages in thread
From: Simon Horman @ 2025-06-26  8:25 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Wed, Jun 25, 2025 at 04:39:49AM -0700, Breno Leitao wrote:
> Add a basic selftest for the netpoll polling mechanism, specifically
> targeting the netpoll poll() side.
> 
> The test creates a scenario where network transmission is running at
> maximum speed, and netpoll needs to poll the NIC. This is achieved by:
> 
>   1. Configuring a single RX/TX queue to create contention
>   2. Generating background traffic to saturate the interface
>   3. Sending netconsole messages to trigger netpoll polling
>   4. Using dynamic netconsole targets via configfs
>   5. Delete and create new netconsole targets after some messages
>   6. Start a bpftrace in parallel to make sure netpoll_poll_dev() is
>      called
>   7. If bpftrace exists and netpoll_poll_dev() was called, stop.
> 
> The test validates a critical netpoll code path by monitoring traffic
> flow and ensuring netpoll_poll_dev() is called when the normal TX path
> is blocked.
> 
> This addresses a gap in netpoll test coverage for a path that is
> tricky for the network stack.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Hi Breno,

As it looks like there will be another version,
could you run pylint over this. The NIPA invocation says:

  ************* Module netpoll_basic
  .../netpoll_basic.py:323:0: C0301: Line too long (111/100) (line-too-long)
  .../netpoll_basic.py:27:0: E0611: No name 'bpftrace' in module 'lib.py' (no-name-in-module)
  .../netpoll_basic.py:79:11: E0606: Possibly using variable 'rx_queue' before assignment (possibly-used-before-assignment)
  .../netpoll_basic.py:79:21: E0606: Possibly using variable 'tx_queue' before assignment (possibly-used-before-assignment)
  .../netpoll_basic.py:253:22: W0613: Unused argument 'netdevnl' (unused-argument)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test
  2025-06-25 22:09   ` Jakub Kicinski
@ 2025-06-26 10:31     ` Breno Leitao
  0 siblings, 0 replies; 19+ messages in thread
From: Breno Leitao @ 2025-06-26 10:31 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Simon Horman, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Wed, Jun 25, 2025 at 03:09:19PM -0700, Jakub Kicinski wrote:
> On Wed, 25 Jun 2025 04:39:49 -0700 Breno Leitao wrote:
> > +    raise KsftSkipEx("netpoll_poll_dev() was not called. Skipping test")
> 
> Let's make this an Xfail. Looks like the condition doesn't trigger 
> in VM testing :(

Exactly, there is a chance this will not trigger it, and you do not want
this bothering you. Having it as Xfail will be a false positive.

I have a plan to make this consitent by writing some configfs hook
for debug, then maybe we can make it a debug?!

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 3/4] selftests: drv-net: Strip '@' prefix from bpftrace map keys
  2025-06-25 22:07   ` Jakub Kicinski
@ 2025-06-26 13:04     ` Breno Leitao
  0 siblings, 0 replies; 19+ messages in thread
From: Breno Leitao @ 2025-06-26 13:04 UTC (permalink / raw)
  To: Jakub Kicinski, ajor
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Simon Horman, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Wed, Jun 25, 2025 at 03:07:10PM -0700, Jakub Kicinski wrote:
> On Wed, 25 Jun 2025 04:39:48 -0700 Breno Leitao wrote:
> > The '@' prefix in bpftrace map keys is specific to bpftrace and can be
> > safely removed when processing results. This patch modifies the bpftrace
> > utility to strip the '@' from map keys before storing them in the result
> > dictionary, making the keys more consistent with Python conventions.
> 
> Make sense, tho, could you double check or ask Alastair if all outputs
> are prefixed with @? Maybe there's some map type or other thingamajig
> that doesn't have the prefix? 

I have a quick chat with Alastair earlier today, and I understood that
all map symbols start with @.

CCing him.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 2/4] selftests: drv-net: Improve bpftrace utility error handling
  2025-06-25 21:48   ` Jakub Kicinski
@ 2025-06-26 13:11     ` Breno Leitao
  0 siblings, 0 replies; 19+ messages in thread
From: Breno Leitao @ 2025-06-26 13:11 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Simon Horman, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Wed, Jun 25, 2025 at 02:48:16PM -0700, Jakub Kicinski wrote:
> On Wed, 25 Jun 2025 04:39:47 -0700 Breno Leitao wrote:
> >      cmd_obj = cmd(cmd_arr, ns=ns, host=host, shell=False)
> > +    if cmd_obj.ret != 0:
> > +        raise Exception("Warning: bpftrace command returned a non-zero exit code.")
> 
> cmd should already raise CmdExitFailure, unless fail=False / fail=None
> is specified.

Oh yea, even easier. I found the code, and it is much easier to just set
fail=True for this case. This will avoid hidden errors.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test
  2025-06-25 11:39 ` [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test Breno Leitao
  2025-06-25 22:09   ` Jakub Kicinski
  2025-06-26  8:25   ` Simon Horman
@ 2025-06-26 16:31   ` Willem de Bruijn
  2025-06-26 17:10     ` Breno Leitao
  2 siblings, 1 reply; 19+ messages in thread
From: Willem de Bruijn @ 2025-06-26 16:31 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan, Simon Horman
  Cc: linux-kernel, netdev, linux-kselftest, Willem de Bruijn, bpf,
	gustavold, Breno Leitao

Breno Leitao wrote:
> Add a basic selftest for the netpoll polling mechanism, specifically
> targeting the netpoll poll() side.
> 
> The test creates a scenario where network transmission is running at
> maximum speed, and netpoll needs to poll the NIC. This is achieved by:
> 
>   1. Configuring a single RX/TX queue to create contention
>   2. Generating background traffic to saturate the interface
>   3. Sending netconsole messages to trigger netpoll polling
>   4. Using dynamic netconsole targets via configfs
>   5. Delete and create new netconsole targets after some messages
>   6. Start a bpftrace in parallel to make sure netpoll_poll_dev() is
>      called
>   7. If bpftrace exists and netpoll_poll_dev() was called, stop.
> 
> The test validates a critical netpoll code path by monitoring traffic
> flow and ensuring netpoll_poll_dev() is called when the normal TX path
> is blocked.
> 
> This addresses a gap in netpoll test coverage for a path that is
> tricky for the network stack.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

> +def bpftrace_call() -> None:
> +    """Call bpftrace to find how many times netpoll_poll_dev() is called.
> +    Output is saved in the global variable `maps`"""
> +
> +    # This is going to update the global variable, that will be seen by the
> +    # main function
> +    global MAPS  # pylint: disable=W0603
> +
> +    # This will be passed to bpftrace as in bpftrace -e "expr"
> +    expr = "BEGIN{ @hits = 0;} kprobe:netpoll_poll_dev { @hits += 1; }"

Is that BEGIN statement needed? I generally just use count().

> +
> +    MAPS = bpftrace(expr, timeout=BPFTRACE_TIMEOUT, json=True)
> +    logging.debug("BPFtrace output: %s", MAPS)
> +
> +
> +def bpftrace_start():
> +    """Start a thread to call `call_bpf` in parallel for 2 seconds."""

Stale comment? BPFTRACE_TIMEOUT is set to 15.

> +    global BPF_THREAD  # pylint: disable=W0603
> +
> +    BPF_THREAD = threading.Thread(target=bpftrace_call)
> +    BPF_THREAD.start()
> +    if not BPF_THREAD.is_alive():
> +        raise KsftSkipEx("BPFtrace thread is not alive. Skipping test")
> +
> +

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test
  2025-06-26 16:31   ` Willem de Bruijn
@ 2025-06-26 17:10     ` Breno Leitao
  0 siblings, 0 replies; 19+ messages in thread
From: Breno Leitao @ 2025-06-26 17:10 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman, linux-kernel, netdev,
	linux-kselftest, bpf, gustavold

On Thu, Jun 26, 2025 at 12:31:10PM -0400, Willem de Bruijn wrote:
> Breno Leitao wrote:
> > Add a basic selftest for the netpoll polling mechanism, specifically
> > targeting the netpoll poll() side.
> > 
> > The test creates a scenario where network transmission is running at
> > maximum speed, and netpoll needs to poll the NIC. This is achieved by:
> > 
> >   1. Configuring a single RX/TX queue to create contention
> >   2. Generating background traffic to saturate the interface
> >   3. Sending netconsole messages to trigger netpoll polling
> >   4. Using dynamic netconsole targets via configfs
> >   5. Delete and create new netconsole targets after some messages
> >   6. Start a bpftrace in parallel to make sure netpoll_poll_dev() is
> >      called
> >   7. If bpftrace exists and netpoll_poll_dev() was called, stop.
> > 
> > The test validates a critical netpoll code path by monitoring traffic
> > flow and ensuring netpoll_poll_dev() is called when the normal TX path
> > is blocked.
> > 
> > This addresses a gap in netpoll test coverage for a path that is
> > tricky for the network stack.
> > 
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> > +def bpftrace_call() -> None:
> > +    """Call bpftrace to find how many times netpoll_poll_dev() is called.
> > +    Output is saved in the global variable `maps`"""
> > +
> > +    # This is going to update the global variable, that will be seen by the
> > +    # main function
> > +    global MAPS  # pylint: disable=W0603
> > +
> > +    # This will be passed to bpftrace as in bpftrace -e "expr"
> > +    expr = "BEGIN{ @hits = 0;} kprobe:netpoll_poll_dev { @hits += 1; }"
> 
> Is that BEGIN statement needed? I generally just use count().

If I use `hits += 1` then yes, but, I've learned that I don't need it if
I use `count()`. So, I will see something like:

kprobe:netpoll_poll_dev { @hits = count(); }

> > +
> > +    MAPS = bpftrace(expr, timeout=BPFTRACE_TIMEOUT, json=True)
> > +    logging.debug("BPFtrace output: %s", MAPS)
> > +
> > +
> > +def bpftrace_start():
> > +    """Start a thread to call `call_bpf` in parallel for 2 seconds."""
> 
> Stale comment? BPFTRACE_TIMEOUT is set to 15.

Yes. I will remove it.

Thanks for the review,
--breno

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test
  2025-06-26  8:25   ` Simon Horman
@ 2025-06-27 15:18     ` Breno Leitao
  0 siblings, 0 replies; 19+ messages in thread
From: Breno Leitao @ 2025-06-27 15:18 UTC (permalink / raw)
  To: Simon Horman
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, linux-kernel, netdev, linux-kselftest,
	Willem de Bruijn, bpf, gustavold

On Thu, Jun 26, 2025 at 09:25:05AM +0100, Simon Horman wrote:
> On Wed, Jun 25, 2025 at 04:39:49AM -0700, Breno Leitao wrote:
> > Add a basic selftest for the netpoll polling mechanism, specifically
> > targeting the netpoll poll() side.
> > 
> > The test creates a scenario where network transmission is running at
> > maximum speed, and netpoll needs to poll the NIC. This is achieved by:
> > 
> >   1. Configuring a single RX/TX queue to create contention
> >   2. Generating background traffic to saturate the interface
> >   3. Sending netconsole messages to trigger netpoll polling
> >   4. Using dynamic netconsole targets via configfs
> >   5. Delete and create new netconsole targets after some messages
> >   6. Start a bpftrace in parallel to make sure netpoll_poll_dev() is
> >      called
> >   7. If bpftrace exists and netpoll_poll_dev() was called, stop.
> > 
> > The test validates a critical netpoll code path by monitoring traffic
> > flow and ensuring netpoll_poll_dev() is called when the normal TX path
> > is blocked.
> > 
> > This addresses a gap in netpoll test coverage for a path that is
> > tricky for the network stack.
> > 
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> Hi Breno,
> 
> As it looks like there will be another version,
> could you run pylint over this. The NIPA invocation says:
>
> 
>   ************* Module netpoll_basic
>   .../netpoll_basic.py:323:0: C0301: Line too long (111/100) (line-too-long)
>   .../netpoll_basic.py:27:0: E0611: No name 'bpftrace' in module 'lib.py' (no-name-in-module)
>   .../netpoll_basic.py:79:11: E0606: Possibly using variable 'rx_queue' before assignment (possibly-used-before-assignment)
>   .../netpoll_basic.py:79:21: E0606: Possibly using variable 'tx_queue' before assignment (possibly-used-before-assignment)
>   .../netpoll_basic.py:253:22: W0613: Unused argument 'netdevnl' (unused-argument)

Thanks for the report. I was able to reproduce them here. The next
version should be warning free. I hope. :-)

Thanks for the review,
--breno

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-06-27 15:18 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-25 11:39 [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Breno Leitao
2025-06-25 11:39 ` [PATCH net-next v2 1/4] selftests: drv-net: add helper/wrapper for bpftrace Breno Leitao
2025-06-25 22:12   ` Jakub Kicinski
2025-06-25 11:39 ` [PATCH net-next v2 2/4] selftests: drv-net: Improve bpftrace utility error handling Breno Leitao
2025-06-25 21:48   ` Jakub Kicinski
2025-06-26 13:11     ` Breno Leitao
2025-06-25 11:39 ` [PATCH net-next v2 3/4] selftests: drv-net: Strip '@' prefix from bpftrace map keys Breno Leitao
2025-06-25 22:07   ` Jakub Kicinski
2025-06-26 13:04     ` Breno Leitao
2025-06-25 11:39 ` [PATCH net-next v2 4/4] selftests: net: add netpoll basic functionality test Breno Leitao
2025-06-25 22:09   ` Jakub Kicinski
2025-06-26 10:31     ` Breno Leitao
2025-06-26  8:25   ` Simon Horman
2025-06-27 15:18     ` Breno Leitao
2025-06-26 16:31   ` Willem de Bruijn
2025-06-26 17:10     ` Breno Leitao
2025-06-25 18:43 ` [PATCH net-next v2 0/4] selftest: net: Add selftest for netpoll Simon Horman
2025-06-25 21:46   ` Jakub Kicinski
2025-06-26  8:17     ` Simon Horman

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).