Skip to content

Conversation

Bastian-Krause
Copy link
Member

Description
Fixes:

  • RawNetworkInterfaceDriver.record(..., timeout=<timeout>) / start_record(..., timeout=<timeout>) never stopping tcpdump if no packets were received.
  • RawNetworkInterfaceDriver.record(..., count=None) / start_record(..., count=None) calling labgrid-raw-interface leading to an error.
  • RawNetworkInterfaceDriver.record(...) / start_record(...) returning before the actual capture started, leading to missed packets

Checklist

  • PR has been tested

Fixes #1157
Fixes #1320

/cc @JoshuaWatt @nick-potenski

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 <timeout> <tcpdump command>

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 <[email protected]>
…ptional

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] 90cf941 ("helpers: add labgrid-raw-interface helper")

Fixes: c1aec37 ("helpers/labgrid-raw-interface: introduce subparsers per program, pass options")
Signed-off-by: Bastian Krause <[email protected]>
…ture 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 <[email protected]>
@Bastian-Krause Bastian-Krause added fix pick to stable Needs a pick to the latest stable branch labels Sep 24, 2025
Copy link

codecov bot commented Sep 24, 2025

Codecov Report

❌ Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.3%. Comparing base (2daa13d) to head (8801164).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
labgrid/driver/rawnetworkinterfacedriver.py 0.0% 14 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1745     +/-   ##
========================================
- Coverage    45.4%   45.3%   -0.1%     
========================================
  Files         172     172             
  Lines       13503   13516     +13     
========================================
  Hits         6131    6131             
- Misses       7372    7385     +13     
Flag Coverage Δ
3.10 45.3% <0.0%> (-0.1%) ⬇️
3.11 45.3% <0.0%> (-0.1%) ⬇️
3.12 45.3% <0.0%> (-0.1%) ⬇️
3.13 45.3% <0.0%> (-0.1%) ⬇️
3.9 45.4% <0.0%> (-0.1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix pick to stable Needs a pick to the latest stable branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants