All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 1/2] net: introduce packet capture support
@ 2019-06-22 18:49 Ramon Fried
  2019-06-22 18:49 ` [U-Boot] [PATCH 2/2] doc: pcap: add pcap cmd documentation Ramon Fried
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ramon Fried @ 2019-06-22 18:49 UTC (permalink / raw)
  To: u-boot

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)
+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;
+}
-- 
2.22.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH 2/2] doc: pcap: add pcap cmd documentation
  2019-06-22 18:49 [U-Boot] [PATCH v3 1/2] net: introduce packet capture support Ramon Fried
@ 2019-06-22 18:49 ` Ramon Fried
  2019-07-11 21:13 ` [U-Boot] [PATCH v3 1/2] net: introduce packet capture support Joe Hershberger
  2019-07-11 21:18 ` Joe Hershberger
  2 siblings, 0 replies; 8+ messages in thread
From: Ramon Fried @ 2019-06-22 18:49 UTC (permalink / raw)
  To: u-boot

Add documentation for new "pcap" command.

Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
---
 doc/README.pcap | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 doc/README.pcap

diff --git a/doc/README.pcap b/doc/README.pcap
new file mode 100644
index 0000000000..25c85ae271
--- /dev/null
+++ b/doc/README.pcap
@@ -0,0 +1,61 @@
+PCAP:
+
+U-boot supports live ethernet packet capture in PCAP(2.4) format.
+This is enabled by CONFIG_CMD_PCAP.
+
+The capture is stored on physical memory, and should be copied to
+a machine capabale of parsing and displaying PCAP files (IE. wireshark)
+If networking works properly one can copy the capture file from physical memory
+using tftpput, or save it to local storage with (sf write, mmc write, fatwrite, etc)
+
+the pcap capturing requires maximum buffer size.
+when the buffer is full, packets will silently drop.
+check the status using "pcap status" to see if the buffer is full,
+if so, consider increasing the buffer size.
+
+Usage example:
+
+# Initialize pcap capture to physical address (0x100000) with maximum size of 100000.
+# Start capture
+pcap start
+
+# Initialize network activity
+env set ipaddr 10.0.2.15; env set serverip 10.0.2.2; tftp uImage64
+
+# Stop capture
+pcap stop
+
+# pcap init 0x100000 100000
+PCAP capture initialized: addr: 0xffffffff80100000 max length: 100000
+
+# pcap start
+# env set ipaddr 10.0.2.15; env set serverip 10.0.2.2; tftp uImage64
+eth0 at 10000000: PHY present at 0
+eth0 at 10000000: link up, 1000Mbps full-duplex (lpa: 0x7c00)
+Using eth0 at 10000000 device
+TFTP from server 10.0.2.2; our IP address is 10.0.2.15
+Filename 'uImage64'.
+Load address: 0xffffffff88000000
+Loading: #################################################################
+         #################################################################
+         #################################################################
+         #################################################################
+         #################################################################
+         #################################################################
+         #################################################################
+         #################################################################
+         #################################################################
+         #
+         18.2 MiB/s
+done
+Bytes transferred = 8359376 (7f8dd0 hex)
+PCAP status:
+        Initialized addr: 0xffffffff80100000    max length: 100000
+        Status: Active.  file size: 99991
+        Incoming packets: 66 Outgoing packets: 67
+        !!! Buffer is full, consider increasing buffer size !!!
+
+
+# pcap stop
+# tftpput 0xffffffff80100000 100000 10.0.2.2:capture.pcap
+
-- 
2.22.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v3 1/2] net: introduce packet capture support
  2019-06-22 18:49 [U-Boot] [PATCH v3 1/2] net: introduce packet capture support Ramon Fried
  2019-06-22 18:49 ` [U-Boot] [PATCH 2/2] doc: pcap: add pcap cmd documentation Ramon Fried
@ 2019-07-11 21:13 ` Joe Hershberger
  2019-07-15  7:07   ` Ramon Fried
  2019-07-11 21:18 ` Joe Hershberger
  2 siblings, 1 reply; 8+ messages in thread
From: Joe Hershberger @ 2019-07-11 21:13 UTC (permalink / raw)
  To: 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>
> ---
> 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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v3 1/2] net: introduce packet capture support
  2019-06-22 18:49 [U-Boot] [PATCH v3 1/2] net: introduce packet capture support Ramon Fried
  2019-06-22 18:49 ` [U-Boot] [PATCH 2/2] doc: pcap: add pcap cmd documentation Ramon Fried
  2019-07-11 21:13 ` [U-Boot] [PATCH v3 1/2] net: introduce packet capture support Joe Hershberger
@ 2019-07-11 21:18 ` Joe Hershberger
  2019-07-18 18:14   ` Ramon Fried
  2 siblings, 1 reply; 8+ messages in thread
From: Joe Hershberger @ 2019-07-11 21:18 UTC (permalink / raw)
  To: 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v3 1/2] net: introduce packet capture support
  2019-07-11 21:13 ` [U-Boot] [PATCH v3 1/2] net: introduce packet capture support Joe Hershberger
@ 2019-07-15  7:07   ` Ramon Fried
  2019-07-15 18:09     ` Joe Hershberger
  0 siblings, 1 reply; 8+ messages in thread
From: Ramon Fried @ 2019-07-15  7:07 UTC (permalink / raw)
  To: u-boot

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 at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v3 1/2] net: introduce packet capture support
  2019-07-15  7:07   ` Ramon Fried
@ 2019-07-15 18:09     ` Joe Hershberger
  0 siblings, 0 replies; 8+ messages in thread
From: Joe Hershberger @ 2019-07-15 18:09 UTC (permalink / raw)
  To: 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;
 [ ... ]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v3 1/2] net: introduce packet capture support
  2019-07-11 21:18 ` Joe Hershberger
@ 2019-07-18 18:14   ` Ramon Fried
  2019-07-18 21:24     ` Joe Hershberger
  0 siblings, 1 reply; 8+ messages in thread
From: Ramon Fried @ 2019-07-18 18:14 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v3 1/2] net: introduce packet capture support
  2019-07-18 18:14   ` Ramon Fried
@ 2019-07-18 21:24     ` Joe Hershberger
  0 siblings, 0 replies; 8+ messages in thread
From: Joe Hershberger @ 2019-07-18 21:24 UTC (permalink / raw)
  To: u-boot

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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-07-18 21:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-22 18:49 [U-Boot] [PATCH v3 1/2] net: introduce packet capture support Ramon Fried
2019-06-22 18:49 ` [U-Boot] [PATCH 2/2] doc: pcap: add pcap cmd documentation Ramon Fried
2019-07-11 21:13 ` [U-Boot] [PATCH v3 1/2] net: introduce packet capture support Joe Hershberger
2019-07-15  7:07   ` Ramon Fried
2019-07-15 18:09     ` Joe Hershberger
2019-07-11 21:18 ` Joe Hershberger
2019-07-18 18:14   ` Ramon Fried
2019-07-18 21:24     ` Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.