Re: [PATCH bpf-next v4 1/6] selftests/bpf: Add traffic monitor functions.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/1/24 20:29, Martin KaFai Lau wrote:
On 7/31/24 12:31 PM, Kui-Feng Lee wrote:
Add functions that capture packets and print log in the background. They
are supposed to be used for debugging flaky network test cases. A monitored test case should call traffic_monitor_start() to start a thread to capture
packets in the background for a given namespace and call
traffic_monitor_stop() to stop capturing. (Or, option '-m' implemented by
the later patches.)

     IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 68, ifindex 1, SYN      IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 60, ifindex 1, SYN, ACK      IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 60, ifindex 1, ACK      IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 52, ifindex 1, ACK      IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 52, ifindex 1, FIN, ACK      IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 52, ifindex 1, RST, ACK

nit. Instead of ifindex, it should be ifname now.

Sure! I will update it.


     Packet file: packets-2172-86-select_reuseport:sockhash-test.log
     #280/87 select_reuseport/sockhash IPv4/TCP LOOPBACK test_detach_bpf:OK

The above is the output of an example. It shows the packets of a connection
and the name of the file that contains captured packets in the directory
/tmp/tmon_pcap. The file can be loaded by tcpdump or wireshark.

This feature only works if TRAFFIC_MONITOR variable has been passed to
build BPF selftests. For example,

   make TRAFFIC_MONITOR=1 -C tools/testing/selftests/bpf

This command will build BPF selftests with this feature enabled.

Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
---
  tools/testing/selftests/bpf/Makefile     |   5 +
  tools/testing/selftests/bpf/test_progs.c | 432 +++++++++++++++++++++++

In the cover letter, it mentioned the traffic monitoring implementation is moved from the network_helpers.c to test_progs.c.

Can you share more about the reason?

network_helpers.c has been used by several test programs.
However, they don't have env that we found in test_progs.c.
That means we could not access env directly. Instead, the caller
have to pass the test name and subtest name to the function.
Leter, we also need to check if a test name matches the patterns. It is
inconvient for users. So, I move these functions to test_progs.c to make
user's life eaiser.



Is it because the traffic monitor now depends on the test_progs's test name, should_tmon...etc ? Can the test name and should_tmon be exported for the network_helpers to use?

Yes! And in later patches, we also introduce a list of patterns.

What other compilation issues did it hit if the traffic monitor codes stay in the network_helpers.c? Some individual binaries (with main()) like test_tcp_check_syncookie_user that links to network_helpers.o but not to test_progs.o?

Yes, they are problems as well. These binary also need to link to
libpcap even they don't use it although this is not an important issue.



+#ifdef TRAFFIC_MONITOR
+struct tmonitor_ctx {
+    pcap_t *pcap;
+    pcap_dumper_t *dumper;
+    pthread_t thread;
+    int wake_fd_r;
+    int wake_fd_w;
+
+    bool done;
+    char pkt_fname[PATH_MAX];
+    int pcap_fd;
+};
+
+/* Is this packet captured with a Ethernet protocol type? */
+static bool is_ethernet(const u_char *packet)
+{
+    u16 arphdr_type;
+
+    memcpy(&arphdr_type, packet + 8, 2);
+    arphdr_type = ntohs(arphdr_type);
+
+    /* Except the following cases, the protocol type contains the
+     * Ethernet protocol type for the packet.
+     *
+     * https://www.tcpdump.org/linktypes/LINKTYPE_LINUX_SLL2.html
+     */
+    switch (arphdr_type) {
+    case 770: /* ARPHRD_FRAD */
+    case 778: /* ARPHDR_IPGRE */
+    case 803: /* ARPHRD_IEEE80211_RADIOTAP */
+        return false;
+    }
+    return true;
+}
+
+/* Show the information of the transport layer in the packet */
+static void show_transport(const u_char *packet, u16 len, u32 ifindex,
+               const char *src_addr, const char *dst_addr,
+               u16 proto, bool ipv6)
+{
+    struct udphdr *udp;
+    struct tcphdr *tcp;
+    u16 src_port, dst_port;
+    const char *transport_str;
+    char *ifname, _ifname[IF_NAMESIZE];
+
+    ifname = if_indextoname(ifindex, _ifname);
+    if (!ifname) {
+        snprintf(_ifname, sizeof(_ifname), "unknown(%d)", ifindex);
+        ifname = _ifname;
+    }
+
+    if (proto == IPPROTO_UDP) {
+        udp = (struct udphdr *)packet;
+        src_port = ntohs(udp->source);
+        dst_port = ntohs(udp->dest);
+        transport_str = "UDP";
+    } else if (proto == IPPROTO_TCP) {
+        tcp = (struct tcphdr *)packet;
+        src_port = ntohs(tcp->source);
+        dst_port = ntohs(tcp->dest);
+        transport_str = "TCP"
+;
+    } else if (proto == IPPROTO_ICMP) {
+        printf("IPv4 ICMP packet: %s -> %s, len %d, type %d, code %d, ifname %s\n",
+               src_addr, dst_addr, len, packet[0], packet[1], ifname);
+        return;
+    } else if (proto == IPPROTO_ICMPV6) {
+        printf("IPv6 ICMPv6 packet: %s -> %s, len %d, type %d, code %d, ifname %s\n",
+               src_addr, dst_addr, len, packet[0], packet[1], ifname);
+        return;
+    } else {
+        printf("%s (proto %d): %s -> %s, ifname %s\n",
+               ipv6 ? "IPv6" : "IPv4", proto, src_addr, dst_addr, ifname);
+        return;
+    }
+
+    /* TCP */
+
+    if (ipv6)
+        printf("IPv6 %s packet: [%s]:%d -> [%s]:%d, len %d, ifname %s",
+               transport_str, src_addr, src_port,
+               dst_addr, dst_port, len, ifname);
+    else
+        printf("IPv4 %s packet: %s:%d -> %s:%d, len %d, ifname %s",
+               transport_str, src_addr, src_port,
+               dst_addr, dst_port, len, ifname);
+
+    if (proto == IPPROTO_TCP) {
+        if (tcp->fin)
+            printf(", FIN");
+        if (tcp->syn)
+            printf(", SYN");
+        if (tcp->rst)
+            printf(", RST");
+        if (tcp->ack)
+            printf(", ACK");
+    }
+
+    printf("\n");

The TCP packet output is done by multiple printf. There is a good chance that one TCP packet is interleaved with the other printf(s), considering the traffic monitoring is done in its own thread. I am seeing this in the tc_redirect test.

Does it help to do multiple snprintf (for the tcp flags?) first and then calls one printf?

Sure, it would help. Or, perform flockfile() and funlockfile().


+}
+
+static void show_ipv6_packet(const u_char *packet, u32 ifindex)
+{
+    struct ipv6hdr *pkt = (struct ipv6hdr *)packet;
+    struct in6_addr src;
+    struct in6_addr dst;
+    char src_str[INET6_ADDRSTRLEN], dst_str[INET6_ADDRSTRLEN];
+    u_char proto;
+
+    memcpy(&src, &pkt->saddr, sizeof(src));
+    memcpy(&dst, &pkt->daddr, sizeof(dst));
+    inet_ntop(AF_INET6, &src, src_str, sizeof(src_str));
+    inet_ntop(AF_INET6, &dst, dst_str, sizeof(dst_str));
+    proto = pkt->nexthdr;
+    show_transport(packet + sizeof(struct ipv6hdr),
+               ntohs(pkt->payload_len),
+               ifindex, src_str, dst_str, proto, true);
+}
+
+static void show_ipv4_packet(const u_char *packet, u32 ifindex)
+{
+    struct iphdr *pkt = (struct iphdr *)packet;
+    struct in_addr src;
+    struct in_addr dst;
+    u_char proto;
+    char src_str[INET_ADDRSTRLEN], dst_str[INET_ADDRSTRLEN];
+
+    memcpy(&src, &pkt->saddr, sizeof(src));
+    memcpy(&dst, &pkt->daddr, sizeof(dst));
+    inet_ntop(AF_INET, &src, src_str, sizeof(src_str));
+    inet_ntop(AF_INET, &dst, dst_str, sizeof(dst_str));
+    proto = pkt->protocol;
+    show_transport(packet + sizeof(struct iphdr),
+               ntohs(pkt->tot_len),
+               ifindex, src_str, dst_str, proto, false);
+}
+
+static void *traffic_monitor_thread(void *arg)
+{
+    char *ifname, _ifname[IF_NAMESIZE];
+    const u_char *packet, *payload;
+    struct tmonitor_ctx *ctx = arg;
+    struct pcap_pkthdr header;
+    pcap_t *pcap = ctx->pcap;
+    pcap_dumper_t *dumper = ctx->dumper;
+    int fd = ctx->pcap_fd;
+    int wake_fd = ctx->wake_fd_r;
+    u16 proto;
+    u32 ifindex;
+    fd_set fds;
+    int nfds, r;
+
+    nfds = (fd > wake_fd ? fd : wake_fd) + 1;
+    FD_ZERO(&fds);
+
+    while (!ctx->done) {
+        FD_SET(fd, &fds);
+        FD_SET(wake_fd, &fds);
+        r = select(nfds, &fds, NULL, NULL, NULL);
+        if (!r)
+            continue;
+        if (r < 0) {
+            if (errno == EINTR)
+                continue;
+            log_err("Fail to select on pcap fd and wake fd: %s", strerror(errno));
+            break;
+        }
+
+        packet = pcap_next(pcap, &header);
+        if (!packet)
+            continue;
+
+        /* According to the man page of pcap_dump(), first argument
+         * is the pcap_dumper_t pointer even it's argument type is
+         * u_char *.
+         */
+        pcap_dump((u_char *)dumper, &header, packet);

The captured file has the "In", "Out", and "M" (Multicast) when it is read by tcpdump. Is it easy to printf that in show_ipv[46]_packet() also? Just want to ensure we didn't miss it if it is easy.

No problem! IIRC, it is part of the SSL2 header.


+
+        /* Not sure what other types of packets look like. Here, we
+         * parse only Ethernet and compatible packets.
+         */
+        if (!is_ethernet(packet)) {
+            printf("Packet captured\n");
+            continue;
+        }
+
+        /* Skip SLL2 header
+         * https://www.tcpdump.org/linktypes/LINKTYPE_LINUX_SLL2.html
+         *
+         * Although the document doesn't mention that, the payload
+         * doesn't include the Ethernet header. The payload starts
+         * from the first byte of the network layer header.
+         */
+        payload = packet + 20;
+
+        memcpy(&proto, packet, 2);
+        proto = ntohs(proto);
+        memcpy(&ifindex, packet + 4, 4);
+        ifindex = ntohl(ifindex);
+
+        if (proto == ETH_P_IPV6) {
+            show_ipv6_packet(payload, ifindex);
+        } else if (proto == ETH_P_IP) {
+            show_ipv4_packet(payload, ifindex);
+        } else {
+            ifname = if_indextoname(ifindex, _ifname);
+            if (!ifname) {
+                snprintf(_ifname, sizeof(_ifname), "unknown(%d)", ifindex);
+                ifname = _ifname;
+            }
+
+            printf("Unknown network protocol type %x, ifname %s\n", proto, ifname);
+        }
+    }
+
+    return NULL;
+}






[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux