[v7,4/4] test-pmd: add more packet verbose decode options
Checks
Commit Message
The existing verbose levels 1..3 provide a messy multi-line
output per packet. I found this unhelpful when diagnosing many
types of problems like packet flow.
This patch keeps the previous levels and adds two new levels:
4: one line per packet is printed in a format resembling
tshark output. With addresses and protocol info.
5: dump packet in hex.
Useful if the driver is messing up the data.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
app/test-pmd/cmdline_flow.c | 3 +-
app/test-pmd/config.c | 33 ++++++---
app/test-pmd/testpmd.h | 11 +++
app/test-pmd/util.c | 77 +++++++++++++++++++--
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 5 +-
5 files changed, 111 insertions(+), 18 deletions(-)
Comments
Hi Stephen,
I have gone through your patch series and the hexdump option would be
quite valuable for use in DTS.
However I am currently facing the issue of distinguishing noise packets
from intentional packets within the verbose output. Prior to your patch,
the intention was to use the Layer 4 port to distinguish between them,
however with the hexdump option, the plan is to now use a custom payload.
The one issue is that with verbose level 5 does not print the required
RSS hash and RSS queue information.
Would it be possible for you to add this to your patch series?
Otherwise, do you have any ideas and/or solutions to tackle this
specific problem?
On 8/2/24 20:56, Stephen Hemminger wrote:
<snip>
> +static uint16_t
> +dump_pkt_burst(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
> + uint16_t nb_pkts, int is_rx)
> +{
> + if (unlikely(nb_pkts == 0))
> + return 0;
> +
> + switch (verbose_level) {
> + case VERBOSE_RX ... VERBOSE_BOTH:
> + dump_pkt_verbose(port_id, queue, pkts, nb_pkts, is_rx);
> + break;
> + case VERBOSE_DISSECT:
> + dump_pkt_brief(port_id, queue, pkts, nb_pkts, is_rx);
> + break;
> + case VERBOSE_HEX:
> + dump_pkt_hex(pkts, nb_pkts);
> + }
> + fflush(stdout);
> + return nb_pkts;
> +}
I have identified the function above that would require altering in
order to accommodate the additional RSS information.
Thanks,
Alex
On Tue, 20 Aug 2024 14:42:56 +0100
Alex Chapman <alex.chapman@arm.com> wrote:
> Hi Stephen,
>
> I have gone through your patch series and the hexdump option would be
> quite valuable for use in DTS.
>
> However I am currently facing the issue of distinguishing noise packets
> from intentional packets within the verbose output. Prior to your patch,
> the intention was to use the Layer 4 port to distinguish between them,
> however with the hexdump option, the plan is to now use a custom payload.
>
> The one issue is that with verbose level 5 does not print the required
> RSS hash and RSS queue information.
The queue is there, in the output. Not sure if the hash matters.
I wanted to keep to tshark format as much as possible.
For DTS, maybe having a yet another JSON verbose format would be best.
Something with mbuf bits in JSON.
On 20/08/2024 16:54, Stephen Hemminger wrote:
> On Tue, 20 Aug 2024 14:42:56 +0100
> Alex Chapman <alex.chapman@arm.com> wrote:
>
>> Hi Stephen,
>>
>> I have gone through your patch series and the hexdump option would be
>> quite valuable for use in DTS.
>>
>> However I am currently facing the issue of distinguishing noise packets
>> from intentional packets within the verbose output. Prior to your patch,
>> the intention was to use the Layer 4 port to distinguish between them,
>> however with the hexdump option, the plan is to now use a custom payload.
>>
>> The one issue is that with verbose level 5 does not print the required
>> RSS hash and RSS queue information.
>
> The queue is there, in the output. Not sure if the hash matters.
> I wanted to keep to tshark format as much as possible.
>
We appreciate that but the RSS info is valuable to us. Seeing as
different enum values offer different info rather than different levels,
maybe we could change the enum to flags
+enum verbose_mode {
+ VERBOSE_OFF = 0,
+ VERBOSE_RX = 0x1,
+ VERBOSE_TX = 0x2,
+ VERBOSE_BOTH = 0x4,
+ VERBOSE_DISSECT = 0x8,
+ VERBOSE_HEX = 0x10
+};
Then the flags can be ORed together:
verbose_flags = VERBOSE_RX | VERBOSE_TX | VERBOSE_HEX | VERBOSE_RSS
And then instead of switch each print is an if that checks
for flag being set in the verbose_flags. This way you only get the RSS
if you request it.
@@ -14143,7 +14143,8 @@ cmd_set_raw_parsed(const struct buffer *in)
upper_layer = proto;
}
}
- if (verbose_level & 0x1)
+
+ if (verbose_level > 0)
printf("total data size is %zu\n", (*total_size));
RTE_ASSERT((*total_size) <= ACTION_RAW_ENCAP_MAX_DATA);
memmove(data, (data_tail - (*total_size)), *total_size);
@@ -6246,26 +6246,37 @@ configure_rxtx_dump_callbacks(uint16_t verbose)
return;
#endif
- RTE_ETH_FOREACH_DEV(portid)
- {
- if (verbose == 1 || verbose > 2)
+ RTE_ETH_FOREACH_DEV(portid) {
+ switch (verbose) {
+ case VERBOSE_OFF:
+ remove_rx_dump_callbacks(portid);
+ remove_tx_dump_callbacks(portid);
+ break;
+ case VERBOSE_RX:
add_rx_dump_callbacks(portid);
- else
+ remove_tx_dump_callbacks(portid);
+ break;
+ case VERBOSE_TX:
+ add_tx_dump_callbacks(portid);
remove_rx_dump_callbacks(portid);
- if (verbose >= 2)
+ break;
+ default:
+ add_rx_dump_callbacks(portid);
add_tx_dump_callbacks(portid);
- else
- remove_tx_dump_callbacks(portid);
+ }
}
}
void
set_verbose_level(uint16_t vb_level)
{
- printf("Change verbose level from %u to %u\n",
- (unsigned int) verbose_level, (unsigned int) vb_level);
- verbose_level = vb_level;
- configure_rxtx_dump_callbacks(verbose_level);
+ if (vb_level < VERBOSE_MAX) {
+ printf("Change verbose level from %u to %u\n", verbose_level, vb_level);
+ verbose_level = vb_level;
+ configure_rxtx_dump_callbacks(verbose_level);
+ } else {
+ fprintf(stderr, "Verbose level %u is out of range\n", vb_level);
+ }
}
void
@@ -489,6 +489,17 @@ enum dcb_mode_enable
extern uint8_t xstats_hide_zero; /**< Hide zero values for xstats display */
+enum verbose_mode {
+ VERBOSE_OFF = 0,
+ VERBOSE_RX,
+ VERBOSE_TX,
+ VERBOSE_BOTH,
+ VERBOSE_DISSECT,
+ VERBOSE_HEX,
+ VERBOSE_MAX
+};
+
+
/* globals used for configuration */
extern uint8_t record_core_cycles; /**< Enables measurement of CPU cycles */
extern uint8_t record_burst_stats; /**< Enables display of RX and TX bursts */
@@ -5,9 +5,11 @@
#include <stdio.h>
+#include <rte_atomic.h>
#include <rte_bitops.h>
#include <rte_net.h>
#include <rte_mbuf.h>
+#include <rte_dissect.h>
#include <rte_ether.h>
#include <rte_vxlan.h>
#include <rte_ethdev.h>
@@ -16,6 +18,7 @@
#include "testpmd.h"
#define MAX_STRING_LEN 8192
+#define MAX_DUMP_LEN 1024
#define MKDUMPSTR(buf, buf_size, cur_len, ...) \
do { \
@@ -67,9 +70,10 @@ get_timestamp(const struct rte_mbuf *mbuf)
timestamp_dynfield_offset, rte_mbuf_timestamp_t *);
}
-static inline void
-dump_pkt_burst(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
- uint16_t nb_pkts, int is_rx)
+/* More verbose older style packet decode */
+static void
+dump_pkt_verbose(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
+ uint16_t nb_pkts, int is_rx)
{
struct rte_mbuf *mb;
const struct rte_ether_hdr *eth_hdr;
@@ -90,8 +94,6 @@ dump_pkt_burst(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
size_t cur_len = 0;
uint64_t restore_info_dynflag;
- if (!nb_pkts)
- return;
restore_info_dynflag = rte_flow_restore_info_dynflag();
MKDUMPSTR(print_buf, buf_size, cur_len,
"port %u/queue %u: %s %u packets\n", port_id, queue,
@@ -299,6 +301,71 @@ dump_pkt_burst(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
}
}
+/* Brief tshark style one line output which is
+ * number time_delta Source Destination Protocol len info
+ */
+static void
+dump_pkt_brief(uint16_t port, uint16_t queue, struct rte_mbuf *pkts[], uint16_t nb_pkts, int is_rx)
+{
+ static uint64_t start_cycles;
+ static RTE_ATOMIC(uint64_t) packet_count = 1;
+ uint64_t now;
+ uint64_t count;
+ double interval;
+ uint16_t i;
+
+ now = rte_rdtsc();
+ if (start_cycles == 0) {
+ start_cycles = now;
+ printf("Seq# Time Port:Que R Description\n");
+ }
+
+ interval = (double)(now - start_cycles) / (double)rte_get_tsc_hz();
+
+ count = rte_atomic_fetch_add_explicit(&packet_count, nb_pkts, rte_memory_order_relaxed);
+
+ for (i = 0; i < nb_pkts; i++) {
+ const struct rte_mbuf *mb = pkts[i];
+ char str[256];
+
+ rte_dissect_mbuf(str, sizeof(str), mb, 0);
+ printf("%6"PRIu64" %11.9f %4u:%-3u %c %s\n",
+ count + i, interval, port, queue, is_rx ? 'R' : 'T', str);
+ }
+}
+
+/* Hex dump of packet data */
+static void
+dump_pkt_hex(struct rte_mbuf *pkts[], uint16_t nb_pkts)
+{
+ uint16_t i;
+
+ for (i = 0; i < nb_pkts; i++)
+ rte_pktmbuf_dump(stdout, pkts[i], MAX_DUMP_LEN);
+
+}
+
+static uint16_t
+dump_pkt_burst(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
+ uint16_t nb_pkts, int is_rx)
+{
+ if (unlikely(nb_pkts == 0))
+ return 0;
+
+ switch (verbose_level) {
+ case VERBOSE_RX ... VERBOSE_BOTH:
+ dump_pkt_verbose(port_id, queue, pkts, nb_pkts, is_rx);
+ break;
+ case VERBOSE_DISSECT:
+ dump_pkt_brief(port_id, queue, pkts, nb_pkts, is_rx);
+ break;
+ case VERBOSE_HEX:
+ dump_pkt_hex(pkts, nb_pkts);
+ }
+ fflush(stdout);
+ return nb_pkts;
+}
+
uint16_t
dump_rx_pkts(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
uint16_t nb_pkts, __rte_unused uint16_t max_pkts,
@@ -677,7 +677,10 @@ Available levels are as following:
* ``0`` silent except for error.
* ``1`` fully verbose except for Tx packets.
* ``2`` fully verbose except for Rx packets.
-* ``> 2`` fully verbose.
+* ``3`` fully verbose except for Tx and Rx packets.
+* ``4`` dissected protocol information for Tx and Rx packets.
+* ``5`` hex dump of packets
+
set log
~~~~~~~