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 92D1DA04B5; Tue, 27 Oct 2020 11:15:46 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B2C962BFF; Tue, 27 Oct 2020 11:15:34 +0100 (CET) Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id 2DD252BF9 for ; Tue, 27 Oct 2020 11:15:34 +0100 (CET) Received: by mail-wm1-f65.google.com with SMTP id l20so732907wme.0 for ; Tue, 27 Oct 2020 03:15:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=BZ9gUF9ugKXXVthV0sgSzu9PQYpMiUs82hsbpAjEF4k=; b=D0ac0qMH4ceF24oA8YBLEsF4ophP2FRX7kL4MJhdkpJk+zvMExkZqylNUvQpMt58d8 SgDW1E+tF1S/2+1kjmHKMm+K8gq5wOxygC4vo6NNN4667ySX74dAf7HoDX0zTfIj/mWS lFkghxjxIPrWujW1qrPb6ND0ihOYrBL2o7if3F4kzcB9ODFSufDrSUGD7sn9X+2c9RzO XKwLkI1Yfznm7WP2D/nv5a0n4dv4SpAkoeS3OLD7WZDB43LirBfSaOBQrflbxzVV7jla c6EszdiR5DJvtWST0bMygCK81yMWc/wfaZq7zpWkAGSBd2HX+W3qL2/6MP5vmdkWpLOZ nxMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=BZ9gUF9ugKXXVthV0sgSzu9PQYpMiUs82hsbpAjEF4k=; b=comuyJYJBqLUJjyKSCIJm1tGqvc+GNWEdFucYlYAPwnqnxRAzVKXkAuneKBBz4KIKm OBSROfKGZ4zT7iumCCLB47LxZi9cIGVqc2fZPLjDwf9NrVIJnzDsxhU0n0FcsXMTmThr rHDum1SGaA7pg0HJa3lm31au6Pdr93FxOG/zGJ+xeGYoEVcAKabL/DXoDwtlmSu6Ka8A O93pRR3eoCXpY8ugqTWumXCx6ruI9HNpUahPUcx1Frb2efip6qDbDSfCHqLpVFmL0r2A FH6oB6txDy595f2oEYhnwXXJQxm5Mxvxi8LyxTnNC6/lYeufkTSEjvaJXGnfdD+ee5a1 eXtg== X-Gm-Message-State: AOAM533yAR4Np266rmAVsUs1+8u+Gt+9Tk2KADxEHnfC1mhwwaoYXnID 2evagqhxJLoswCUuMDcj4j+6Vw== X-Google-Smtp-Source: ABdhPJzeQHOyzVvLIzGUQoe1Yos8H69fts2FbxOsIjUEo2zwRnbACOGJGSiFtq1baZ8JZqPVLhc9kQ== X-Received: by 2002:a7b:c015:: with SMTP id c21mr1877094wmb.22.1603793732875; Tue, 27 Oct 2020 03:15:32 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id g17sm1450753wrw.37.2020.10.27.03.15.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Oct 2020 03:15:32 -0700 (PDT) Date: Tue, 27 Oct 2020 11:15:31 +0100 From: Olivier Matz To: Thomas Monjalon Cc: dev@dpdk.org, ferruh.yigit@intel.com, david.marchand@redhat.com, bruce.richardson@intel.com, andrew.rybchenko@oktetlabs.ru, akhil.goyal@nxp.com, Harry van Haaren Message-ID: <20201027101531.GP1898@platinum> References: <20201026052105.1561859-1-thomas@monjalon.net> <20201026222013.2147904-1-thomas@monjalon.net> <20201026222013.2147904-7-thomas@monjalon.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201026222013.2147904-7-thomas@monjalon.net> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v2 06/15] event/sw: switch test counter to dynamic mbuf field X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Oct 26, 2020 at 11:20:04PM +0100, Thomas Monjalon wrote: > The test worker_loopback used the deprecated mbuf field udata64. > It is moved to a dynamic field in order to allow removal of udata64. > > Signed-off-by: Thomas Monjalon > Reviewed-by: Andrew Rybchenko > --- > drivers/event/sw/sw_evdev_selftest.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/drivers/event/sw/sw_evdev_selftest.c b/drivers/event/sw/sw_evdev_selftest.c > index 5c7e527917..9af20cecf1 100644 > --- a/drivers/event/sw/sw_evdev_selftest.c > +++ b/drivers/event/sw/sw_evdev_selftest.c > @@ -40,6 +40,11 @@ struct test { > uint32_t service_id; > }; > > +static int counter_dynfield_offset; In general, I wonder if we shouldn't initialize offset to -1. > +#define COUNTER_FIELD_TYPE uint8_t Another general comment, I suggest to use a typedef instead of a define when relevant. > +#define COUNTER_FIELD(mbuf) (*RTE_MBUF_DYNFIELD(mbuf, \ > + counter_dynfield_offset, COUNTER_FIELD_TYPE *)) > + I'm not sure this comment applies here, but since it's a simple example, it's a good place for another general comment. The RTE_MBUF_DYNFIELD() macro is convenient because it can be used to set or get a value of any type, but in my opinion it is not always easy to read: RTE_MBUF_DYNFIELD(m, off, type) = value; In some situations, having wrappers may make the code more readable: static inline void mbuf_set_counter(struct rte_mbuf *m, counter_field_t counter); static inline counter_field_t mbuf_get_counter(struct rte_mbuf *m); static inline void mbuf_inc_counter(struct rte_mbuf *m); > static struct rte_event release_ev; > > static inline struct rte_mbuf * > @@ -2987,8 +2992,8 @@ worker_loopback_worker_fn(void *arg) > } > > ev[i].queue_id = 0; > - ev[i].mbuf->udata64++; > - if (ev[i].mbuf->udata64 != 16) { > + COUNTER_FIELD(ev[i].mbuf)++; > + if (COUNTER_FIELD(ev[i].mbuf) != 16) { > ev[i].op = RTE_EVENT_OP_FORWARD; > enqd = rte_event_enqueue_burst(evdev, port, > &ev[i], 1); > @@ -3028,7 +3033,7 @@ worker_loopback_producer_fn(void *arg) > m = rte_pktmbuf_alloc(t->mbuf_pool); > } while (m == NULL); > > - m->udata64 = 0; > + COUNTER_FIELD(m) = 0; > > struct rte_event ev = { > .op = RTE_EVENT_OP_NEW, > @@ -3061,6 +3066,18 @@ worker_loopback(struct test *t, uint8_t disable_implicit_release) > int err; > int w_lcore, p_lcore; > > + static const struct rte_mbuf_dynfield counter_dynfield_desc = { > + .name = "rte_event_sw_dynfield_selftest_counter", > + .size = sizeof(COUNTER_FIELD_TYPE), > + .align = __alignof__(COUNTER_FIELD_TYPE), > + }; > + counter_dynfield_offset = > + rte_mbuf_dynfield_register(&counter_dynfield_desc); > + if (counter_dynfield_offset < 0) { > + printf("Error registering mbuf field\n"); > + return -rte_errno; > + } > + > if (init(t, 8, 2) < 0 || > create_atomic_qids(t, 8) < 0) { > printf("%d: Error initializing device\n", __LINE__); > -- > 2.28.0 >