diff mbox series

[ovs-dev,v3,7/8] utilities: upcall_monitor: Add extra info to pcap.

Message ID 20250227172340.4120887-8-amorenoz@redhat.com
State Changes Requested
Delegated to: aaron conole
Headers show
Series utilities: upcall_monitor: Monitor drops. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Adrián Moreno Feb. 27, 2025, 5:23 p.m. UTC
Use pcapng instead of pcap format and store the result, the key (if
available) and the input port name so they are visible in
wireshark/tshark.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 utilities/usdt-scripts/upcall_monitor.py | 53 +++++++++++++++++++-----
 1 file changed, 42 insertions(+), 11 deletions(-)

Comments

Eelco Chaudron March 11, 2025, 3:01 p.m. UTC | #1
On 27 Feb 2025, at 18:23, Adrian Moreno wrote:

> Use pcapng instead of pcap format and store the result, the key (if
> available) and the input port name so they are visible in
> wireshark/tshark.
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>

Some comments minor below.

> ---
>  utilities/usdt-scripts/upcall_monitor.py | 53 +++++++++++++++++++-----
>  1 file changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py
> index a1adeee0a..77378751f 100755
> --- a/utilities/usdt-scripts/upcall_monitor.py
> +++ b/utilities/usdt-scripts/upcall_monitor.py
> @@ -118,7 +118,12 @@
>
>  from bcc import BPF, USDT, USDTException
>  from os.path import exists
> -from scapy.all import hexdump, wrpcap
> +try:
> +    # Try using pcapng support from scapy >= 2.4.
> +    from scapy.all import hexdump, PcapNgWriter
> +except ImportError:
> +    from scapy.all import hexdump, wrpcap
> +
>  from scapy.layers.l2 import Ether
>
>  from usdt_lib import DpPortMapping
> @@ -282,40 +287,48 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx)
>  #endif
>  """
>
> +pcap_writer = None
> +
>
>  #
>  # print_key()
>  #
>  def print_key(event, decode_dump):

As this is no longer printing a key, I would change it to format_key().

> +    lines = []
>      if event.key_size < options.flow_key_size:
>          key_len = event.key_size
>      else:
>          key_len = options.flow_key_size
>
>      if not key_len:
> -        return
> +        return []
>
>      if options.flow_key_decode != 'none':
> -        print("  Flow key size {} bytes, size captured {} bytes.".
> -              format(event.key_size, key_len))
> +        lines.append("  Flow key size {} bytes, size captured {} bytes.".
> +                     format(event.key_size, key_len))
>
>      if options.flow_key_decode == 'hex':
>          #
>          # Abuse scapy's hex dump to dump flow key
>          #
> -        print(re.sub('^', ' ' * 4, hexdump(Ether(bytes(event.key)[:key_len]),
> -                                           dump=True),
> -                     flags=re.MULTILINE))
> +        lines.extend(re.sub('^', ' ' * 4,
> +            hexdump(
> +                Ether(bytes(event.key)[:key_len]),
> +                dump=True),
> +            flags=re.MULTILINE).split("\n"))
>
>      if options.flow_key_decode == "nlraw":
> -        for line in decode_dump:
> -            print(line)
> +        lines.extend(decode_dump)
> +
> +    return lines
>
>
>  #
>  # print_event()
>  #
>  def print_event(ctx, data, size):
> +    global pcap_writer
> +
>      event = b["events"].event(data)
>      dp = event.dpif_name.decode("utf-8")
>
> @@ -350,7 +363,9 @@ def print_event(ctx, data, size):
>      #
>      # Dump flow key information
>      #
> -    print_key(event, key_dump)
> +    key_lines = print_key(event, key_dump)
> +    for line in key_lines:
> +        print(line)
>
>      #
>      # Decode packet only if there is data
> @@ -383,7 +398,23 @@ 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)
> +        try:
> +            if pcap_writer is None:
> +                pcap_writer = PcapNgWriter(options.pcap)
> +
> +            comment = "cpu={} comm={} pid={} upcall_type={} result={}". format(

Adding the time stamp here might also be useful to “quickly” see the inter-packet gap.

> +                event.cpu, event.comm.decode("utf-8"), event.pid,
> +                event.upcall_type, event.result)
> +
> +            if options.flow_key_decode != 'none':
> +                comment = comment + "\n" + "\n".join(key_lines)
> +
> +            packet.comment = comment
> +            packet.sniffed_on = "{} ({})".format(port, dp)
> +            pcap_writer.write(packet)
> +        except NameError:  # PcapNgWriter not found
> +            wrpcap(options.pcap, packet, append=True,
> +                   snaplen=options.packet_size)
>
>
>  #
> -- 
> 2.48.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Eelco Chaudron March 11, 2025, 3:06 p.m. UTC | #2
On 11 Mar 2025, at 16:01, Eelco Chaudron wrote:

> On 27 Feb 2025, at 18:23, Adrian Moreno wrote:
>
>> Use pcapng instead of pcap format and store the result, the key (if
>> available) and the input port name so they are visible in
>> wireshark/tshark.
>>
>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>
> Some comments minor below.

Did some testing and the port number does not seem to be part of the capture.

./upcall_monitor.py -d decode -k nlraw -r error -w error.pcap

$ tshark -r error.pcap -V
Packet comments
    cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11

        [Expert Info (Comment/Comment): cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
]
            [cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
]
            [Severity level: Comment]
            [Group: Comment]
Frame 1: 1496 bytes on wire (11968 bits), 64 bytes captured (512 bits) on interface unknown, id 0
    Interface id: 0 (unknown)
        Interface name: unknown
                        ^^^^^^^

>> ---
>>  utilities/usdt-scripts/upcall_monitor.py | 53 +++++++++++++++++++-----
>>  1 file changed, 42 insertions(+), 11 deletions(-)
>>
>> diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py
>> index a1adeee0a..77378751f 100755
>> --- a/utilities/usdt-scripts/upcall_monitor.py
>> +++ b/utilities/usdt-scripts/upcall_monitor.py
>> @@ -118,7 +118,12 @@
>>
>>  from bcc import BPF, USDT, USDTException
>>  from os.path import exists
>> -from scapy.all import hexdump, wrpcap
>> +try:
>> +    # Try using pcapng support from scapy >= 2.4.
>> +    from scapy.all import hexdump, PcapNgWriter
>> +except ImportError:
>> +    from scapy.all import hexdump, wrpcap
>> +
>>  from scapy.layers.l2 import Ether
>>
>>  from usdt_lib import DpPortMapping
>> @@ -282,40 +287,48 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx)
>>  #endif
>>  """
>>
>> +pcap_writer = None
>> +
>>
>>  #
>>  # print_key()
>>  #
>>  def print_key(event, decode_dump):
>
> As this is no longer printing a key, I would change it to format_key().
>
>> +    lines = []
>>      if event.key_size < options.flow_key_size:
>>          key_len = event.key_size
>>      else:
>>          key_len = options.flow_key_size
>>
>>      if not key_len:
>> -        return
>> +        return []
>>
>>      if options.flow_key_decode != 'none':
>> -        print("  Flow key size {} bytes, size captured {} bytes.".
>> -              format(event.key_size, key_len))
>> +        lines.append("  Flow key size {} bytes, size captured {} bytes.".
>> +                     format(event.key_size, key_len))
>>
>>      if options.flow_key_decode == 'hex':
>>          #
>>          # Abuse scapy's hex dump to dump flow key
>>          #
>> -        print(re.sub('^', ' ' * 4, hexdump(Ether(bytes(event.key)[:key_len]),
>> -                                           dump=True),
>> -                     flags=re.MULTILINE))
>> +        lines.extend(re.sub('^', ' ' * 4,
>> +            hexdump(
>> +                Ether(bytes(event.key)[:key_len]),
>> +                dump=True),
>> +            flags=re.MULTILINE).split("\n"))
>>
>>      if options.flow_key_decode == "nlraw":
>> -        for line in decode_dump:
>> -            print(line)
>> +        lines.extend(decode_dump)
>> +
>> +    return lines
>>
>>
>>  #
>>  # print_event()
>>  #
>>  def print_event(ctx, data, size):
>> +    global pcap_writer
>> +
>>      event = b["events"].event(data)
>>      dp = event.dpif_name.decode("utf-8")
>>
>> @@ -350,7 +363,9 @@ def print_event(ctx, data, size):
>>      #
>>      # Dump flow key information
>>      #
>> -    print_key(event, key_dump)
>> +    key_lines = print_key(event, key_dump)
>> +    for line in key_lines:
>> +        print(line)
>>
>>      #
>>      # Decode packet only if there is data
>> @@ -383,7 +398,23 @@ 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)
>> +        try:
>> +            if pcap_writer is None:
>> +                pcap_writer = PcapNgWriter(options.pcap)
>> +
>> +            comment = "cpu={} comm={} pid={} upcall_type={} result={}". format(
>
> Adding the time stamp here might also be useful to “quickly” see the inter-packet gap.
>
>> +                event.cpu, event.comm.decode("utf-8"), event.pid,
>> +                event.upcall_type, event.result)
>> +
>> +            if options.flow_key_decode != 'none':
>> +                comment = comment + "\n" + "\n".join(key_lines)
>> +
>> +            packet.comment = comment
>> +            packet.sniffed_on = "{} ({})".format(port, dp)
>> +            pcap_writer.write(packet)
>> +        except NameError:  # PcapNgWriter not found
>> +            wrpcap(options.pcap, packet, append=True,
>> +                   snaplen=options.packet_size)
>>
>>
>>  #
>> -- 
>> 2.48.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Adrián Moreno March 21, 2025, 10:14 a.m. UTC | #3
On Tue, Mar 11, 2025 at 04:06:41PM +0100, Eelco Chaudron wrote:
>
>
> On 11 Mar 2025, at 16:01, Eelco Chaudron wrote:
>
> > On 27 Feb 2025, at 18:23, Adrian Moreno wrote:
> >
> >> Use pcapng instead of pcap format and store the result, the key (if
> >> available) and the input port name so they are visible in
> >> wireshark/tshark.
> >>
> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> >
> > Some comments minor below.
>
> Did some testing and the port number does not seem to be part of the capture.
>
> ./upcall_monitor.py -d decode -k nlraw -r error -w error.pcap
>
> $ tshark -r error.pcap -V
> Packet comments
>     cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
>
>         [Expert Info (Comment/Comment): cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
> ]
>             [cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
> ]
>             [Severity level: Comment]
>             [Group: Comment]
> Frame 1: 1496 bytes on wire (11968 bits), 64 bytes captured (512 bits) on interface unknown, id 0
>     Interface id: 0 (unknown)
>         Interface name: unknown
>                         ^^^^^^^
>

That's weird, I cannot reproduce it. How did you generate the failed
upcall?


> >> ---
> >>  utilities/usdt-scripts/upcall_monitor.py | 53 +++++++++++++++++++-----
> >>  1 file changed, 42 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py
> >> index a1adeee0a..77378751f 100755
> >> --- a/utilities/usdt-scripts/upcall_monitor.py
> >> +++ b/utilities/usdt-scripts/upcall_monitor.py
> >> @@ -118,7 +118,12 @@
> >>
> >>  from bcc import BPF, USDT, USDTException
> >>  from os.path import exists
> >> -from scapy.all import hexdump, wrpcap
> >> +try:
> >> +    # Try using pcapng support from scapy >= 2.4.
> >> +    from scapy.all import hexdump, PcapNgWriter
> >> +except ImportError:
> >> +    from scapy.all import hexdump, wrpcap
> >> +
> >>  from scapy.layers.l2 import Ether
> >>
> >>  from usdt_lib import DpPortMapping
> >> @@ -282,40 +287,48 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx)
> >>  #endif
> >>  """
> >>
> >> +pcap_writer = None
> >> +
> >>
> >>  #
> >>  # print_key()
> >>  #
> >>  def print_key(event, decode_dump):
> >
> > As this is no longer printing a key, I would change it to format_key().
> >
> >> +    lines = []
> >>      if event.key_size < options.flow_key_size:
> >>          key_len = event.key_size
> >>      else:
> >>          key_len = options.flow_key_size
> >>
> >>      if not key_len:
> >> -        return
> >> +        return []
> >>
> >>      if options.flow_key_decode != 'none':
> >> -        print("  Flow key size {} bytes, size captured {} bytes.".
> >> -              format(event.key_size, key_len))
> >> +        lines.append("  Flow key size {} bytes, size captured {} bytes.".
> >> +                     format(event.key_size, key_len))
> >>
> >>      if options.flow_key_decode == 'hex':
> >>          #
> >>          # Abuse scapy's hex dump to dump flow key
> >>          #
> >> -        print(re.sub('^', ' ' * 4, hexdump(Ether(bytes(event.key)[:key_len]),
> >> -                                           dump=True),
> >> -                     flags=re.MULTILINE))
> >> +        lines.extend(re.sub('^', ' ' * 4,
> >> +            hexdump(
> >> +                Ether(bytes(event.key)[:key_len]),
> >> +                dump=True),
> >> +            flags=re.MULTILINE).split("\n"))
> >>
> >>      if options.flow_key_decode == "nlraw":
> >> -        for line in decode_dump:
> >> -            print(line)
> >> +        lines.extend(decode_dump)
> >> +
> >> +    return lines
> >>
> >>
> >>  #
> >>  # print_event()
> >>  #
> >>  def print_event(ctx, data, size):
> >> +    global pcap_writer
> >> +
> >>      event = b["events"].event(data)
> >>      dp = event.dpif_name.decode("utf-8")
> >>
> >> @@ -350,7 +363,9 @@ def print_event(ctx, data, size):
> >>      #
> >>      # Dump flow key information
> >>      #
> >> -    print_key(event, key_dump)
> >> +    key_lines = print_key(event, key_dump)
> >> +    for line in key_lines:
> >> +        print(line)
> >>
> >>      #
> >>      # Decode packet only if there is data
> >> @@ -383,7 +398,23 @@ 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)
> >> +        try:
> >> +            if pcap_writer is None:
> >> +                pcap_writer = PcapNgWriter(options.pcap)
> >> +
> >> +            comment = "cpu={} comm={} pid={} upcall_type={} result={}". format(
> >
> > Adding the time stamp here might also be useful to “quickly” see the inter-packet gap.
> >
> >> +                event.cpu, event.comm.decode("utf-8"), event.pid,
> >> +                event.upcall_type, event.result)
> >> +
> >> +            if options.flow_key_decode != 'none':
> >> +                comment = comment + "\n" + "\n".join(key_lines)
> >> +
> >> +            packet.comment = comment
> >> +            packet.sniffed_on = "{} ({})".format(port, dp)
> >> +            pcap_writer.write(packet)
> >> +        except NameError:  # PcapNgWriter not found
> >> +            wrpcap(options.pcap, packet, append=True,
> >> +                   snaplen=options.packet_size)
> >>
> >>
> >>  #
> >> --
> >> 2.48.1
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Eelco Chaudron March 21, 2025, 10:19 a.m. UTC | #4
On 21 Mar 2025, at 11:14, Adrián Moreno wrote:

> On Tue, Mar 11, 2025 at 04:06:41PM +0100, Eelco Chaudron wrote:
>>
>>
>> On 11 Mar 2025, at 16:01, Eelco Chaudron wrote:
>>
>>> On 27 Feb 2025, at 18:23, Adrian Moreno wrote:
>>>
>>>> Use pcapng instead of pcap format and store the result, the key (if
>>>> available) and the input port name so they are visible in
>>>> wireshark/tshark.
>>>>
>>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>>
>>> Some comments minor below.
>>
>> Did some testing and the port number does not seem to be part of the capture.
>>
>> ./upcall_monitor.py -d decode -k nlraw -r error -w error.pcap
>>
>> $ tshark -r error.pcap -V
>> Packet comments
>>     cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
>>
>>         [Expert Info (Comment/Comment): cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
>> ]
>>             [cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
>> ]
>>             [Severity level: Comment]
>>             [Group: Comment]
>> Frame 1: 1496 bytes on wire (11968 bits), 64 bytes captured (512 bits) on interface unknown, id 0
>>     Interface id: 0 (unknown)
>>         Interface name: unknown
>>                         ^^^^^^^
>>
>
> That's weird, I cannot reproduce it. How did you generate the failed
> upcall?

Just start ovs_perf with 10k flows, which will bombard OVS. The port number is correct in the script output, just not in the pcap.

This is my version of scapy (dont think I upgraded on the mean time ;)

$ pip show scapy
Name: scapy
Version: 2.5.0
Summary: Scapy: interactive packet manipulation tool
Home-page: https://scapy.net
Author: Philippe BIONDI
Author-email: guillaume@valadon.net
License: GPL-2.0-only
Location: /usr/local/lib/python3.9/site-packages
Requires:
Required-by:

>>>> ---
>>>>  utilities/usdt-scripts/upcall_monitor.py | 53 +++++++++++++++++++-----
>>>>  1 file changed, 42 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py
>>>> index a1adeee0a..77378751f 100755
>>>> --- a/utilities/usdt-scripts/upcall_monitor.py
>>>> +++ b/utilities/usdt-scripts/upcall_monitor.py
>>>> @@ -118,7 +118,12 @@
>>>>
>>>>  from bcc import BPF, USDT, USDTException
>>>>  from os.path import exists
>>>> -from scapy.all import hexdump, wrpcap
>>>> +try:
>>>> +    # Try using pcapng support from scapy >= 2.4.
>>>> +    from scapy.all import hexdump, PcapNgWriter
>>>> +except ImportError:
>>>> +    from scapy.all import hexdump, wrpcap
>>>> +
>>>>  from scapy.layers.l2 import Ether
>>>>
>>>>  from usdt_lib import DpPortMapping
>>>> @@ -282,40 +287,48 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx)
>>>>  #endif
>>>>  """
>>>>
>>>> +pcap_writer = None
>>>> +
>>>>
>>>>  #
>>>>  # print_key()
>>>>  #
>>>>  def print_key(event, decode_dump):
>>>
>>> As this is no longer printing a key, I would change it to format_key().
>>>
>>>> +    lines = []
>>>>      if event.key_size < options.flow_key_size:
>>>>          key_len = event.key_size
>>>>      else:
>>>>          key_len = options.flow_key_size
>>>>
>>>>      if not key_len:
>>>> -        return
>>>> +        return []
>>>>
>>>>      if options.flow_key_decode != 'none':
>>>> -        print("  Flow key size {} bytes, size captured {} bytes.".
>>>> -              format(event.key_size, key_len))
>>>> +        lines.append("  Flow key size {} bytes, size captured {} bytes.".
>>>> +                     format(event.key_size, key_len))
>>>>
>>>>      if options.flow_key_decode == 'hex':
>>>>          #
>>>>          # Abuse scapy's hex dump to dump flow key
>>>>          #
>>>> -        print(re.sub('^', ' ' * 4, hexdump(Ether(bytes(event.key)[:key_len]),
>>>> -                                           dump=True),
>>>> -                     flags=re.MULTILINE))
>>>> +        lines.extend(re.sub('^', ' ' * 4,
>>>> +            hexdump(
>>>> +                Ether(bytes(event.key)[:key_len]),
>>>> +                dump=True),
>>>> +            flags=re.MULTILINE).split("\n"))
>>>>
>>>>      if options.flow_key_decode == "nlraw":
>>>> -        for line in decode_dump:
>>>> -            print(line)
>>>> +        lines.extend(decode_dump)
>>>> +
>>>> +    return lines
>>>>
>>>>
>>>>  #
>>>>  # print_event()
>>>>  #
>>>>  def print_event(ctx, data, size):
>>>> +    global pcap_writer
>>>> +
>>>>      event = b["events"].event(data)
>>>>      dp = event.dpif_name.decode("utf-8")
>>>>
>>>> @@ -350,7 +363,9 @@ def print_event(ctx, data, size):
>>>>      #
>>>>      # Dump flow key information
>>>>      #
>>>> -    print_key(event, key_dump)
>>>> +    key_lines = print_key(event, key_dump)
>>>> +    for line in key_lines:
>>>> +        print(line)
>>>>
>>>>      #
>>>>      # Decode packet only if there is data
>>>> @@ -383,7 +398,23 @@ 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)
>>>> +        try:
>>>> +            if pcap_writer is None:
>>>> +                pcap_writer = PcapNgWriter(options.pcap)
>>>> +
>>>> +            comment = "cpu={} comm={} pid={} upcall_type={} result={}". format(
>>>
>>> Adding the time stamp here might also be useful to “quickly” see the inter-packet gap.
>>>
>>>> +                event.cpu, event.comm.decode("utf-8"), event.pid,
>>>> +                event.upcall_type, event.result)
>>>> +
>>>> +            if options.flow_key_decode != 'none':
>>>> +                comment = comment + "\n" + "\n".join(key_lines)
>>>> +
>>>> +            packet.comment = comment
>>>> +            packet.sniffed_on = "{} ({})".format(port, dp)
>>>> +            pcap_writer.write(packet)
>>>> +        except NameError:  # PcapNgWriter not found
>>>> +            wrpcap(options.pcap, packet, append=True,
>>>> +                   snaplen=options.packet_size)
>>>>
>>>>
>>>>  #
>>>> --
>>>> 2.48.1
>>>>
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev@openvswitch.org
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
Adrián Moreno March 26, 2025, 10:24 a.m. UTC | #5
On Fri, Mar 21, 2025 at 11:19:56AM +0100, Eelco Chaudron wrote:
>
>
> On 21 Mar 2025, at 11:14, Adrián Moreno wrote:
>
> > On Tue, Mar 11, 2025 at 04:06:41PM +0100, Eelco Chaudron wrote:
> >>
> >>
> >> On 11 Mar 2025, at 16:01, Eelco Chaudron wrote:
> >>
> >>> On 27 Feb 2025, at 18:23, Adrian Moreno wrote:
> >>>
> >>>> Use pcapng instead of pcap format and store the result, the key (if
> >>>> available) and the input port name so they are visible in
> >>>> wireshark/tshark.
> >>>>
> >>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> >>>
> >>> Some comments minor below.
> >>
> >> Did some testing and the port number does not seem to be part of the capture.
> >>
> >> ./upcall_monitor.py -d decode -k nlraw -r error -w error.pcap
> >>
> >> $ tshark -r error.pcap -V
> >> Packet comments
> >>     cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
> >>
> >>         [Expert Info (Comment/Comment): cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
> >> ]
> >>             [cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
> >> ]
> >>             [Severity level: Comment]
> >>             [Group: Comment]
> >> Frame 1: 1496 bytes on wire (11968 bits), 64 bytes captured (512 bits) on interface unknown, id 0
> >>     Interface id: 0 (unknown)
> >>         Interface name: unknown
> >>                         ^^^^^^^
> >>
> >
> > That's weird, I cannot reproduce it. How did you generate the failed
> > upcall?
>
> Just start ovs_perf with 10k flows, which will bombard OVS. The port number is correct in the script output, just not in the pcap.
>
> This is my version of scapy (dont think I upgraded on the mean time ;)
>
> $ pip show scapy
> Name: scapy
> Version: 2.5.0
> Summary: Scapy: interactive packet manipulation tool
> Home-page: https://scapy.net
> Author: Philippe BIONDI
> Author-email: guillaume@valadon.net
> License: GPL-2.0-only
> Location: /usr/local/lib/python3.9/site-packages
> Requires:
> Required-by:
>

Ugh, this feature was introduced in scapy 2.6

https://github.com/secdev/scapy/commit/56b4fa4adc6603b410c87c64a3ea3278ef69ca01

Not much we can do about this other than adding the interface name to
the comment just in case. WDYT?

Thanks.
Adrián

> >>>> ---
> >>>>  utilities/usdt-scripts/upcall_monitor.py | 53 +++++++++++++++++++-----
> >>>>  1 file changed, 42 insertions(+), 11 deletions(-)
> >>>>
> >>>> diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py
> >>>> index a1adeee0a..77378751f 100755
> >>>> --- a/utilities/usdt-scripts/upcall_monitor.py
> >>>> +++ b/utilities/usdt-scripts/upcall_monitor.py
> >>>> @@ -118,7 +118,12 @@
> >>>>
> >>>>  from bcc import BPF, USDT, USDTException
> >>>>  from os.path import exists
> >>>> -from scapy.all import hexdump, wrpcap
> >>>> +try:
> >>>> +    # Try using pcapng support from scapy >= 2.4.
> >>>> +    from scapy.all import hexdump, PcapNgWriter
> >>>> +except ImportError:
> >>>> +    from scapy.all import hexdump, wrpcap
> >>>> +
> >>>>  from scapy.layers.l2 import Ether
> >>>>
> >>>>  from usdt_lib import DpPortMapping
> >>>> @@ -282,40 +287,48 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx)
> >>>>  #endif
> >>>>  """
> >>>>
> >>>> +pcap_writer = None
> >>>> +
> >>>>
> >>>>  #
> >>>>  # print_key()
> >>>>  #
> >>>>  def print_key(event, decode_dump):
> >>>
> >>> As this is no longer printing a key, I would change it to format_key().
> >>>
> >>>> +    lines = []
> >>>>      if event.key_size < options.flow_key_size:
> >>>>          key_len = event.key_size
> >>>>      else:
> >>>>          key_len = options.flow_key_size
> >>>>
> >>>>      if not key_len:
> >>>> -        return
> >>>> +        return []
> >>>>
> >>>>      if options.flow_key_decode != 'none':
> >>>> -        print("  Flow key size {} bytes, size captured {} bytes.".
> >>>> -              format(event.key_size, key_len))
> >>>> +        lines.append("  Flow key size {} bytes, size captured {} bytes.".
> >>>> +                     format(event.key_size, key_len))
> >>>>
> >>>>      if options.flow_key_decode == 'hex':
> >>>>          #
> >>>>          # Abuse scapy's hex dump to dump flow key
> >>>>          #
> >>>> -        print(re.sub('^', ' ' * 4, hexdump(Ether(bytes(event.key)[:key_len]),
> >>>> -                                           dump=True),
> >>>> -                     flags=re.MULTILINE))
> >>>> +        lines.extend(re.sub('^', ' ' * 4,
> >>>> +            hexdump(
> >>>> +                Ether(bytes(event.key)[:key_len]),
> >>>> +                dump=True),
> >>>> +            flags=re.MULTILINE).split("\n"))
> >>>>
> >>>>      if options.flow_key_decode == "nlraw":
> >>>> -        for line in decode_dump:
> >>>> -            print(line)
> >>>> +        lines.extend(decode_dump)
> >>>> +
> >>>> +    return lines
> >>>>
> >>>>
> >>>>  #
> >>>>  # print_event()
> >>>>  #
> >>>>  def print_event(ctx, data, size):
> >>>> +    global pcap_writer
> >>>> +
> >>>>      event = b["events"].event(data)
> >>>>      dp = event.dpif_name.decode("utf-8")
> >>>>
> >>>> @@ -350,7 +363,9 @@ def print_event(ctx, data, size):
> >>>>      #
> >>>>      # Dump flow key information
> >>>>      #
> >>>> -    print_key(event, key_dump)
> >>>> +    key_lines = print_key(event, key_dump)
> >>>> +    for line in key_lines:
> >>>> +        print(line)
> >>>>
> >>>>      #
> >>>>      # Decode packet only if there is data
> >>>> @@ -383,7 +398,23 @@ 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)
> >>>> +        try:
> >>>> +            if pcap_writer is None:
> >>>> +                pcap_writer = PcapNgWriter(options.pcap)
> >>>> +
> >>>> +            comment = "cpu={} comm={} pid={} upcall_type={} result={}". format(
> >>>
> >>> Adding the time stamp here might also be useful to “quickly” see the inter-packet gap.
> >>>
> >>>> +                event.cpu, event.comm.decode("utf-8"), event.pid,
> >>>> +                event.upcall_type, event.result)
> >>>> +
> >>>> +            if options.flow_key_decode != 'none':
> >>>> +                comment = comment + "\n" + "\n".join(key_lines)
> >>>> +
> >>>> +            packet.comment = comment
> >>>> +            packet.sniffed_on = "{} ({})".format(port, dp)
> >>>> +            pcap_writer.write(packet)
> >>>> +        except NameError:  # PcapNgWriter not found
> >>>> +            wrpcap(options.pcap, packet, append=True,
> >>>> +                   snaplen=options.packet_size)
> >>>>
> >>>>
> >>>>  #
> >>>> --
> >>>> 2.48.1
> >>>>
> >>>> _______________________________________________
> >>>> dev mailing list
> >>>> dev@openvswitch.org
> >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
>
Eelco Chaudron March 26, 2025, 1:20 p.m. UTC | #6
On 26 Mar 2025, at 11:24, Adrián Moreno wrote:

> On Fri, Mar 21, 2025 at 11:19:56AM +0100, Eelco Chaudron wrote:
>>
>>
>> On 21 Mar 2025, at 11:14, Adrián Moreno wrote:
>>
>>> On Tue, Mar 11, 2025 at 04:06:41PM +0100, Eelco Chaudron wrote:
>>>>
>>>>
>>>> On 11 Mar 2025, at 16:01, Eelco Chaudron wrote:
>>>>
>>>>> On 27 Feb 2025, at 18:23, Adrian Moreno wrote:
>>>>>
>>>>>> Use pcapng instead of pcap format and store the result, the key (if
>>>>>> available) and the input port name so they are visible in
>>>>>> wireshark/tshark.
>>>>>>
>>>>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>>>>
>>>>> Some comments minor below.
>>>>
>>>> Did some testing and the port number does not seem to be part of the capture.
>>>>
>>>> ./upcall_monitor.py -d decode -k nlraw -r error -w error.pcap
>>>>
>>>> $ tshark -r error.pcap -V
>>>> Packet comments
>>>>     cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
>>>>
>>>>         [Expert Info (Comment/Comment): cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
>>>> ]
>>>>             [cpu=18 comm=ksoftirqd/18 pid=128 upcall_type=1 result=-11
>>>> ]
>>>>             [Severity level: Comment]
>>>>             [Group: Comment]
>>>> Frame 1: 1496 bytes on wire (11968 bits), 64 bytes captured (512 bits) on interface unknown, id 0
>>>>     Interface id: 0 (unknown)
>>>>         Interface name: unknown
>>>>                         ^^^^^^^
>>>>
>>>
>>> That's weird, I cannot reproduce it. How did you generate the failed
>>> upcall?
>>
>> Just start ovs_perf with 10k flows, which will bombard OVS. The port number is correct in the script output, just not in the pcap.
>>
>> This is my version of scapy (dont think I upgraded on the mean time ;)
>>
>> $ pip show scapy
>> Name: scapy
>> Version: 2.5.0
>> Summary: Scapy: interactive packet manipulation tool
>> Home-page: https://scapy.net
>> Author: Philippe BIONDI
>> Author-email: guillaume@valadon.net
>> License: GPL-2.0-only
>> Location: /usr/local/lib/python3.9/site-packages
>> Requires:
>> Required-by:
>>
>
> Ugh, this feature was introduced in scapy 2.6
>
> https://github.com/secdev/scapy/commit/56b4fa4adc6603b410c87c64a3ea3278ef69ca01
>
> Not much we can do about this other than adding the interface name to
> the comment just in case. WDYT?

Adding it to the comment for <scapy 2.6 (assuming we can get the scape version run time) would be a good change.

//Eelco

>>>>>> ---
>>>>>>  utilities/usdt-scripts/upcall_monitor.py | 53 +++++++++++++++++++-----
>>>>>>  1 file changed, 42 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py
>>>>>> index a1adeee0a..77378751f 100755
>>>>>> --- a/utilities/usdt-scripts/upcall_monitor.py
>>>>>> +++ b/utilities/usdt-scripts/upcall_monitor.py
>>>>>> @@ -118,7 +118,12 @@
>>>>>>
>>>>>>  from bcc import BPF, USDT, USDTException
>>>>>>  from os.path import exists
>>>>>> -from scapy.all import hexdump, wrpcap
>>>>>> +try:
>>>>>> +    # Try using pcapng support from scapy >= 2.4.
>>>>>> +    from scapy.all import hexdump, PcapNgWriter
>>>>>> +except ImportError:
>>>>>> +    from scapy.all import hexdump, wrpcap
>>>>>> +
>>>>>>  from scapy.layers.l2 import Ether
>>>>>>
>>>>>>  from usdt_lib import DpPortMapping
>>>>>> @@ -282,40 +287,48 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx)
>>>>>>  #endif
>>>>>>  """
>>>>>>
>>>>>> +pcap_writer = None
>>>>>> +
>>>>>>
>>>>>>  #
>>>>>>  # print_key()
>>>>>>  #
>>>>>>  def print_key(event, decode_dump):
>>>>>
>>>>> As this is no longer printing a key, I would change it to format_key().
>>>>>
>>>>>> +    lines = []
>>>>>>      if event.key_size < options.flow_key_size:
>>>>>>          key_len = event.key_size
>>>>>>      else:
>>>>>>          key_len = options.flow_key_size
>>>>>>
>>>>>>      if not key_len:
>>>>>> -        return
>>>>>> +        return []
>>>>>>
>>>>>>      if options.flow_key_decode != 'none':
>>>>>> -        print("  Flow key size {} bytes, size captured {} bytes.".
>>>>>> -              format(event.key_size, key_len))
>>>>>> +        lines.append("  Flow key size {} bytes, size captured {} bytes.".
>>>>>> +                     format(event.key_size, key_len))
>>>>>>
>>>>>>      if options.flow_key_decode == 'hex':
>>>>>>          #
>>>>>>          # Abuse scapy's hex dump to dump flow key
>>>>>>          #
>>>>>> -        print(re.sub('^', ' ' * 4, hexdump(Ether(bytes(event.key)[:key_len]),
>>>>>> -                                           dump=True),
>>>>>> -                     flags=re.MULTILINE))
>>>>>> +        lines.extend(re.sub('^', ' ' * 4,
>>>>>> +            hexdump(
>>>>>> +                Ether(bytes(event.key)[:key_len]),
>>>>>> +                dump=True),
>>>>>> +            flags=re.MULTILINE).split("\n"))
>>>>>>
>>>>>>      if options.flow_key_decode == "nlraw":
>>>>>> -        for line in decode_dump:
>>>>>> -            print(line)
>>>>>> +        lines.extend(decode_dump)
>>>>>> +
>>>>>> +    return lines
>>>>>>
>>>>>>
>>>>>>  #
>>>>>>  # print_event()
>>>>>>  #
>>>>>>  def print_event(ctx, data, size):
>>>>>> +    global pcap_writer
>>>>>> +
>>>>>>      event = b["events"].event(data)
>>>>>>      dp = event.dpif_name.decode("utf-8")
>>>>>>
>>>>>> @@ -350,7 +363,9 @@ def print_event(ctx, data, size):
>>>>>>      #
>>>>>>      # Dump flow key information
>>>>>>      #
>>>>>> -    print_key(event, key_dump)
>>>>>> +    key_lines = print_key(event, key_dump)
>>>>>> +    for line in key_lines:
>>>>>> +        print(line)
>>>>>>
>>>>>>      #
>>>>>>      # Decode packet only if there is data
>>>>>> @@ -383,7 +398,23 @@ 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)
>>>>>> +        try:
>>>>>> +            if pcap_writer is None:
>>>>>> +                pcap_writer = PcapNgWriter(options.pcap)
>>>>>> +
>>>>>> +            comment = "cpu={} comm={} pid={} upcall_type={} result={}". format(
>>>>>
>>>>> Adding the time stamp here might also be useful to “quickly” see the inter-packet gap.
>>>>>
>>>>>> +                event.cpu, event.comm.decode("utf-8"), event.pid,
>>>>>> +                event.upcall_type, event.result)
>>>>>> +
>>>>>> +            if options.flow_key_decode != 'none':
>>>>>> +                comment = comment + "\n" + "\n".join(key_lines)
>>>>>> +
>>>>>> +            packet.comment = comment
>>>>>> +            packet.sniffed_on = "{} ({})".format(port, dp)
>>>>>> +            pcap_writer.write(packet)
>>>>>> +        except NameError:  # PcapNgWriter not found
>>>>>> +            wrpcap(options.pcap, packet, append=True,
>>>>>> +                   snaplen=options.packet_size)
>>>>>>
>>>>>>
>>>>>>  #
>>>>>> --
>>>>>> 2.48.1
>>>>>>
>>>>>> _______________________________________________
>>>>>> dev mailing list
>>>>>> dev@openvswitch.org
>>>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>>
>>
diff mbox series

Patch

diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py
index a1adeee0a..77378751f 100755
--- a/utilities/usdt-scripts/upcall_monitor.py
+++ b/utilities/usdt-scripts/upcall_monitor.py
@@ -118,7 +118,12 @@ 
 
 from bcc import BPF, USDT, USDTException
 from os.path import exists
-from scapy.all import hexdump, wrpcap
+try:
+    # Try using pcapng support from scapy >= 2.4.
+    from scapy.all import hexdump, PcapNgWriter
+except ImportError:
+    from scapy.all import hexdump, wrpcap
+
 from scapy.layers.l2 import Ether
 
 from usdt_lib import DpPortMapping
@@ -282,40 +287,48 @@  int kretprobe__ovs_dp_upcall(struct pt_regs *ctx)
 #endif
 """
 
+pcap_writer = None
+
 
 #
 # print_key()
 #
 def print_key(event, decode_dump):
+    lines = []
     if event.key_size < options.flow_key_size:
         key_len = event.key_size
     else:
         key_len = options.flow_key_size
 
     if not key_len:
-        return
+        return []
 
     if options.flow_key_decode != 'none':
-        print("  Flow key size {} bytes, size captured {} bytes.".
-              format(event.key_size, key_len))
+        lines.append("  Flow key size {} bytes, size captured {} bytes.".
+                     format(event.key_size, key_len))
 
     if options.flow_key_decode == 'hex':
         #
         # Abuse scapy's hex dump to dump flow key
         #
-        print(re.sub('^', ' ' * 4, hexdump(Ether(bytes(event.key)[:key_len]),
-                                           dump=True),
-                     flags=re.MULTILINE))
+        lines.extend(re.sub('^', ' ' * 4,
+            hexdump(
+                Ether(bytes(event.key)[:key_len]),
+                dump=True),
+            flags=re.MULTILINE).split("\n"))
 
     if options.flow_key_decode == "nlraw":
-        for line in decode_dump:
-            print(line)
+        lines.extend(decode_dump)
+
+    return lines
 
 
 #
 # print_event()
 #
 def print_event(ctx, data, size):
+    global pcap_writer
+
     event = b["events"].event(data)
     dp = event.dpif_name.decode("utf-8")
 
@@ -350,7 +363,9 @@  def print_event(ctx, data, size):
     #
     # Dump flow key information
     #
-    print_key(event, key_dump)
+    key_lines = print_key(event, key_dump)
+    for line in key_lines:
+        print(line)
 
     #
     # Decode packet only if there is data
@@ -383,7 +398,23 @@  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)
+        try:
+            if pcap_writer is None:
+                pcap_writer = PcapNgWriter(options.pcap)
+
+            comment = "cpu={} comm={} pid={} upcall_type={} result={}". format(
+                event.cpu, event.comm.decode("utf-8"), event.pid,
+                event.upcall_type, event.result)
+
+            if options.flow_key_decode != 'none':
+                comment = comment + "\n" + "\n".join(key_lines)
+
+            packet.comment = comment
+            packet.sniffed_on = "{} ({})".format(port, dp)
+            pcap_writer.write(packet)
+        except NameError:  # PcapNgWriter not found
+            wrpcap(options.pcap, packet, append=True,
+                   snaplen=options.packet_size)
 
 
 #