Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rawnetworkinterface stream #1320

Merged
merged 5 commits into from
Oct 18, 2024

Conversation

JoshuaWatt
Copy link
Contributor

@JoshuaWatt JoshuaWatt commented Jan 19, 2024

Description

Adds support for live streaming packets from a RawNetworkInterfaceDriver. This can be useful for writing tests that parse packets in real time looking for a specific pattern, for example:

import dpkt
    
drv = target.get_driver("RawNetworkInterfaceDriver")
    
with drv.record("-", timeout=60) as p:
    pcap = dpkt.pcap.Reader(p.stdout)
    for timestamp, buf in pcap:
        eth = dpkt.ethernet.Ethernet(buf)

        if _some_criteria_:
            break
    else:
        assert False, "Packet not found!"

Checklist

  • Documentation for the feature
  • Tests for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • Add a section on how to use the feature to doc/usage.rst
  • Add a section on how to use the feature to doc/development.rst
  • PR has been tested
  • Man pages have been regenerated

@JoshuaWatt
Copy link
Contributor Author

Of note, we've had a local driver that was very similar to RawNetworkInterfaceDriver for some time that was used to stream packets; the intention of this patch is to bring it in line with the functionality in that driver so we can eliminate it. For posterity, here is our driver:

import attr
import subprocess
import dpkt
from contextlib import contextmanager

from labgrid import target_factory
from labgrid.driver import Driver


@target_factory.reg_driver
@attr.s(eq=False)
class InterfaceCaptureDriver(Driver):
    """Driver to obtain tcpdump output from device's network"""

    bindings = {
        "iface": {"RemoteNetworkInterface", "USBNetworkInterface", "NetworkInterface"},
    }

    @contextmanager
    def live_capture(self, timeout=None):
        """
        Perform live capture of traffic from device's network.

        :param int timeout: When specified, performs capture for ``timeout`` seconds. \
            Defaults to `None`.

        :yields: A `dpkt.pcap.Reader` object of the captured network traffic
        """
        extra = getattr(self.iface, "extra", {})
        command = ["sudo", "tcpdump", "-i", self.iface.ifname, "-U", "-s0", "-w", "-"]

        if timeout is not None:
            command.extend(["-G", str(timeout), "-W", "1"])

        if "proxy_required" in extra:
            command = ["ssh", extra["proxy"], "--"] + command

        with subprocess.Popen(command, stdout=subprocess.PIPE) as p:
            yield dpkt.pcap.Reader(p.stdout)

Copy link

codecov bot commented Jan 19, 2024

Codecov Report

Attention: Patch coverage is 14.28571% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 62.7%. Comparing base (722838b) to head (d68686a).
Report is 9 commits behind head on master.

❗ Current head d68686a differs from pull request most recent head fdeb30b. Consider uploading reports for the commit fdeb30b to get more accurate results

Files Patch % Lines
labgrid/driver/rawnetworkinterfacedriver.py 14.2% 12 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1320     +/-   ##
========================================
+ Coverage    62.2%   62.7%   +0.5%     
========================================
  Files         164     163      -1     
  Lines       12191   12008    -183     
========================================
- Hits         7584    7534     -50     
+ Misses       4607    4474    -133     

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

Copy link
Member

@Bastian-Krause Bastian-Krause left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good overall.

In the commit message of d68686a ("rawnetworkinterfacedriver: Implement live streaming"):

-with drv.record("-", timeout=60) as p
+with drv.record("-", timeout=60) as p:

labgrid/driver/rawnetworkinterfacedriver.py Outdated Show resolved Hide resolved
helpers/labgrid-raw-interface Outdated Show resolved Hide resolved
helpers/labgrid-raw-interface Outdated Show resolved Hide resolved
labgrid/driver/rawnetworkinterfacedriver.py Outdated Show resolved Hide resolved
helpers/labgrid-raw-interface Outdated Show resolved Hide resolved
helpers/labgrid-raw-interface Outdated Show resolved Hide resolved
helpers/labgrid-raw-interface Outdated Show resolved Hide resolved
labgrid/driver/rawnetworkinterfacedriver.py Outdated Show resolved Hide resolved
labgrid/driver/rawnetworkinterfacedriver.py Outdated Show resolved Hide resolved
labgrid/driver/rawnetworkinterfacedriver.py Outdated Show resolved Hide resolved
labgrid/driver/rawnetworkinterfacedriver.py Outdated Show resolved Hide resolved
@Bastian-Krause
Copy link
Member

@JoshuaWatt I'm interested in using the changes in this PR in our network tests. Could you have a look at the review comments? If you don't have the time to integrate the changes, I'd be willing to take this over.

@JoshuaWatt
Copy link
Contributor Author

@JoshuaWatt I'm interested in using the changes in this PR in our network tests. Could you have a look at the review comments? If you don't have the time to integrate the changes, I'd be willing to take this over.

I'll take a look this week. I was AFK for EOSS

Copy link
Member

@Bastian-Krause Bastian-Krause left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the feedback. The changes look good to me. I'll try to test this within the next weeks and add my approval then.

@JoshuaWatt
Copy link
Contributor Author

Any update on this?

Copy link
Member

@Bastian-Krause Bastian-Krause left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. I've now tested this successfully.

I'm basically using it the same way as suggested, but with scapy instead of dpkt:

from scapy.all import PcapReader

drv = target.get_driver("RawNetworkInterfaceDriver")

with drv.record(None, count=1, timeout=5) as record:
        pcap = PcapReader(record.stdout)
        packet = pcap.next()
        ...

@JoshuaWatt
Copy link
Contributor Author

Ping

Copy link
Member

@jluebbe jluebbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good otherwise.

labgrid/driver/rawnetworkinterfacedriver.py Outdated Show resolved Hide resolved
helpers/labgrid-raw-interface Outdated Show resolved Hide resolved
@Bastian-Krause Bastian-Krause added the needs author info Requires more information from the PR/Issue author label Sep 16, 2024
@Bastian-Krause
Copy link
Member

@JoshuaWatt Are you still interested in this or should we take this over? I'm basing more and more network tests upon this PR, so I'd like to see this merged rather sooner than later.

@JoshuaWatt
Copy link
Contributor Author

@JoshuaWatt Are you still interested in this or should we take this over? I'm basing more and more network tests upon this PR, so I'd like to see this merged rather sooner than later.

Ya, Sorry. I've been really busy with other stuff lately. Feel free to take it over if you need it

@Bastian-Krause
Copy link
Member

Bastian-Krause commented Oct 11, 2024

@JoshuaWatt Will do, thanks for your work!

@Bastian-Krause Bastian-Krause removed the needs author info Requires more information from the PR/Issue author label Oct 12, 2024
Bastian-Krause and others added 5 commits October 14, 2024 11:44
… options

Adding a subparser per program keeps the code cleaner, especially when
introducing ethtool commands in the future. To keep up with various
options, pass the args namespace instead of adding kwargs for each new
setting.

Signed-off-by: Bastian Krause <[email protected]>
Reworks the way that timeouts are handled so that instead of terminating
the process in stop_record, tcpdump will exit after a set time. This is
will allow for live streaming of packets in a future patch

Signed-off-by: Joshua Watt <[email protected]>
Signed-off-by: Bastian Krause <[email protected]>
Adds support for live streaming of captured packets such that they can
be processed in real time by tests instead of needing to capture for a
set period of time time and do post-processing.

As an example:

```python
import dpkt

drv = target.get_driver("RawNetworkInterfaceDriver")

with drv.record(None, timeout=60) as p:
    pcap = dpkt.pcap.Reader(p.stdout)
    for timestamp, buf in pcap:
        eth = dpkt.ethernet.Ethernet(buf)
        ....
```

Signed-off-by: Joshua Watt <[email protected]>
Signed-off-by: Bastian Krause <[email protected]>
@Bastian-Krause
Copy link
Member

Rebased on latest master.

@jluebbe jluebbe merged commit adc614a into labgrid-project:master Oct 18, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants