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 20A8541BE7; Mon, 6 Feb 2023 09:58:45 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E862242BD9; Mon, 6 Feb 2023 09:58:44 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 0158640FAE for ; Mon, 6 Feb 2023 09:58:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675673922; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=0Wnr5J7LOvalRSy10ssG8GX931KLWPHnXeVvwHkBQRY=; b=eBomaW6YbYWjpB/BZ6Ue1qSGdp7XZKF5HXdpUTXweDEl4vH6h15kF1ie7MHtm38v2FLgIo B3yN59h2F1Pk7OoCzsvP/p6pC4WSlwjyB62RtxoorxPJvNvJTdkIKYu6IuIBg0I8oYaASo VGxVOaUUUL/1LZ9ea+uZTLj6CySSXQc= Received: from mail-pf1-f198.google.com (mail-pf1-f198.google.com [209.85.210.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-533-ZzqhC0kAPgmtXGBM9zkC2Q-1; Mon, 06 Feb 2023 03:58:41 -0500 X-MC-Unique: ZzqhC0kAPgmtXGBM9zkC2Q-1 Received: by mail-pf1-f198.google.com with SMTP id u18-20020a62ed12000000b00593cc641da4so6079538pfh.0 for ; Mon, 06 Feb 2023 00:58:41 -0800 (PST) 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:message-id :reply-to; bh=0Wnr5J7LOvalRSy10ssG8GX931KLWPHnXeVvwHkBQRY=; b=vclGIlwJAwdv0nFzBPxAIdR18HaSdhYsbp6beVj0jiX3wTsfmVhASYwxnzCS6FpZJR 0UQoeMww5n8/kNJboCYxbmu9VIHhcbrCgFM2OpPBGtUE6Cv9lH+4nXcE5dWLRAzaFT1I k4TqA/FI0iMh8XRTUjUM5jaw1tUYShoMeQce50Besqi+b9lUlwt3uvMzlblYfvN5Ao3c NPRrklSEa1FkFjtiSGLrqodPDC2hNqShZmiMfFVWiJaoFfta/FTAJPLP3SU1UZvw64mb Y1lMpTMADmGx04cNV/143MHB1mXBBkVT+qZP4dC2299h3w5+kB5NW42/x9gSTbRPCR17 6TGg== X-Gm-Message-State: AO0yUKXtYt63EqwO9RCXGqB4bIOBkxQRNG2rS7m0Wa4XNqVRP335Nx2l bkaYcQSYP9UthaUQUDU2J6sSz14cPQLfyHY3W3FM6Ux3enBHOnSoClYTd6miJE5kI3vEImOo1lJ x0YzfxxWcwi29sFW9e90= X-Received: by 2002:a05:6a00:1ca0:b0:592:6299:43d2 with SMTP id y32-20020a056a001ca000b00592629943d2mr3881256pfw.1.1675673920277; Mon, 06 Feb 2023 00:58:40 -0800 (PST) X-Google-Smtp-Source: AK7set9i46uq3XxocDPd35RA4PWDEyKK5qyXiXsA2Kudw7oKbMQnOIWEqagz6ic/AMJ3lWn2nOGqplQ/Qsku9GavWc0= X-Received: by 2002:a05:6a00:1ca0:b0:592:6299:43d2 with SMTP id y32-20020a056a001ca000b00592629943d2mr3881249pfw.1.1675673919956; Mon, 06 Feb 2023 00:58:39 -0800 (PST) MIME-Version: 1.0 References: <20221123102612.1688865-1-rjarry@redhat.com> <20230202134329.539625-1-rjarry@redhat.com> <20230202134329.539625-5-rjarry@redhat.com> In-Reply-To: <20230202134329.539625-5-rjarry@redhat.com> From: David Marchand Date: Mon, 6 Feb 2023 09:58:28 +0100 Message-ID: Subject: Re: [PATCH v8 4/5] app/testpmd: report lcore usage To: Robin Jarry Cc: dev@dpdk.org, =?UTF-8?Q?Morten_Br=C3=B8rup?= , Konstantin Ananyev , Kevin Laatz , Aman Singh , Yuying Zhang X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 Salut Robin, On Thu, Feb 2, 2023 at 2:44 PM Robin Jarry wrote: > > Reuse the --record-core-cycles option to account for busy cycles. One > turn of packet_fwd_t is considered "busy" if there was at least one > received or transmitted packet. > > Add a new busy_cycles field in struct fwd_stream. Update get_end_cycles > to accept an additional argument for the number of processed packets. > Update fwd_stream.busy_cycles when the number of packets is greater than > zero. > > When --record-core-cycles is specified, register a callback with > rte_lcore_register_usage_cb(). In the callback, use the new lcore_id > field in struct fwd_lcore to identify the correct index in fwd_lcores > and return the sum of busy/total cycles of all fwd_streams. > > This makes the cycles counters available in rte_lcore_dump() and the > lcore telemetry API: > > testpmd> dump_lcores > lcore 3, socket 0, role RTE, cpuset 3 > lcore 4, socket 0, role RTE, cpuset 4, busy cycles 1228584096/9239923140 > lcore 5, socket 0, role RTE, cpuset 5, busy cycles 1255661768/9218141538 I have been playing a bit with this series with two lcores, each one polling a net/null port. At first it looked good, but then I started to have one idle lcore, by asking net/null not to receive anything. $ build-clang/app/dpdk-testpmd -c 7 --no-huge -m 40 -a 0:0.0 --vdev net_null1,no-rx=1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 -ia --record-core-cycles --nb-cores=2 One thing that struck me is that an idle lcore was always showing less "total_cycles" than a busy one. The more time testpmd was running, the bigger the divergence between lcores would be. Re-reading the API, it is unclear to me (which is the reason for my comments on patch 2). Let's first sort out my patch 2 comments and we may revisit this patch 4 implementation afterwards (as I think we are not accounting some mainloop cycles with current implementation). For now, I have some comments on the existing data structures, see below. [snip] > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index e366f81a0f46..105f75ad5f35 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -2053,7 +2053,7 @@ fwd_stats_display(void) > fs->rx_bad_outer_ip_csum; > > if (record_core_cycles) > - fwd_cycles += fs->core_cycles; > + fwd_cycles += fs->busy_cycles; > } > for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) { > pt_id = fwd_ports_ids[i]; > @@ -2184,6 +2184,7 @@ fwd_stats_reset(void) > > memset(&fs->rx_burst_stats, 0, sizeof(fs->rx_burst_stats)); > memset(&fs->tx_burst_stats, 0, sizeof(fs->tx_burst_stats)); > + fs->busy_cycles = 0; > fs->core_cycles = 0; > } > } > @@ -2260,6 +2261,7 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd) > tics_datum = rte_rdtsc(); > tics_per_1sec = rte_get_timer_hz(); > #endif > + fc->lcore_id = rte_lcore_id(); A fwd_lcore object is bound to a single lcore, so this lcore_id is unneeded. > fsm = &fwd_streams[fc->stream_idx]; > nb_fs = fc->stream_nb; > do { > @@ -2288,6 +2290,38 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd) > } while (! fc->stopped); > } > > +static int > +lcore_usage_callback(unsigned int lcore_id, struct rte_lcore_usage *usage) > +{ > + struct fwd_stream **fsm; > + struct fwd_lcore *fc; > + streamid_t nb_fs; > + streamid_t sm_id; > + int c; > + > + for (c = 0; c < nb_lcores; c++) { > + fc = fwd_lcores[c]; > + if (fc->lcore_id != lcore_id) > + continue; You can find which fwd_lcore is mapped to a lcore using existing structures. This requires updating some helper, something like: diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index 7d24d25970..e5297ee7fb 100644 --- a/app/test-pmd/testpmd.h +++ b/app/test-pmd/testpmd.h @@ -785,25 +785,31 @@ is_proc_primary(void) return rte_eal_process_type() == RTE_PROC_PRIMARY; } -static inline unsigned int -lcore_num(void) +void +parse_fwd_portlist(const char *port); + +static inline struct fwd_lcore * +lcore_to_fwd_lcore(uint16_t lcore_id) { unsigned int i; - for (i = 0; i < RTE_MAX_LCORE; ++i) - if (fwd_lcores_cpuids[i] == rte_lcore_id()) - return i; + for (i = 0; i < cur_fwd_config.nb_fwd_lcores; ++i) { + if (fwd_lcores_cpuids[i] == lcore_id) + return fwd_lcores[i]; + } - rte_panic("lcore_id of current thread not found in fwd_lcores_cpuids\n"); + return NULL; } -void -parse_fwd_portlist(const char *port); - static inline struct fwd_lcore * current_fwd_lcore(void) { - return fwd_lcores[lcore_num()]; + struct fwd_lcore *fc = lcore_to_fwd_lcore(rte_lcore_id()); + + if (fc == NULL) + rte_panic("lcore_id of current thread not found in fwd_lcores_cpuids\n"); + + return fc; } /* Mbuf Pools */ And then by using this new helper, lcore_usage_callback becomes simpler: +static int +lcore_usage_callback(unsigned int lcore_id, struct rte_lcore_usage *usage) +{ + struct fwd_stream **fsm; + struct fwd_lcore *fc; + streamid_t nb_fs; + streamid_t sm_id; + + fc = lcore_to_fwd_lcore(lcore_id); + if (fc == NULL) + return -1; + + fsm = &fwd_streams[fc->stream_idx]; + nb_fs = fc->stream_nb; + usage->busy_cycles = 0; + usage->total_cycles = 0; + + for (sm_id = 0; sm_id < nb_fs; sm_id++) { + if (fsm[sm_id]->disabled) + continue; + + usage->busy_cycles += fsm[sm_id]->busy_cycles; + usage->total_cycles += fsm[sm_id]->core_cycles; + } + + return 0; +} + -- David Marchand