[U-Boot] [PATCH v3 1/2] net: introduce packet capture support
Ramon Fried
rfried.dev at gmail.com
Mon Jul 15 07:07:51 UTC 2019
On Fri, Jul 12, 2019 at 12:18 AM Joe Hershberger <joe.hershberger at ni.com> wrote:
> On Sat, Jun 22, 2019 at 1:50 PM Ramon Fried <rfried.dev at gmail.com> wrote:
>> Add support for capturing ethernet packets and storing
>> them in memory in PCAP(2.4) format, later to be analyzed by
>> any PCAP viewer software (IE. Wireshark)
>>
>> This feature greatly assist debugging network issues such
>> as detecting dropped packets, packet corruption etc.
>>
>> Signed-off-by: Ramon Fried <rfried.dev at gmail.com>
>> Reviewed-by: Alex Marginean <alexm.osslist at gmail.com>
>> Tested-by: Alex Marginean <alexm.osslist at gmail.com>
>> ---
>> v2: Fix type assignmnet to header.ts_sec
>> v3: Several suggestion changes by Bin and Alex:
>> * Change to implementation to command based.
>> * Added counters for incoming/outgoing packets.
>> * Support clearing the capture for multiple captures.
>> * Added documentation (separate patch).
>>
>> cmd/Kconfig | 7 +++
>> cmd/net.c | 63 +++++++++++++++++++++
>> include/net.h | 56 +++++++++++++++++++
>> net/Makefile | 1 +
>> net/net.c | 8 +++
>> net/pcap.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 287 insertions(+)
>> create mode 100644 net/pcap.c
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 0badcb3fe0..638f979f28 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -1239,6 +1239,13 @@ config BOOTP_NTPSERVER
>> bool "Request & store 'ntpserverip' from BOOTP/DHCP server"
>> depends on CMD_BOOTP
>>
>> +config CMD_PCAP
>> + bool "pcap capture"
>> + help
>> + Selecting this will allow capturing all Ethernet packets and store
>> + them in physical memory in a PCAP formated file,
>> + later to be analyzed by PCAP reader application (IE. WireShark).
>> +
>> config BOOTP_PXE
>> bool "Send PXE client arch to BOOTP/DHCP server"
>> default y
>> diff --git a/cmd/net.c b/cmd/net.c
>> index 89721b8f8b..5022f1dbcc 100644
>> --- a/cmd/net.c
>> +++ b/cmd/net.c
>> @@ -457,3 +457,66 @@ U_BOOT_CMD(
>> );
>>
>> #endif /* CONFIG_CMD_LINK_LOCAL */
>> +
>> +#if defined(CONFIG_CMD_PCAP)
>
> Please just move this to a separate cmd/pcap.c instead of this ifdef.
NP
>> +static int do_pcap_init(cmd_tbl_t *cmdtp, int flag, int argc,
>> + char * const argv[])
>> +{
>> + phys_addr_t addr;
>> + unsigned int size;
>> +
>> + if (argc != 3)
>> + return CMD_RET_USAGE;
>> +
>> + addr = simple_strtoul(argv[1], NULL, 16);
>> + size = simple_strtoul(argv[2], NULL, 10);
>> +
>> + return pcap_init(addr, size) ? CMD_RET_FAILURE : CMD_RET_SUCCESS;
>> +}
>> +
>> +static int do_pcap_start(cmd_tbl_t *cmdtp, int flag, int argc,
>> + char * const argv[])
>> +{
>> + return pcap_start_stop(true) ? CMD_RET_FAILURE : CMD_RET_SUCCESS;
>> +}
>> +
>> +static int do_pcap_stop(cmd_tbl_t *cmdtp, int flag, int argc,
>> + char * const argv[])
>> +{
>> + return pcap_start_stop(false) ? CMD_RET_FAILURE : CMD_RET_SUCCESS;
>> +}
>> +
>> +static int do_pcap_status(cmd_tbl_t *cmdtp, int flag, int argc,
>> + char * const argv[])
>> +{
>> + return pcap_print_status() ? CMD_RET_FAILURE : CMD_RET_SUCCESS;
>> +}
>> +
>> +static int do_pcap_clear(cmd_tbl_t *cmdtp, int flag, int argc,
>> + char * const argv[])
>> +{
>> + return pcap_clear() ? CMD_RET_FAILURE : CMD_RET_SUCCESS;
>> +}
>> +
>> +static char pcap_help_text[] =
>> + "- network packet capture\n\n"
>> + "pcap\n"
>> + "pcap init\t\t\t<addr> <max_size>\n"
>> + "pcap start\t\t\tstart capture\n"
>> + "pcap stop\t\t\tstop capture\n"
>> + "pcap status\t\t\tprint status\n"
>> + "pcap clear\t\t\tclear capture buffer\n"
>> + "\n"
>> + "With:\n"
>> + "\t<addr>: user address to which pcap will be stored (hexedcimal)\n"
>> + "\t<max_size>: Maximum size of pcap file (decimal)\n"
>> + "\n";
>> +
>> +U_BOOT_CMD_WITH_SUBCMDS(pcap, "pcap", pcap_help_text,
>> + U_BOOT_SUBCMD_MKENT(init, 3, 0, do_pcap_init),
>> + U_BOOT_SUBCMD_MKENT(start, 1, 0, do_pcap_start),
>> + U_BOOT_SUBCMD_MKENT(stop, 1, 0, do_pcap_stop),
>> + U_BOOT_SUBCMD_MKENT(status, 1, 0, do_pcap_status),
>> + U_BOOT_SUBCMD_MKENT(clear, 1, 0, do_pcap_clear),
>> +);
>> +#endif /* CONFIG_CMD_PCAP */
>> diff --git a/include/net.h b/include/net.h
>> index 44b32385c4..f0289c3cde 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -630,6 +630,58 @@ bool arp_is_waiting(void); /* Waiting for ARP reply? */
>> void net_set_icmp_handler(rxhand_icmp_f *f); /* Set ICMP RX handler */
>> void net_set_timeout_handler(ulong, thand_f *);/* Set timeout handler */
>>
>> +/* PCAP extension */
>
> Please put this stuff in include/net/pcap.h
NP
>> +
>> +/**
>> + * pcap_init() - Initialize PCAP memory buffer
>> + *
>> + * @paddr physicaly memory address to store buffer
>> + * @size maximum size of capture file in memory
>> + *
>> + * @return 0 on success, -ERROR on error
>> + */
>> +int pcap_init(phys_addr_t paddr, unsigned long size);
>> +
>> +/**
>> + * pcap_start_stop() - start / stop pcap capture
>> + *
>> + * @start if true, start capture if false stop capture
>> + *
>> + * @return 0 on success, -ERROR on error
>> + */
>> +int pcap_start_stop(bool start);
>> +
>> +/**
>> + * pcap_clear() - clear pcap capture buffer and statistics
>> + *
>> + * @return 0 on success, -ERROR on error
>> + */
>> +int pcap_clear(void);
>> +
>> +/**
>> + * pcap_print_status() - print status of pcap capture
>> + *
>> + * @return 0 on success, -ERROR on error
>> + */
>> +int pcap_print_status(void);
>> +
>> +/**
>> + * pcap_active() - check if pcap is enabled
>> + *
>> + * @return TRUE if active, FALSE if not.
>> + */
>> +bool pcap_active(void);
>> +
>> +/**
>> + * pcap_post() - Post a packet to PCAP file
>> + *
>> + * @packet: packet to post
>> + * @len: packet length in bytes
>> + * @outgoing packet direction (outgoing/incoming)
>> + * @return 0 on success, -ERROR on error
>> + */
>> +int pcap_post(const void *packet, size_t len, bool outgoing);
>> +
>> /* Network loop state */
>> enum net_loop_state {
>> NETLOOP_CONTINUE,
>> @@ -658,6 +710,10 @@ static inline void net_send_packet(uchar *pkt, int len)
>> {
>> /* Currently no way to return errors from eth_send() */
>> (void) eth_send(pkt, len);
>> +
>> +#if defined(CONFIG_CMD_PCAP)
>> + pcap_post(pkt, len, true);
>
> Maybe move this call inside of eth_send().
NP
>> +#endif
>> }
>>
>> /*
>> diff --git a/net/Makefile b/net/Makefile
>> index ce36362168..d70a7c6834 100644
>> --- a/net/Makefile
>> +++ b/net/Makefile
>> @@ -20,6 +20,7 @@ obj-$(CONFIG_CMD_LINK_LOCAL) += link_local.o
>> obj-$(CONFIG_NET) += net.o
>> obj-$(CONFIG_CMD_NFS) += nfs.o
>> obj-$(CONFIG_CMD_PING) += ping.o
>> +obj-$(CONFIG_CMD_PCAP) += pcap.o
>
> Please maintain alpha order.
NP
>> obj-$(CONFIG_CMD_RARP) += rarp.o
>> obj-$(CONFIG_CMD_SNTP) += sntp.o
>> obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
>> diff --git a/net/net.c b/net/net.c
>> index 58b0417cbe..b6b5a49153 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -671,6 +671,11 @@ done:
>> net_set_icmp_handler(NULL);
>> #endif
>> net_set_state(prev_net_state);
>> +
>> +#if defined(CONFIG_CMD_PCAP)
>> + if (pcap_active())
>> + pcap_print_status();
>> +#endif
>> return ret;
>> }
>>
>> @@ -1083,6 +1088,9 @@ void net_process_received_packet(uchar *in_packet, int len)
>>
>> debug_cond(DEBUG_NET_PKT, "packet received\n");
>>
>> +#if defined(CONFIG_CMD_PCAP)
>> + pcap_post(in_packet, len, false);
>> +#endif
>> net_rx_packet = in_packet;
>> net_rx_packet_len = len;
>> et = (struct ethernet_hdr *)in_packet;
>> diff --git a/net/pcap.c b/net/pcap.c
>> new file mode 100644
>> index 0000000000..071bfeb510
>> --- /dev/null
>> +++ b/net/pcap.c
>> @@ -0,0 +1,152 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright 2019 Ramon Fried <rfried.dev at gmail.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <net.h>
>> +#include <time.h>
>> +#include <asm/io.h>
>> +
>> +#define LINKTYPE_ETHERNET 1
>> +
>> +static bool initialized;
>> +static bool running;
>> +static bool buffer_full;
>> +static void *buf;
>> +static unsigned int max_size;
>> +static unsigned int pos;
>> +
>> +static unsigned long incoming_count;
>> +static unsigned long outgoing_count;
>> +
>> +struct pcap_header {
>> + u32 magic;
>> + u16 version_major;
>> + u16 version_minor;
>> + s32 thiszone;
>> + u32 sigfigs;
>> + u32 snaplen;
>> + u32 network;
>> +};
>> +
>> +struct pcap_packet_header {
>> + u32 ts_sec;
>> + u32 ts_usec;
>> + u32 incl_len;
>> + u32 orig_len;
>> +};
>> +
>> +static struct pcap_header file_header = {
>> + .magic = 0xa1b2c3d4,
>> + .version_major = 2,
>> + .version_minor = 4,
>> + .snaplen = 65535,
>> + .network = LINKTYPE_ETHERNET,
>> +};
>> +
>> +int pcap_init(phys_addr_t paddr, unsigned long size)
>> +{
>> + buf = map_physmem(paddr, size, 0);
>> + if (!buf) {
>> + printf("Failed mapping PCAP memory\n");
>> + return -ENOMEM;
>> + }
>> +
>> + printf("PCAP capture initialized: addr: 0x%lx max length: %lu\n",
>> + (unsigned long)buf, size);
>> +
>> + memcpy(buf, &file_header, sizeof(file_header));
>> + pos = sizeof(file_header);
>> + max_size = size;
>> + initialized = true;
>> + running = false;
>> + buffer_full = false;
>> + incoming_count = 0;
>> + outgoing_count = 0;
>> + return 0;
>> +}
>> +
>> +int pcap_start_stop(bool start)
>> +{
>> + if (!initialized) {
>> + printf("error: pcap was not initialized\n");
>> + return -ENODEV;
>> + }
>> +
>> + running = start;
>> +
>> + return 0;
>> +}
>> +
>> +int pcap_clear(void)
>> +{
>> + if (!initialized) {
>> + printf("error: pcap was not initialized\n");
>> + return -ENODEV;
>> + }
>> +
>> + pos = sizeof(file_header);
>> + incoming_count = 0;
>> + outgoing_count = 0;
>> + buffer_full = false;
>> +
>> + printf("pcap capture cleared\n");
>> + return 0;
>> +}
>> +
>> +int pcap_post(const void *packet, size_t len, bool outgoing)
>> +{
>> + struct pcap_packet_header header;
>> + u64 cur_time = timer_get_us();
>> +
>> + if (!initialized || !running || !buf)
>> + return -ENODEV;
>> +
>> + if ((pos + len + sizeof(header)) >= max_size) {
>> + buffer_full = true;
>> + return -ENOMEM;
>
> It seems like an error should be printed the first time the limit is
> hit, not just wait until a status is requested.
Printing error on the first time was my initial implementation.
problem is that it's printed along with other ####### that tftp uses.
This doesn't look nice. what do you think ?
>> + }
>> +
>> + header.ts_sec = cur_time / 1000000;
>> + header.ts_usec = cur_time % 1000000;
>> + header.incl_len = len;
>> + header.orig_len = len;
>> +
>> + memcpy(buf + pos, &header, sizeof(header));
>> + pos += sizeof(header);
>> + memcpy(buf + pos, packet, len);
>> + pos += len;
>
> You should also provide an env variable of the size so that users can
> use that to write to a file.
That's really nice, didn't think about it.
> Something like: env_set_hex("pcapsize", pos);
>
>> +
>> + if (outgoing)
>> + outgoing_count++;
>> + else
>> + incoming_count++;
>> +
>> + return 0;
>> +}
>> +
>> +int pcap_print_status(void)
>> +{
>> + if (!initialized) {
>> + printf("pcap was not initialized\n");
>> + return -ENODEV;
>> + }
>> + printf("PCAP status:\n");
>> + printf("\tInitialized addr: 0x%lx\tmax length: %u\n",
>> + (unsigned long)buf, max_size);
>> + printf("\tStatus: %s.\t file size: %u\n", running ? "Active" : "Idle",
>> + pos);
>> + printf("\tIncoming packets: %lu Outgoing packets: %lu\n",
>> + incoming_count, outgoing_count);
>> +
>> + if (buffer_full)
>> + printf("\t!!! Buffer is full, consider increasing buffer size !!!\n");
>> +
>> + return 0;
>> +}
>> +
>> +bool pcap_active(void)
>> +{
>> + return running;
>> +}
>> --
>> 2.22.0
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
More information about the U-Boot
mailing list