From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3C0C8A04BC for ; Thu, 17 Sep 2020 14:34:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 11F961D643; Thu, 17 Sep 2020 14:34:49 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 1AA231D630; Thu, 17 Sep 2020 14:34:44 +0200 (CEST) IronPort-SDR: JFxmYz7boq0ZqgigwmhMXNP49bRQExcLKju1Yn7JXkgDB6XMoixDRSjSxMAEVW981CFKMMpIbQ s/wkHncY4FBg== X-IronPort-AV: E=McAfee;i="6000,8403,9746"; a="139691341" X-IronPort-AV: E=Sophos;i="5.76,436,1592895600"; d="scan'208";a="139691341" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Sep 2020 05:34:41 -0700 IronPort-SDR: 0vXcpNds9deCS7BirOpkZkUGzHwIYUeZY67J1KuamcsWqzQ88ThSQwEAjVwhhAySZeW918u9tt eanhlimbxeRA== X-IronPort-AV: E=Sophos;i="5.76,436,1592895600"; d="scan'208";a="483727667" Received: from dhunt5-mobl5.ger.corp.intel.com (HELO [10.213.228.49]) ([10.213.228.49]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Sep 2020 05:34:40 -0700 To: Lukasz Wojciechowski , Bruce Richardson Cc: dev@dpdk.org, stable@dpdk.org References: <20200915193449.13310-1-l.wojciechow@partner.samsung.com> <20200915193449.13310-4-l.wojciechow@partner.samsung.com> From: David Hunt Message-ID: Date: Thu, 17 Sep 2020 13:34:38 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20200915193449.13310-4-l.wojciechow@partner.samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB Subject: Re: [dpdk-stable] [PATCH v1 3/6] app/test: fix freeing mbufs in distributor tests X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi Lukasz, On 15/9/2020 8:34 PM, Lukasz Wojciechowski wrote: > Sanity tests with mbuf alloc and shutdown tests assume that > mbufs passed to worker cores are freed in handlers. > Such packets should not be returned to the distributor's main > core. The only packets that should be returned are the packets > send after completion of the tests in quit_workers function. > > This patch fixes freeing mbufs, stops returning them > to distributor's core and cleans up unused variables. > > Fixes: c0de0eb82e40 ("distributor: switch over to new API") > Cc: david.hunt@intel.com > Cc: stable@dpdk.org > > Signed-off-by: Lukasz Wojciechowski > --- > app/test/test_distributor.c | 37 +++++++++++-------------------------- > 1 file changed, 11 insertions(+), 26 deletions(-) > > diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c > index 0e49e3714..da13a9a3f 100644 > --- a/app/test/test_distributor.c > +++ b/app/test/test_distributor.c > @@ -277,24 +277,21 @@ handle_work_with_free_mbufs(void *arg) > struct rte_mbuf *buf[8] __rte_cache_aligned; > struct worker_params *wp = arg; > struct rte_distributor *d = wp->dist; > - unsigned int count = 0; > unsigned int i; > unsigned int num = 0; > unsigned int id = __atomic_fetch_add(&worker_idx, 1, __ATOMIC_RELAXED); > > for (i = 0; i < 8; i++) > buf[i] = NULL; > - num = rte_distributor_get_pkt(d, id, buf, buf, num); > + num = rte_distributor_get_pkt(d, id, buf, buf, 0); > while (!quit) { > - count += num; > __atomic_fetch_add(&worker_stats[id].handled_packets, num, > __ATOMIC_ACQ_REL); > for (i = 0; i < num; i++) > rte_pktmbuf_free(buf[i]); > num = rte_distributor_get_pkt(d, > - id, buf, buf, num); > + id, buf, buf, 0); > } > - count += num; > __atomic_fetch_add(&worker_stats[id].handled_packets, num, > __ATOMIC_ACQ_REL); > rte_distributor_return_pkt(d, id, buf, num); > @@ -322,7 +319,6 @@ sanity_test_with_mbuf_alloc(struct worker_params *wp, struct rte_mempool *p) > rte_distributor_process(d, NULL, 0); > for (j = 0; j < BURST; j++) { > bufs[j]->hash.usr = (i+j) << 1; > - rte_mbuf_refcnt_set(bufs[j], 1); > } > > rte_distributor_process(d, bufs, BURST); > @@ -346,20 +342,15 @@ sanity_test_with_mbuf_alloc(struct worker_params *wp, struct rte_mempool *p) > static int > handle_work_for_shutdown_test(void *arg) > { > - struct rte_mbuf *pkt = NULL; > struct rte_mbuf *buf[8] __rte_cache_aligned; > struct worker_params *wp = arg; > struct rte_distributor *d = wp->dist; > - unsigned int count = 0; > unsigned int num = 0; > - unsigned int total = 0; > unsigned int i; > - unsigned int returned = 0; > unsigned int zero_id = 0; > const unsigned int id = __atomic_fetch_add(&worker_idx, 1, > __ATOMIC_RELAXED); > - > - num = rte_distributor_get_pkt(d, id, buf, buf, num); > + num = rte_distributor_get_pkt(d, id, buf, buf, 0); > > zero_id = __atomic_load_n(&zero_idx, __ATOMIC_ACQUIRE); > if (id == zero_id && num > 0) { > @@ -371,13 +362,12 @@ handle_work_for_shutdown_test(void *arg) > /* wait for quit single globally, or for worker zero, wait > * for zero_quit */ > while (!quit && !(id == zero_id && zero_quit)) { > - count += num; > __atomic_fetch_add(&worker_stats[id].handled_packets, num, > __ATOMIC_ACQ_REL); > for (i = 0; i < num; i++) > rte_pktmbuf_free(buf[i]); > num = rte_distributor_get_pkt(d, > - id, buf, buf, num); > + id, buf, buf, 0); > > zero_id = __atomic_load_n(&zero_idx, __ATOMIC_ACQUIRE); > if (id == zero_id && num > 0) { > @@ -385,12 +375,7 @@ handle_work_for_shutdown_test(void *arg) > __ATOMIC_ACQUIRE); > __atomic_store_n(&zero_idx, zero_id, __ATOMIC_RELEASE); > } > - > - total += num; > } > - count += num; > - returned = rte_distributor_return_pkt(d, id, buf, num); > - > __atomic_fetch_add(&worker_stats[id].handled_packets, num, > __ATOMIC_ACQ_REL); > if (id == zero_id) { > @@ -400,20 +385,20 @@ handle_work_for_shutdown_test(void *arg) > while (zero_quit) > usleep(100); > > + for (i = 0; i < num; i++) > + rte_pktmbuf_free(buf[i]); > num = rte_distributor_get_pkt(d, > - id, buf, buf, num); > + id, buf, buf, 0); > > while (!quit) { > - count += num; > - rte_pktmbuf_free(pkt); > - num = rte_distributor_get_pkt(d, id, buf, buf, num); > __atomic_fetch_add(&worker_stats[id].handled_packets, > num, __ATOMIC_ACQ_REL); > + for (i = 0; i < num; i++) > + rte_pktmbuf_free(buf[i]); > + num = rte_distributor_get_pkt(d, id, buf, buf, 0); > } > - returned = rte_distributor_return_pkt(d, > - id, buf, num); > - printf("Num returned = %d\n", returned); > } > + rte_distributor_return_pkt(d, id, buf, num); > return 0; > } > Nice cleanup, Thanks. Acked-by: David Hunt