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 9D827A04DD; Wed, 28 Oct 2020 08:27:36 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7D7859B04; Wed, 28 Oct 2020 08:27:35 +0100 (CET) Received: from mail-io1-f65.google.com (mail-io1-f65.google.com [209.85.166.65]) by dpdk.org (Postfix) with ESMTP id BDF0A72EC for ; Wed, 28 Oct 2020 08:27:33 +0100 (CET) Received: by mail-io1-f65.google.com with SMTP id y20so4331990iod.5 for ; Wed, 28 Oct 2020 00:27:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=R5oCPbV+YvFjevU1dmESrTFIE634ubNJ3Ypx9gwWY00=; b=cpo2mHsRaTP3/aKfXYNjGgDMYNwf4uXk74XEUFs1x/fdV+07atWZZlOCQKOwesZEwm OzOMXEENghE4qpwHDDMXbaKU3IDnuImzNI4tWgQMcm29rqx21I81URuFwJuNkEuBwxhM EuREOaGREb/7SAJXNpOafRxEYRTXa9ysvc2934LV5IhG6lUHCqkTNYn2iEBbGVppn5X+ Ea6ovtUIL8ZtQEgZCPVkBDnwulRa1W8DbrHq/BuXPj4Kce+q92lm3whRLHjUSfMG0KoO zlfgWtHbQsvM65gpPKKswkusHd5PX4vr7QC1hNb/JEgsl+IKIl2up8frxTsHvAzsNQh0 KKaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=R5oCPbV+YvFjevU1dmESrTFIE634ubNJ3Ypx9gwWY00=; b=Vzm1+3jM5d7HMtqOlOKK8L4YV2IkGUK8K7j+yE89o5iON8k6CRDHd6hkoQBgCGEJMa fmoV4UfyYxb496siD77h9LqbI6imkm3Hrt9eCS/kGDqHC7ENY518prQ0L1EJox82MNQJ mKL2jJXx2FM0/Ah311q9YJhOIOgP0nkrAyMmIS53nsFuVc0f63dalLWnKAWgaF2ngMfL YebX1YIKAnUZc8k8iKMWudE6NpA9utnfxG6u9zcKA8KZWTB/odKEwu5syx5TfGFMaR24 4cpa1dbU7uypp4xF7gfxpkxxhRRipno2fO6TJFZm38r8bj3K04pCBVHck2UMqyjm8sZg u0XA== X-Gm-Message-State: AOAM530Lw1SvWdLWj0j4bkjSvYK3o6tn1dXAYzT39yEquElDDsSpL5Wp disDNMRoZk93nxkaQ0zHSA9d/KlfbJG19lMh1cw= X-Google-Smtp-Source: ABdhPJwoTz8JnDUoxecyY80M7qlIWrAvgYjS0xmWgJE9s9S1j1zYUbx0dHHrHv4yRXCwdek+FryPoIC0xz6YOLCTdwQ= X-Received: by 2002:a6b:fb07:: with SMTP id h7mr5019015iog.163.1603870051967; Wed, 28 Oct 2020 00:27:31 -0700 (PDT) MIME-Version: 1.0 References: <20201027221343.28551-1-david.marchand@redhat.com> <20201027221343.28551-8-david.marchand@redhat.com> In-Reply-To: <20201027221343.28551-8-david.marchand@redhat.com> From: Jerin Jacob Date: Wed, 28 Oct 2020 12:57:15 +0530 Message-ID: To: David Marchand , "McDaniel, Timothy" Cc: dpdk-dev , Jerin Jacob , Pavan Nikhilesh , Liang Ma , Peter Mccarthy , Harry van Haaren , Ray Kinsella , Neil Horman Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH 7/8] event: switch sequence number to dynamic 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 Wed, Oct 28, 2020 at 3:46 AM David Marchand wrote: > > The eventdev drivers have been hacking the deprecated field seqn for > internal test usage. > It is moved to a dynamic mbuf field in order to allow removal of seqn. > > Signed-off-by: David Marchand Overall looks good some minor comments: # use eventdev: instead of event: in git commit > --- > app/test-eventdev/evt_main.c | 3 ++ > app/test-eventdev/test_order_common.c | 2 +- > app/test-eventdev/test_order_common.h | 5 ++- > drivers/event/octeontx/ssovf_evdev_selftest.c | 32 ++++++++-------- > drivers/event/octeontx2/otx2_evdev_selftest.c | 31 +++++++-------- > drivers/event/opdl/opdl_test.c | 8 ++-- > drivers/event/sw/sw_evdev_selftest.c | 34 +++++++++-------- > lib/librte_eventdev/rte_eventdev.c | 21 +++++++++- > lib/librte_eventdev/rte_eventdev.h | 38 ++++++++++++++++--- > lib/librte_eventdev/version.map | 2 + > 10 files changed, 116 insertions(+), 60 deletions(-) > > diff --git a/app/test-eventdev/evt_main.c b/app/test-eventdev/evt_main.c > index a8d304bab3..832bb21d7c 100644 > --- a/app/test-eventdev/evt_main.c > +++ b/app/test-eventdev/evt_main.c > @@ -89,6 +89,9 @@ main(int argc, char **argv) > if (!evdevs) > rte_panic("no eventdev devices found\n"); > > + if (rte_event_test_seqn_dynfield_register() < 0) > + rte_panic("failed to register event dev sequence number\n= "); > + > /* Populate the default values of the options */ > evt_options_default(&opt); > > diff --git a/app/test-eventdev/test_order_common.c b/app/test-eventdev/te= st_order_common.c > index c5f7317440..d15ff80273 100644 > --- a/app/test-eventdev/test_order_common.c > +++ b/app/test-eventdev/test_order_common.c > @@ -50,7 +50,7 @@ order_producer(void *arg) > > const flow_id_t flow =3D (uintptr_t)m % nb_flows; > /* Maintain seq number per flow */ > - m->seqn =3D producer_flow_seq[flow]++; > + *rte_event_test_seqn(m) =3D producer_flow_seq[flow]++; > flow_id_save(flow, m, &ev); > > while (rte_event_enqueue_burst(dev_id, port, &ev, 1) !=3D= 1) { > diff --git a/app/test-eventdev/test_order_common.h b/app/test-eventdev/te= st_order_common.h > index 9e3415e421..d4ad31da46 100644 > --- a/app/test-eventdev/test_order_common.h > +++ b/app/test-eventdev/test_order_common.h > @@ -89,9 +89,10 @@ order_process_stage_1(struct test_order *const t, > { > const uint32_t flow =3D (uintptr_t)ev->mbuf % nb_flows; > /* compare the seqn against expected value */ > - if (ev->mbuf->seqn !=3D expected_flow_seq[flow]) { > + if (*rte_event_test_seqn(ev->mbuf) !=3D expected_flow_seq[flow]) = { > evt_err("flow=3D%x seqn mismatch got=3D%x expected=3D%x", > - flow, ev->mbuf->seqn, expected_flow_seq[flow]); > + flow, *rte_event_test_seqn(ev->mbuf), > + expected_flow_seq[flow]); > t->err =3D true; > rte_smp_wmb(); > } # Since rte_event_test_seqn_dynfield_register() is the public API, I would like to limit the scope only to self test so that rte_event_test_seqn_dynfield_register() need not be exposed. Could you have a separate application-specific dynamic field here? # Also this patch used in fastpath, better to have offset stored in fastpath cache line. see http://mails.dpdk.org/archives/dev/2020-October/189588.html > +#define RTE_EVENT_TEST_SEQN_DYNFIELD_NAME "rte_event_test_seqn_dynfield" > +int rte_event_test_seqn_dynfield_offset =3D -1; > + > +int > +rte_event_test_seqn_dynfield_register(void) > +{ > + static const struct rte_mbuf_dynfield event_test_seqn_dynfield_de= sc =3D { > + .name =3D RTE_EVENT_TEST_SEQN_DYNFIELD_NAME, > + .size =3D sizeof(rte_event_test_seqn_t), > + .align =3D __alignof__(rte_event_test_seqn_t), > + }; > + rte_event_test_seqn_dynfield_offset =3D > + rte_mbuf_dynfield_register(&event_test_seqn_dynfield_desc= ); > + return rte_event_test_seqn_dynfield_offset; > +} > + > int > rte_event_eth_rx_adapter_caps_get(uint8_t dev_id, uint16_t eth_port_id, > uint32_t *caps) > @@ -1247,8 +1263,11 @@ int rte_event_dev_selftest(uint8_t dev_id) > RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL); > struct rte_eventdev *dev =3D &rte_eventdevs[dev_id]; > > - if (dev->dev_ops->dev_selftest !=3D NULL) > + if (dev->dev_ops->dev_selftest !=3D NULL) { > + if (rte_event_test_seqn_dynfield_register() < 0) > + return -ENOMEM; > return (*dev->dev_ops->dev_selftest)(); > + } > return -ENOTSUP; > } > > diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte= _eventdev.h > index ce1fc2ce0f..1656ff8dce 100644 > --- a/lib/librte_eventdev/rte_eventdev.h > +++ b/lib/librte_eventdev/rte_eventdev.h > @@ -211,13 +211,15 @@ extern "C" { > #endif > > #include > +#include > #include > +#include > +#include > #include > #include > > #include "rte_eventdev_trace_fp.h" > > -struct rte_mbuf; /* we just use mbuf pointers; no need to include rte_mb= uf.h */ > struct rte_event; > > /* Event device capability bitmap flags */ > @@ -570,9 +572,9 @@ struct rte_event_queue_conf { > */ > uint32_t nb_atomic_order_sequences; > /**< The maximum number of outstanding events waiting to be > - * reordered by this queue. In other words, the number of entries= in > - * this queue=E2=80=99s reorder buffer.When the number of events = in the > - * reorder buffer reaches to *nb_atomic_order_sequences* then the > + * event_tested by this queue. In other words, the number of entr= ies in > + * this queue=E2=80=99s event_test buffer.When the number of even= ts in the > + * event_test buffer reaches to *nb_atomic_order_sequences* then = the > * scheduler cannot schedule the events from this queue and inval= id > * event will be returned from dequeue until one or more entries = are > * freed up/released. > @@ -935,7 +937,7 @@ rte_event_dev_close(uint8_t dev_id); > * Event ordering is based on the received event(s), but also other > * (newly allocated or stored) events are ordered when enqueued within t= he same > * ordered context. Events not enqueued (e.g. released or stored) within= the > - * context are considered missing from reordering and are skipped at th= is time > + * context are considered missing from event_testing and are skipped at= this time > * (but can be ordered again within another context). > * > * @see rte_event_queue_setup(), rte_event_dequeue_burst(), RTE_EVENT_OP= _RELEASE > @@ -1021,7 +1023,7 @@ rte_event_dev_close(uint8_t dev_id); > * then this function hints the scheduler that the user has done all tha= t need > * to maintain event order in the current ordered context. > * The scheduler is allowed to release the ordered context of this port = and > - * avoid reordering any following enqueues. > + * avoid event_testing any following enqueues. > * > * Early ordered context release may increase parallelism and thus syste= m > * performance. > @@ -1111,6 +1113,30 @@ struct rte_event { > }; > }; > > +typedef uint32_t rte_event_test_seqn_t; > +extern int rte_event_test_seqn_dynfield_offset; > + > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice > + * > + * Read test sequence number from mbuf. > + * > + * @param mbuf Structure to read from. > + * @return pointer to test sequence number. > + */ > +__rte_experimental > +static inline rte_event_test_seqn_t * > +rte_event_test_seqn(const struct rte_mbuf *mbuf) > +{ > + return RTE_MBUF_DYNFIELD(mbuf, rte_event_test_seqn_dynfield_offse= t, > + rte_event_test_seqn_t *); > +} > + > +__rte_experimental > +int > +rte_event_test_seqn_dynfield_register(void); > + > /* Ethdev Rx adapter capability bitmap flags */ > #define RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT 0x1 > /**< This flag is sent when the packet transfer mechanism is in HW. > diff --git a/lib/librte_eventdev/version.map b/lib/librte_eventdev/versio= n.map > index 8ae8420f9b..e49382ba99 100644 > --- a/lib/librte_eventdev/version.map > +++ b/lib/librte_eventdev/version.map > @@ -138,4 +138,6 @@ EXPERIMENTAL { > __rte_eventdev_trace_port_setup; > # added in 20.11 > rte_event_pmd_pci_probe_named; > + rte_event_test_seqn_dynfield_offset; > + rte_event_test_seqn_dynfield_register; Could you change symbol name to rte_event_dev_selftest_seqn_dynfield_offset= () to limit the scope only to self test. also, it is not required to expose rte_event_test_seqn_dynfield_register() in that case. > }; > -- > 2.23.0 >