From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D689EA00C3; Wed, 21 Sep 2022 16:42:48 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C6C0D4067C; Wed, 21 Sep 2022 16:42:48 +0200 (CEST) Received: from mail-qt1-f176.google.com (mail-qt1-f176.google.com [209.85.160.176]) by mails.dpdk.org (Postfix) with ESMTP id BF9594014F for ; Wed, 21 Sep 2022 16:42:47 +0200 (CEST) Received: by mail-qt1-f176.google.com with SMTP id r20so4202980qtn.12 for ; Wed, 21 Sep 2022 07:42:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=jjTeTB5f4xydGY80mgYOPh97ZN40uvSP6Z55dUIoIvA=; b=hqLowKNLl5q6BVfOea3MUg3xZWC5oMij6dL5/EhIdTS4scEwnaxnK4CwgkhcS/0XXF Pd1nPeZETh51kx9uj6CMns3jYTeBY7k6zBMXqgEO5wmDeShrsd9gfBfy6nsQvjkO3h2g T+eyANelW0RxonJjUnG6vrrX4u3vwjsJFJCWQ3BX8dXiAmABZzXG009Q6OMaPhtzHKTt vUCo8IjyOuWhwEIufZxUXBEE9wJq9cLJ5+1HJX7gzqgggC101+WDFDmnYTuyZXblL7vO g57/Rp0dKklVz7JPPtpO1S5Yp587oCpex33dObLuErD/6ZuZhD++salxXjis7hA9jNdD 0UzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=jjTeTB5f4xydGY80mgYOPh97ZN40uvSP6Z55dUIoIvA=; b=QjPrMsHk8Rt5DnODkuNuiQOG9Hz5B1U7K3pTJERrA2eq9xnAcsCrRi2FNNSY7Vba4J pEXU7OBPgshRcnV9RK0IEJp1FolCi/zD22FlXFggjOFPHtjgL9unZ02iJUsf7xtbQdug 5ohOSxntxZBVHB95JMabuvRde70zGQOBNvxA7/HQifEUJlZJglSDph/Wl1BIyE3IOO5t x3+icMT8ww3l3raOnGG/cxMOcP3mrfbuadNRJOuVcqVB9HEmXOliMm5dZb1EVxF4Dpwg T6mRxU5SsHIoAEULJ+OvNNf07FZ3nSHyLAj+nGdW8JrFhzHHQFlY+Ddrmyt0ZlI248fw GcnA== X-Gm-Message-State: ACrzQf0SkWqYG3hh6Wd6nhMVaSJPK1NI9W4kGunSb+RXWm2EtRuhf3RL zTU3mXVwfkOxBPivfe5hdUrg+i5C9gf8wgeFCcXRzwuLoJU= X-Google-Smtp-Source: AMsMyM59Lny/mz6riP3jDDS2GJ2VDTBmhd+DP/vFfhLgqySbxyOUKznAhC9ewrlMcLX62O2jMEHQy32nzmjRThKOWbw= X-Received: by 2002:ac8:5ccd:0:b0:35c:e18b:2be3 with SMTP id s13-20020ac85ccd000000b0035ce18b2be3mr15715325qta.502.1663771366983; Wed, 21 Sep 2022 07:42:46 -0700 (PDT) MIME-Version: 1.0 References: <20220902084533.675698-1-felix.moessbauer@siemens.com> <20220902084533.675698-3-felix.moessbauer@siemens.com> In-Reply-To: <20220902084533.675698-3-felix.moessbauer@siemens.com> From: Jerin Jacob Date: Wed, 21 Sep 2022 20:12:21 +0530 Message-ID: Subject: Re: [PATCH v6 2/2] Add l2reflect measurement application To: Felix Moessbauer Cc: dev@dpdk.org, henning.schild@siemens.com, jan.kiszka@siemens.com, thomas@monjalon.net Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Fri, Sep 2, 2022 at 2:16 PM Felix Moessbauer wrote: > > The l2reflect application implements a ping-pong benchmark to > measure the latency between two instances. For communication, > we use raw ethernet and send one packet at a time. The timing data > is collected locally and min/max/avg values are displayed in a TUI. > Finally, a histogram of the latencies is printed which can be One highlevel comment, IMO, We don't need to add code for capturing in histogram and analyze it in C code. We can simply use, rte_trace and so that we can visualize with standard trace viewers. Also add add python based script parse the CTF emitted by trace and find min/max/avg values to simply the code. > further processed with the jitterdebugger visualization scripts. > To debug latency spikes, a max threshold can be defined. > If it is hit, a trace point is created on both instances. > > Signed-off-by: Felix Moessbauer > Signed-off-by: Henning Schild > --- > app/l2reflect/colors.c | 34 ++ > app/l2reflect/colors.h | 19 + > app/l2reflect/l2reflect.h | 53 ++ > app/l2reflect/main.c | 1007 +++++++++++++++++++++++++++++++++++++ > app/l2reflect/meson.build | 21 + > app/l2reflect/payload.h | 26 + > app/l2reflect/stats.c | 225 +++++++++ > app/l2reflect/stats.h | 67 +++ > app/l2reflect/utils.c | 67 +++ > app/l2reflect/utils.h | 20 + > app/meson.build | 1 + Need to add a doc for this example at doc/guides/sample_app_ug/ > > diff --git a/app/l2reflect/colors.c b/app/l2reflect/colors.c > new file mode 100644 > index 0000000000..af881d8788 > --- /dev/null > +++ b/app/l2reflect/colors.c > @@ -0,0 +1,34 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2020 Siemens AG 2022. Please fix in all files. > diff --git a/app/l2reflect/l2reflect.h b/app/l2reflect/l2reflect.h > new file mode 100644 > index 0000000000..922bd7c281 > --- /dev/null > +++ b/app/l2reflect/l2reflect.h > @@ -0,0 +1,53 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2020 Siemens AG > + */ > + > +#include > + > +#ifndef _L2REFLECT_L2REFLECT_H_ > +#define _L2REFLECT_L2REFLECT_H_ > + > +#define RTE_LOGTYPE_L2REFLECT RTE_LOGTYPE_USER1 Don't define RTE_ symbols in application. > + > +/* max size that common 1G NICs support */ > +#define MAX_JUMBO_PKT_LEN 9600 > + > +/* Used to compare MAC addresses. */ > +#define MAC_ADDR_CMP 0xFFFFFFFFFFFFull > + > +#define MAX(x, y) (((x) > (y)) ? (x) : (y)) > +#define MIN(x, y) (((x) < (y)) ? (x) : (y)) Use RTE_MAX and friends. Similar comments for other macros. > +extern int l2reflect_hist; > +extern unsigned int l2reflect_hist_buckets; > +extern atomic_int l2reflect_output_hist; > +extern int l2reflect_interrupt; > +extern uint64_t l2reflect_sleep_msec; > +extern uint64_t l2reflect_pkt_bytes; > +extern uint16_t l2reflect_port_number; > +extern atomic_int l2reflect_state; > +extern struct rte_ether_addr l2reflect_port_eth_addr; > +extern struct rte_ether_addr l2reflect_remote_eth_addr; It is better to move all global variables to app_ctx kind of structure and allocate from hugepage as some of them are used in fastpath and easy to track. > +/* Configurable number of RX/TX ring descriptors */ > +#define RTE_TEST_RX_DESC_DEFAULT 128 > +#define RTE_TEST_TX_DESC_DEFAULT 128 Don't define RTE_ symbols in application. > +/* Send a burst of one packet */ > +static inline int > +l2reflect_send_packet(struct rte_mbuf **m, uint16_t port) > +{ > + unsigned int ret; > + struct rte_ether_hdr *eth; > + struct my_magic_packet *pkt; > + uint16_t type; > + > + eth = rte_pktmbuf_mtod(*m, struct rte_ether_hdr *); > + pkt = (struct my_magic_packet *)eth; > + type = pkt->type; > + > + if (likely(type == TRACE_TYPE_DATA)) > + clock_gettime(CLOCK_MONOTONIC, &last_sent); Use rte_rdtsc_precise() instead of system call in fastpath > + ret = rte_eth_tx_burst(port, l2reflect_q, m, 1); > + if (unlikely(ret < 1)) > + rte_pktmbuf_free(*m); > + return 0; > +} > + > +static inline void > +l2reflect_simple_forward(struct rte_mbuf *m) > +{ > + struct rte_ether_hdr *eth; > + struct my_magic_packet *pkt; > + > + eth = rte_pktmbuf_mtod(m, struct rte_ether_hdr *); > + pkt = (struct my_magic_packet *)eth; > + > + /* dst addr */ > + rte_ether_addr_copy(ð->src_addr, ð->dst_addr); > + /* src addr */ > + rte_ether_addr_copy(&l2reflect_port_eth_addr, ð->src_addr); > + > + if (unlikely(pkt->magic == MAGIC_TRACE_PAYLOAD)) > + rte_eal_trace_generic_str("sending traced packet"); Create app_trace tracepoint specific to application with new type to avoid emit the string. > + > + l2reflect_send_packet(&m, l2reflect_port_number); > +} > + > +/* > + * process a single packet. > + * return false if latency threshold is hit > + */ > +static inline int > +process_packet( > + struct my_magic_packet *pkt, > + struct timespec *rx_time, > + uint64_t *diff) > +{ > + if (pkt->type == TRACE_TYPE_DATA) { > + rte_memcpy(&last_recv, rx_time, sizeof(*rx_time)); > + *diff = calcdiff_ns(last_recv, last_sent); > + if (!unlikely(add_to_record(*diff))) { > + /* TODO: improve tracing */ > + rte_eal_trace_generic_u64(record.max_round_ns / 1000); Add a new trace point as an application specific item. > + return 0; > + } > + } > + if (pkt->magic == MAGIC_TRACE_PAYLOAD) > + rte_eal_trace_generic_str("received traced packet"); Add a new trace point as an application specific item. > + > + return 1; > +} > + > +/* > + * free all packet buffers in the range [begin, end[. > + */ > +static void > +free_pktbufs( > + struct rte_mbuf **bufs, > + int begin, > + int end) > +{ > + int i = begin; > + for (; i < end; i++) > + rte_pktmbuf_free(bufs[0]); Freeing the same packet multiple times? Use burst varint if possible. > +} > + > +/* > + * return 1 in case the ball was lost (cheap check) > + */ > +static inline void > +check_ball_lost(const uint64_t dp_idle) { > + /* only check if we are in running state and have a breakval */ > + if (unlikely(dp_idle & RX_TIMEOUT_MASK) && > + l2reflect_state == S_RUNNING && > + l2reflect_break_usec && > + record.rounds > WARMUP_ROUNDS) { > + RTE_LOG(INFO, L2REFLECT, "lost ball after %" PRIu64 " rounds\n", record.rounds); > + l2reflect_state = S_LOCAL_TERM; > + } > +} > +} > + > + /* > + * in quiet mode the primary executes the main packet loop > + * otherwise the one worker does it and the primary prints stats > + */ > + if (quiet) { > + assert(rte_lcore_count() == 1); > +#ifdef HAS_SYS_IO > + if (disable_int) { > + iopl(3); > + asm("cli"); Do we need x86 specific items in application? > + } > +#endif > + RTE_LOG(INFO, L2REFLECT, "PID %i, Parent %i\n", getpid(), getppid()); > + l2reflect_launch_one_lcore(NULL); > + } else { > + assert(rte_lcore_count() == 2); > + /* the worker reflects the packets */ > + RTE_LCORE_FOREACH_WORKER(lcore_id) > + { > + rte_eal_remote_launch(l2reflect_launch_one_lcore, NULL, > + lcore_id); > + } > + > + /* the primary prints the stats */ > + init_record(); > + l2reflect_stats_loop(); > + rte_eal_mp_wait_lcore(); > + } > + rte_eal_cleanup(); > + > + if (l2reflect_hist) > + output_histogram_snapshot(); > + > + cleanup_record(); > + > + return 0; > +} > diff --git a/app/l2reflect/meson.build b/app/l2reflect/meson.build > new file mode 100644 > index 0000000000..14b154ef06 > --- /dev/null > +++ b/app/l2reflect/meson.build > @@ -0,0 +1,21 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2022 Siemens AG > + > +cc = meson.get_compiler('c') > + > +jansson = dependency('libjansson', required: false) > + > +if not jansson.found() > + jansson = cc.find_library('jansson', required: false) > +endif > + > +if cc.has_header('sys/io.h') > + cflags += '-DHAS_SYS_IO' > +endif > + > +sources = files('main.c', 'utils.c', 'stats.c', 'colors.c') > +deps += ['ethdev'] > +if jansson.found() > + ext_deps += jansson > + cflags += '-DRTE_HAS_JANSSON' > +endif > diff --git a/app/l2reflect/payload.h b/app/l2reflect/payload.h > new file mode 100644 > index 0000000000..c1fae5d5e4 > --- /dev/null > +++ b/app/l2reflect/payload.h > @@ -0,0 +1,26 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2022 Siemens AG > + */ > +#include > + > +#ifndef _L2REFLECT_PAYLOAD_H_ > +#define _L2REFLECT_PAYLOAD_H_ > + > +#define MAGIC_TRACE_PAYLOAD 0xd00faffeaffed00full > +/* IEEE Std 802 - Local Experimental Ethertype */ > +#define ETHER_TYPE_L2REFLECT 0x88B5 > + > +struct my_magic_packet { > + /* l2 packet header */ > + struct rte_ether_hdr eth; > + /* type of the l2reflect packet */ > + uint8_t type; > + /* magic easy-to-spot pattern for tracing */ > + uint64_t magic; > + /* break if latency is larger than this */ > + uint64_t breakval; > + /* intended size of the packet */ > + uint64_t req_pkt_bytes; > +}; > + > +#endif /* _L2REFLECT_PAYLOAD_H_ */