Opened 9 months ago

Closed 9 months ago

#18801 closed bug (fixed)

inconsistent L2 and L3 data for packets captured on the loopback interface

Reported by: dovsienko Owned by: axeld
Priority: normal Milestone: R1/beta5
Component: Network & Internet/Stack Version: R1/Development
Keywords: Cc: korli
Blocked By: Blocking:
Platform: All

Description

Using today's snapshot (hrev57581), I see that packets captured on an Ethernet interface arrive with the Ethernet header and with a sockaddr structure filled in as sockaddr_dl, both of which are expected (Ethernet header not shown below):

> ./tcpdump -n -i /dev/net/ipro1000/0 -c 1 -q
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on /dev/net/ipro1000/0, link-type EN10MB (Ethernet), snapshot length 262144 bytes
DEBUG: in pcap_read_haiku() fromLength == 60 bytesReceived == 270
DEBUG: sockaddr_dl.sdl_len == 60
DEBUG: sockaddr_dl.sdl_family == 4          <---- AF_LINK
DEBUG: sockaddr_dl.sdl_e_type == 0x0800     <---- ETHER_TYPE_IP
DEBUG: sockaddr_dl.sdl_index == 0xffffffff
DEBUG: sockaddr_dl.sdl_type == 6            <---- IFT_ETHER
DEBUG: sockaddr_dl.sdl_nlen == 0
DEBUG: sockaddr_dl.sdl_alen == 6            <---- ETHER_ADDRESS_LENGTH
DEBUG: sockaddr_dl.sdl_slen == 0
06:55:28.787210 IP 192.168.0.25.22 > 192.168.0.18.42948: tcp 204

However, packets captured on the loopback interface also have an Ethernet header with all-zeroes addresses, even though the interface is purely level 3 (I suppose loopback interfaces are not going to support Ethernet, AppleTalk, IPX or Bluetooth). Even more so, the sockaddr structure is filled as sockaddr_in:

> ./tcpdump -n -i loop -c 1 -e
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on loop, link-type EN10MB (Ethernet), snapshot length 262144 bytes
DEBUG: in pcap_read_haiku() fromLength == 32 bytesReceived == 98
DEBUG: sockaddr_in.sin_len == 32
DEBUG: sockaddr_in.sin_family == 1          <---- AF_INET
DEBUG: sockaddr_in.sin_port == 0
DEBUG: sockaddr_in.sin_addr == 0x7f000001   <---- INADDR_LOOPBACK
07:12:28.727572 00:00:00:00:00:00 > 00:00:00:00:00:00, ethertype IPv4 (0x0800), length 98: 127.0.0.1 > 127.0.0.1: ICMP echo request, id 53444, seq 21459, length 64

Currently libpcap on Haiku uses DLT_EN10MB for all network interfaces, supposedly because this is what de facto comes out. Arguably, this solution space does not fit the problem space well. It seems to me, it would be a better choice to deliver L3 packets (plus correct sockaddr metadata) from a loopback interface capture, then libpcap could process the packets as either DLT_RAW, DLT_LOOP or DLT_NULL (more detail) and the library users would see the result similar to this:

root@freebsd-aarch64-3:~ # tcpdump -i lo0 -L
Data link types for lo0 (use option -y to set):
  NULL (BSD loopback)

openbsd-aarch64# tcpdump -i lo0 -L 
1 link type supported:
	LOOP

This reproduces using the master branch of libpcap at commit 3b0edbc with the following debug code:

--- a/pcap-haiku.c
+++ b/pcap-haiku.c
@@ -63,6 +63,28 @@ pcap_read_haiku(pcap_t* handle, int maxPackets _U_, pcap_handler callback,
                socklen_t fromLength = sizeof(from);
                bytesReceived = recvfrom(handle->fd, buffer, handle->bufsize, MSG_TRUNC,
                        (struct sockaddr*)&from, &fromLength);
+
+               printf ("DEBUG: in %s() fromLength == %u bytesReceived == %zu\n", __func__, fromLength, bytesReceived);
+               switch (from.sdl_family) {
+               case AF_LINK:
+                       printf ("DEBUG: sockaddr_dl.sdl_len == %u\n", from.sdl_len);
+                       printf ("DEBUG: sockaddr_dl.sdl_family == %u\n", from.sdl_family);
+                       printf ("DEBUG: sockaddr_dl.sdl_e_type == 0x%04x\n", ntohs(from.sdl_e_type));
+                       printf ("DEBUG: sockaddr_dl.sdl_index == 0x%x\n", from.sdl_index);
+                       printf ("DEBUG: sockaddr_dl.sdl_type == %u\n", from.sdl_type);
+                       printf ("DEBUG: sockaddr_dl.sdl_nlen == %u\n", from.sdl_nlen);
+                       printf ("DEBUG: sockaddr_dl.sdl_alen == %u\n", from.sdl_alen);
+                       printf ("DEBUG: sockaddr_dl.sdl_slen == %u\n", from.sdl_slen);
+                       break;
+               case AF_INET:
+                       struct sockaddr_in *from_inet = (struct sockaddr_in *)&from;
+                       printf ("DEBUG: sockaddr_in.sin_len == %u\n", from_inet->sin_len);
+                       printf ("DEBUG: sockaddr_in.sin_family == %u\n", from_inet->sin_family);
+                       printf ("DEBUG: sockaddr_in.sin_port == %u\n", from_inet->sin_port);
+                       printf ("DEBUG: sockaddr_in.sin_addr == 0x%08x\n", ntohl(from_inet->sin_addr.s_addr));
+                       break;
+               }
+
        } while (bytesReceived < 0 && errno == B_INTERRUPTED);
 
        if (bytesReceived < 0) {

Attachments (2)

Screenshot from 2024-02-15 17-49-32.png (91.8 KB ) - added by korli 9 months ago.
DLT_NULL
Screenshot from 2024-02-15 17-51-29.png (102.5 KB ) - added by korli 9 months ago.
DLT_LOOP

Download all attachments as: .zip

Change History (11)

comment:1 by korli, 9 months ago

Thanks for the detailed report. Any chance to check with r1beta4 if it works as expected?

If yes, we could first patch libpcap according to this report, and then fix on the haiku master branch to behave like r1beta4.

comment:2 by dovsienko, 9 months ago

Thank you for the follow-up.

On the latest r1beta4 the loopback interface allows to start a packet capture, but does not deliver any packets. On IRC waddlesplash explained that he fixed that bug in the development branch via commit 4e965302, so I have switched my Haiku VM to the latest nightly build and packets indeed appeared on the capture socket, which made other imperfections visible. Then he suggested making a bug report.

It would be nice to get Haiku and libpcap code just right at both ends of the capture socket instead of maintaining workarounds at both ends. I am one of the maintainers of libpcap and if you have time to work on this now, I also have time to work on this now.

comment:3 by waddlesplash, 9 months ago

On IRC waddlesplash explained that he fixed that bug in the development branch via commit 4e965302

Ah, I just committed this change, korli was the one to write it.

Deleting the code to add ethernet headers from there, as well as from the loopback frame handler (and then adjusting TUN to match) should be all that's needed?

comment:4 by korli, 9 months ago

I tried your suggestion, switching to DLT_NULL or DLT_LOOP for the loopback, while using the patch from waddlesplash: https://review.haiku-os.org/c/haiku/+/7403 The packets don't get recognized.

by korli, 9 months ago

DLT_NULL

by korli, 9 months ago

DLT_LOOP

comment:5 by dovsienko, 9 months ago

Thank you, this requires some changes at the libpcap end to present the correct DLT and packet framing to tcpdump. Let me try and see what else gets in the way.

comment:6 by korli, 9 months ago

Comparing with Ubuntu, tcpdump still uses EN10MB on the loopback. Difficult to compare the packets then.

comment:7 by korli, 9 months ago

Patching the actual libpcap version at Haikuports:

diff --git a/pcap-haiku.cpp b/pcap-haiku.cpp
index 8ae9119..59f15b9 100644
--- a/pcap-haiku.cpp
+++ b/pcap-haiku.cpp
@@ -189,7 +189,7 @@ pcap_activate_haiku(pcap_t *handle)
        }
 
        handle->offset = 0;
-       handle->linktype = DLT_EN10MB;
+       //handle->linktype = DLT_EN10MB;
        // TODO: check interface type!
 
        return 0;
@@ -240,6 +240,16 @@ pcap_create_interface(const char *device, char *errorBuffer)
                return NULL;
        }
 
+       // get address
+       if (ioctl(socket, SIOCGIFADDR, &request, sizeof(struct ifreq)) < 0) {
+               snprintf(errorBuffer, PCAP_ERRBUF_SIZE, "Cannot get address: %s\n",
+                       strerror(errno));
+               close(socket);
+               return NULL;
+       }
+
+       uint8 sdl_type = ((sockaddr_dl&)request.ifr_addr).sdl_type;
+
        // start monitoring
        if (ioctl(socket, SIOCSPACKETCAP, &request, sizeof(struct ifreq)) < 0) {
                snprintf(errorBuffer, PCAP_ERRBUF_SIZE, "Cannot start monitoring: %s\n",
@@ -261,6 +271,7 @@ pcap_create_interface(const char *device, char *errorBuffer)
 
        handle->selectable_fd = socket;
        handle->fd = socket;
+       handle->linktype = sdl_type == IFT_LOOP ? DLT_LOOP : DLT_EN10MB;
 
        handle->activate_op = pcap_activate_haiku;
 

comment:8 by dovsienko, 9 months ago

The DLT_NULL and DLT_LOOP cases above did not work because the packet framing was not right: the packet data still had the Ethernet header, but tcpdump expected a 4-byte header followed by the IPv4/IPv6 header. The recent revision waddlesplash prepared delivers raw IPv4/IPv6 packets for the loopback and tun mode tunnels, which fits DLT_RAW immediately, let's keep it this way.

Thank you for suggesting these changes, I tried to use SIOCGIFADDR in my working copy, but it did not immediately work because not all ioctl() get requests work for all AFs. After some debugging I managed to get it right, and this works for Ethernet, loopback and both types of tunnels using OpenVPN, please use the latest revision of libpcap master branch for testing and settling the changes in Haiku.

One thing that still does not make sense is that SIOCGIFTYPE rejects AF_LINK sockets, and for AF_INET sockets always sets ifr_type to 0. Another is that SIOCGIFSTATS works on AF_INET sockets, but rejects AF_LINK. Also tap tunnel interfaces deliver multicast, broadcast and most Rx packets with incorrect sockaddr data. None of that is a blocker though.

comment:9 by waddlesplash, 9 months ago

Milestone: UnscheduledR1/beta5
Resolution: fixed
Status: newclosed

Fixed in hrev57585. Thanks for the detailed report, and for working on Haiku support in libpcap! It's much appreciated.

Note: See TracTickets for help on using tickets.