Skip to content
This repository has been archived by the owner on Sep 20, 2022. It is now read-only.

sniffer: remove gnrc_netif header; read LQI #33

Merged
merged 1 commit into from
Jun 19, 2018

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jun 18, 2018

Since the device is in raw mode, we don't have any network layer
modules included and we subscribe to GNRC_NETTYPE_UNDEF it is safe
to assume that pkt->next in dump_pkt() is the gnrc_netif header.
This only contains GNRC-internal information and should thus be removed
from the dump (though Wireshark seems to be okay with the extra bytes,
a reader of the raw data might be confused).

Since this header however contains the LQI, which the sniffer claims to
output but always returns 0, the LQI value in the gnrc_netif header
is read and set before the deletion of that header.

Since the device is in raw mode, we don't have any network layer
modules included and we subscribe to `GNRC_NETTYPE_UNDEF` it is safe
to assume that `pkt->next` in `dump_pkt()` is the `gnrc_netif` header.
This only contains GNRC-internal information and should thus be removed
from the dump (though Wireshark seems to be okay with the extra bytes,
a reader of the raw data might be confused).

Since this header however contains the LQI, which the sniffer claims to
output but always returns 0, the LQI value in the `gnrc_netif` header
is read and set before the deletion of that header.
Copy link
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

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

untested ACK

@miri64
Copy link
Member Author

miri64 commented Jun 18, 2018

@cgundogan are you still planning to test?

@cgundogan
Copy link
Member

@cgundogan are you still planning to test?

@miri64 nope:

% ./sniffer.py serial /dev/ttyACM1 500000 17 | wireshark-gtk -k -i -
Traceback (most recent call last):
  File "./sniffer.py", line 171, in <module>
    main(sys.argv)
  File "./sniffer.py", line 149, in main
    configure_interface(conn, int(argv[4]))
  File "./sniffer.py", line 63, in configure_interface
    match = re.search(r'^Iface +(\d+)', line.decode())
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xf8 in position 2: invalid start byte

It probably is just a config error somewhere on my side, @pyropeter do you want to test? It seems to work for you.

@pyropeter
Copy link

I will test this.

By the way: I got the same error as @cgundogan until I lowered the baud rate to 115200.

@jnohlgard
Copy link
Member

Looks like the sniffer script needs better input sanitation for when there are garbage characters in the stream.

@miri64
Copy link
Member Author

miri64 commented Jun 19, 2018

Yeah... the LQI isn't even parsed by the script, so this PR shouldn't make the problem (and it doesn't make sense to test this PR with that script either then ;-)).

@miri64
Copy link
Member Author

miri64 commented Jun 19, 2018

(as a "try to get it to fail" test, yes, but not as functionality test. There it is enough if with make term is shown as a hex value now ;-))

@miri64
Copy link
Member Author

miri64 commented Jun 19, 2018

@cgundogan @pyropeter @gebart but regarding input sanitation: have a look at #35

@pyropeter
Copy link

This seems to work. The output shows some values as LQI, and wireshark still shows the traffic.

@cgundogan
Copy link
Member

@pyropeter thanks a bunch!

@cgundogan cgundogan merged commit 1d21756 into RIOT-OS:master Jun 19, 2018
@pyropeter
Copy link

To add LQI information to the packet dump, we should wait for IETF-OPSAWG-WG/draft-ietf-opsawg-pcap#48

@miri64 miri64 deleted the sniffer/fix/rm-netif_hdr branch June 19, 2018 16:34
@miri64
Copy link
Member Author

miri64 commented Jun 19, 2018

Question is, if developments to PCAPNG are backported to PCAP ;-)

chrysn pushed a commit to chrysn-pull-requests/RIOT that referenced this pull request Sep 20, 2022
…-netif_hdr

sniffer: remove gnrc_netif header; read LQI
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants