On Wed, Oct 28, 2020 at 8:27 AM Jerin Jacob <jerinjac...@gmail.com> wrote:
> > diff --git a/app/test-eventdev/test_order_common.h
> > b/app/test-eventdev/test_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 = (uintptr_t)ev->mbuf % nb_flows;
> > /* compare the seqn against expected value */
> > - if (ev->mbuf->seqn != expected_flow_seq[flow]) {
> > + if (*rte_event_test_seqn(ev->mbuf) != expected_flow_seq[flow]) {
> > evt_err("flow=%x seqn mismatch got=%x expected=%x",
> > - flow, ev->mbuf->seqn, expected_flow_seq[flow]);
> > + flow, *rte_event_test_seqn(ev->mbuf),
> > + expected_flow_seq[flow]);
> > t->err = 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
Ack for a test app dynamic field.
On the second comment, I'll wait for Thomas respin.
> > diff --git a/lib/librte_eventdev/version.map
> > b/lib/librte_eventdev/version.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.
How about moving this to rte_eventdev_pmd.h and make it a pmd only API?
=> rte_event_pmd_ prefix
I would mark it rte_internal too.
--
David Marchand
