From 03429b44cdbbd7455367fc877ea661834f99d275 Mon Sep 17 00:00:00 2001 From: Bastian Krause Date: Wed, 10 Sep 2025 14:25:18 +0200 Subject: [PATCH 1/3] helpers/labgrid-raw-interface: fix tcpdump timeout handling The current tcpdump timeout handling does not work. This is supposed to exit after 5 seconds: tcpdump --interface=eth0 -w - -G 5 -W 1 But that does not work if no packets are received. So in order to make the timeout actually work, prefix tcpdump with coreutils' timeout command instead: timeout --signal=INT --preserve-status By sending SIGINT (`--signal=INT`), tcpdump exits gracefully and finishes writing received packets. Combined with `--preserve-status`, tcpdump's actual exit status is preserved, being 0 even on SIGINT if nothing else went wrong. An alternative would be to use subprocess, but that would prevent us from using `os.execvp()`, replacing the process. So the timeout command is the better approach here. Signed-off-by: Bastian Krause --- helpers/labgrid-raw-interface | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/helpers/labgrid-raw-interface b/helpers/labgrid-raw-interface index f640fc5b0..4dd5b2c3f 100755 --- a/helpers/labgrid-raw-interface +++ b/helpers/labgrid-raw-interface @@ -73,12 +73,7 @@ def main(program, options): args.append(str(options.count)) if options.timeout: - # The timeout is implemented by specifying the number of seconds before rotating the - # dump file, but limiting the number of files to 1 - args.append("-G") - args.append(str(options.timeout)) - args.append("-W") - args.append("1") + args = ["timeout", "--signal=INT", "--preserve-status", str(options.timeout)] + args elif program == "ip": args.append("link") From 1e207b22b4a58355960863d593c8d759c176c925 Mon Sep 17 00:00:00 2001 From: Bastian Krause Date: Wed, 10 Sep 2025 14:26:22 +0200 Subject: [PATCH 2/3] helpers/labgrid-raw-interface: make tcpdump count argument actually optional The intention of labgrid-raw-interface was to have an optional "count" argument, allowing for packet capture on an interface until a timeout is reached, no matter how many packets are received. This is used when calling: RawNetworkInterfaceDriver.start_record(filename, timeout=10) However, the missing `nargs="?"` kwarg when adding the argument actually required the argument to be given: usage: labgrid-raw-interface tcpdump [-h] [--timeout TIMEOUT] ifname count labgrid-raw-interface tcpdump: error: the following arguments are required: count The original commit introducing the option actually included it [1]. Fix that by re-adding `nargs="?"`. [1] 90cf9413 ("helpers: add labgrid-raw-interface helper") Fixes: c1aec37e ("helpers/labgrid-raw-interface: introduce subparsers per program, pass options") Signed-off-by: Bastian Krause --- helpers/labgrid-raw-interface | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helpers/labgrid-raw-interface b/helpers/labgrid-raw-interface index 4dd5b2c3f..55b7e00fa 100755 --- a/helpers/labgrid-raw-interface +++ b/helpers/labgrid-raw-interface @@ -126,7 +126,7 @@ if __name__ == "__main__": # tcpdump tcpdump_parser = subparsers.add_parser("tcpdump") tcpdump_parser.add_argument("ifname", type=str, help="interface name") - tcpdump_parser.add_argument("count", type=int, default=None, help="amount of frames to capture while recording") + tcpdump_parser.add_argument("count", type=int, nargs="?", default=None, help="amount of frames to capture while recording") tcpdump_parser.add_argument( "--timeout", type=int, default=None, help="Amount of time to capture while recording. 0 means capture forever" ) From eef08feb9626055fed1fb7ead0301d8c3573dfa2 Mon Sep 17 00:00:00 2001 From: Bastian Krause Date: Wed, 10 Sep 2025 14:27:37 +0200 Subject: [PATCH 3/3] driver/rawnetworkinterfacedriver: make start_record() block until capture started The expectation of RawNetworkInterfaceDriver.start_record() is that it returns once the packet capturing is running, so later code can start right away generating packets that are received on the interface. This does not work reliably currently because the labgrid-raw-interface helper (wrapping tcpdump) is started via subprocess.Popen(), but that does not mean the capture is actually running just yet. To fix this, wait until the subprocess emits "tcpdump: listening on", which is tcpdump's way of telling that it's now actually capturing. Signed-off-by: Bastian Krause --- labgrid/driver/rawnetworkinterfacedriver.py | 22 ++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/labgrid/driver/rawnetworkinterfacedriver.py b/labgrid/driver/rawnetworkinterfacedriver.py index 8e5a7a703..8aecd5cf0 100644 --- a/labgrid/driver/rawnetworkinterfacedriver.py +++ b/labgrid/driver/rawnetworkinterfacedriver.py @@ -216,10 +216,30 @@ def start_record(self, filename, *, count=None, timeout=None): cmd.append(str(timeout)) cmd = self._wrap_command(cmd) if filename is None: - self._record_handle = subprocess.Popen(cmd, stdout=subprocess.PIPE) + self._record_handle = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) else: with open(filename, "wb") as outdata: self._record_handle = subprocess.Popen(cmd, stdout=outdata, stderr=subprocess.PIPE) + + # wait for capture start + stderr = b"" + while True: + line = self._record_handle.stderr.readline() + if not line: # process ended prematurely + try: + self._stop(self._record_handle) + except subprocess.CalledProcessError as e: + # readd consumed stderr to exception + e.stderr = stderr + raise + + if line.startswith(b"tcpdump: listening on"): + break + + # collect and log other lines in stderr before capturing has started + stderr += line + self.logger.warning(line.decode().rstrip()) + return self._record_handle @Driver.check_active