On 3 Feb 2025, at 11:15, Adrián Moreno wrote:
> On Thu, Jan 30, 2025 at 02:35:04PM +0100, Eelco Chaudron wrote: >> >> >> On 17 Jan 2025, at 15:25, Adrian Moreno wrote: >> >>> Use pcapng instead of pcap format and store the result and the input >>> port name so they are visible in wireshark/tshark. >>> >>> Signed-off-by: Adrian Moreno <[email protected]> >> >> In general, this looks good to me. Some small comments below. >> >> //Eelco >> >>> --- >>> utilities/usdt-scripts/upcall_monitor.py | 13 +++++++++++-- >>> 1 file changed, 11 insertions(+), 2 deletions(-) >>> >>> diff --git a/utilities/usdt-scripts/upcall_monitor.py >>> b/utilities/usdt-scripts/upcall_monitor.py >>> index 104225cad..59828b064 100755 >>> --- a/utilities/usdt-scripts/upcall_monitor.py >>> +++ b/utilities/usdt-scripts/upcall_monitor.py >>> @@ -118,7 +118,7 @@ >>> >>> from bcc import BPF, USDT, USDTException >>> from os.path import exists >>> -from scapy.all import hexdump, wrpcap >>> +from scapy.all import hexdump, PcapNgWriter >>> from scapy.layers.l2 import Ether >>> >>> from usdt_lib import DpPortMapping >>> @@ -284,6 +284,8 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx) >>> #endif >>> """ >>> >>> +pcap_writer = None >>> + >>> >>> # >>> # print_key() >>> @@ -318,6 +320,8 @@ def print_key(event, decode_dump): >>> # print_event() >>> # >>> def print_event(ctx, data, size): >>> + global pcap_writer >>> + >>> event = b["events"].event(data) >>> dp = event.dpif_name.decode("utf-8") >>> >>> @@ -380,7 +384,12 @@ def print_event(ctx, data, size): >>> print(re.sub('^', ' ' * 4, packet.show(dump=True), >>> flags=re.MULTILINE)) >>> >>> if options.pcap is not None: >>> - wrpcap(options.pcap, packet, append=True, >>> snaplen=options.packet_size) >> >> If I’m correct this is introduced in scapy 2.4, which might not be available >> in all distros by default, for example, RHEL8? Are we ok with this and >> assume they need to pip-install it? >> > > That's a good point. We have a flaky vague dependency system, most of > the tools just fail to import dependencies and users have to install > them one by one :-). > > On this particular dependency, we can probably try to import > PcapNgWriter and fall back to wrpcap on failure. I guess these are debug script, and in most cases people might just copy them over so they are on manual install ;) > But it would be nice to > properly package (at least as a python package with dependency tracking) > these scripts. WDYT? I guess it depends on the individual packaging (per distro). I think for RPMs they are in the test package. Not sure if this will add additional unwanted packages just for the debug scripts. >>> + if pcap_writer is None: >>> + pcap_writer = PcapNgWriter(options.pcap) >>> + >>> + packet.comment = f"result={event.result}" >> >> Should we put in the whole line related to this packet? i.e. the print() >> from above, maybe including the key_dump? >> > > Yep, that can also go there, for sure. It'd be pretty convenient. Thanks. >>> + packet.sniffed_on = port >>> + pcap_writer.write(packet) >>> >>> >>> # >>> -- >>> 2.48.1 >>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
