| Message ID | 20190622184950.16454-1-rfried.dev@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Joe Hershberger |
| Headers | show |
| Series | [U-Boot,v3,1/2] net: introduce packet capture support | expand |
On Sat, Jun 22, 2019 at 1:50 PM Ramon Fried <rfried.dev@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@gmail.com> > Reviewed-by: Alex Marginean <alexm.osslist@gmail.com> > Tested-by: Alex Marginean <alexm.osslist@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. > +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 > + > +/** > + * 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(). > +#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. > 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@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. > + } > + > + 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. 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@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Sat, Jun 22, 2019 at 1:50 PM Ramon Fried <rfried.dev@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@gmail.com> > Reviewed-by: Alex Marginean <alexm.osslist@gmail.com> > Tested-by: Alex Marginean <alexm.osslist@gmail.com> This is a nice feature to have. We need a unit test for it, though. Please add it to a sandbox test and exercise it, then have the test try to load the resulting file with tshark to ensure a valid format. There are potentially other things in the header that is printed that can be used for further validation. Thanks for this feature. -Joe
On Fri, Jul 12, 2019 at 12:18 AM Joe Hershberger <joe.hershberger@ni.com> wrote: > On Sat, Jun 22, 2019 at 1:50 PM Ramon Fried <rfried.dev@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@gmail.com> >> Reviewed-by: Alex Marginean <alexm.osslist@gmail.com> >> Tested-by: Alex Marginean <alexm.osslist@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@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@lists.denx.de >> https://lists.denx.de/listinfo/u-boot
On Mon, Jul 15, 2019 at 2:08 AM Ramon Fried <rfried.dev@gmail.com> wrote: > > On Fri, Jul 12, 2019 at 12:18 AM Joe Hershberger <joe.hershberger@ni.com> wrote: > > On Sat, Jun 22, 2019 at 1:50 PM Ramon Fried <rfried.dev@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@gmail.com> > >> Reviewed-by: Alex Marginean <alexm.osslist@gmail.com> > >> Tested-by: Alex Marginean <alexm.osslist@gmail.com> [ ... ] > > 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 ? I think this should be an uncommon enough of an issue that it's OK to break up the #####. > >> + } > >> + > >> + 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; [ ... ]
On Fri, Jul 12, 2019 at 12:18 AM Joe Hershberger <joe.hershberger@ni.com> wrote: > > On Sat, Jun 22, 2019 at 1:50 PM Ramon Fried <rfried.dev@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@gmail.com> > > Reviewed-by: Alex Marginean <alexm.osslist@gmail.com> > > Tested-by: Alex Marginean <alexm.osslist@gmail.com> > > This is a nice feature to have. We need a unit test for it, though. > Please add it to a sandbox test and exercise it, then have the test > try to load the resulting file with tshark to ensure a valid format. > There are potentially other things in the header that is printed that > can be used for further validation. Hi Joe, I fixed all your notes regarding the patches, except the test. I thought I'll find some net framework to start with, but I didn't find any tests under u-boot/tests/* that actually transfer a file between the host and u-boot sandbox. Did I miss anything ? I did find something interesting under u-boot/api/api_net.c that can perhaps be leveraged to something like that. Do you know how to begin with a test you described above ? Meanwhile, I posted the updated patches (without the test) so they can be reviewed by you. Thanks, Ramon. > > Thanks for this feature. > -Joe
On Thu, Jul 18, 2019 at 1:14 PM Ramon Fried <rfried.dev@gmail.com> wrote: > > On Fri, Jul 12, 2019 at 12:18 AM Joe Hershberger <joe.hershberger@ni.com> wrote: > > > > On Sat, Jun 22, 2019 at 1:50 PM Ramon Fried <rfried.dev@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@gmail.com> > > > Reviewed-by: Alex Marginean <alexm.osslist@gmail.com> > > > Tested-by: Alex Marginean <alexm.osslist@gmail.com> > > > > This is a nice feature to have. We need a unit test for it, though. > > Please add it to a sandbox test and exercise it, then have the test > > try to load the resulting file with tshark to ensure a valid format. > > There are potentially other things in the header that is printed that > > can be used for further validation. > Hi Joe, > I fixed all your notes regarding the patches, except the test. > I thought I'll find some net framework to start with, but I didn't find > any tests under u-boot/tests/* that actually transfer a file between the > host and u-boot sandbox. > Did I miss anything ? I'm not sure if it's appropriate but maybe a sandbox test like test/dm/eth.c. > I did find something interesting under u-boot/api/api_net.c that can perhaps > be leveraged to something like that. Probably not, but maybe if you elaborate a bit more about what you have in mind. > Do you know how to begin with a test you described above ? > Meanwhile, I posted the updated patches (without the test) so they can > be reviewed by you. > > Thanks, > Ramon. > > > > Thanks for this feature. > > -Joe > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
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) +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 */ + +/** + * 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); +#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 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@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; + } + + 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; + + 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; +}