[v3,12/15] app/eventdev: switch flow ID to dynamic mbuf field
Checks
Commit Message
The order test stored the flow ID in the deprecated mbuf field udata64.
It is moved to a dynamic field in order to allow removal of udata64.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
app/test-eventdev/test_order_atq.c | 4 ++--
app/test-eventdev/test_order_common.c | 21 ++++++++++++++++-----
app/test-eventdev/test_order_common.h | 19 +++++++++++++++++++
app/test-eventdev/test_order_queue.c | 4 ++--
4 files changed, 39 insertions(+), 9 deletions(-)
Comments
On Wed, Oct 28, 2020 at 2:35 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> The order test stored the flow ID in the deprecated mbuf field udata64.
> It is moved to a dynamic field in order to allow removal of udata64.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> app/test-eventdev/test_order_atq.c | 4 ++--
> app/test-eventdev/test_order_common.c | 21 ++++++++++++++++-----
> app/test-eventdev/test_order_common.h | 19 +++++++++++++++++++
> app/test-eventdev/test_order_queue.c | 4 ++--
> 4 files changed, 39 insertions(+), 9 deletions(-)
+ timothy.mcdaniel@intel.com
>
> diff --git a/app/test-eventdev/test_order_atq.c b/app/test-eventdev/test_order_atq.c
> index cfcb1dc4e9..b135ac4e55 100644
> --- a/app/test-eventdev/test_order_atq.c
> +++ b/app/test-eventdev/test_order_atq.c
> @@ -35,7 +35,7 @@ order_atq_worker(void *arg, const bool flow_id_cap)
> }
>
> if (!flow_id_cap)
> - ev.flow_id = ev.mbuf->udata64;
> + flow_id_copy_from_mbuf(&ev);
>
> if (ev.sub_event_type == 0) { /* stage 0 from producer */
> order_atq_process_stage_0(&ev);
> @@ -72,7 +72,7 @@ order_atq_worker_burst(void *arg, const bool flow_id_cap)
>
> for (i = 0; i < nb_rx; i++) {
> if (!flow_id_cap)
> - ev[i].flow_id = ev[i].mbuf->udata64;
> + flow_id_copy_from_mbuf(&ev[i]);
>
> if (ev[i].sub_event_type == 0) { /*stage 0 */
> order_atq_process_stage_0(&ev[i]);
> diff --git a/app/test-eventdev/test_order_common.c b/app/test-eventdev/test_order_common.c
> index dc55d93921..c5f7317440 100644
> --- a/app/test-eventdev/test_order_common.c
> +++ b/app/test-eventdev/test_order_common.c
> @@ -4,6 +4,8 @@
>
> #include "test_order_common.h"
>
> +int flow_id_dynfield_offset = -1;
See below,
> +
> int
> order_test_result(struct evt_test *test, struct evt_options *opt)
> {
> @@ -46,13 +48,10 @@ order_producer(void *arg)
> if (m == NULL)
> continue;
>
> - const uint32_t flow = (uintptr_t)m % nb_flows;
> + const flow_id_t flow = (uintptr_t)m % nb_flows;
> /* Maintain seq number per flow */
> m->seqn = producer_flow_seq[flow]++;
> - m->udata64 = flow;
> -
> - ev.flow_id = flow;
> - ev.mbuf = m;
> + flow_id_save(flow, m, &ev);
>
> while (rte_event_enqueue_burst(dev_id, port, &ev, 1) != 1) {
> if (t->err)
> @@ -139,6 +138,18 @@ order_test_setup(struct evt_test *test, struct evt_options *opt)
> {
> void *test_order;
>
> + static const struct rte_mbuf_dynfield flow_id_dynfield_desc = {
> + .name = "test_event_dynfield_flow_id",
> + .size = sizeof(flow_id_t),
> + .align = __alignof__(flow_id_t),
> + };
> + flow_id_dynfield_offset =
> + rte_mbuf_dynfield_register(&flow_id_dynfield_desc);
Since this path used in fastpath, could you move flow_id_dynfield_offset to
test_order's initial entry as that cache will be warm always.
> + if (flow_id_dynfield_offset < 0) {
> + evt_err("failed to register mbuf field");
> + return -rte_errno;
> + }
> +
> test_order = rte_zmalloc_socket(test->name, sizeof(struct test_order),
> RTE_CACHE_LINE_SIZE, opt->socket_id);
See above
The rest of the eventdev and Graph changes are fine.
28/10/2020 05:54, Jerin Jacob:
> On Wed, Oct 28, 2020 at 2:35 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > The order test stored the flow ID in the deprecated mbuf field udata64.
> > It is moved to a dynamic field in order to allow removal of udata64.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > + static const struct rte_mbuf_dynfield flow_id_dynfield_desc = {
> > + .name = "test_event_dynfield_flow_id",
> > + .size = sizeof(flow_id_t),
> > + .align = __alignof__(flow_id_t),
> > + };
> > + flow_id_dynfield_offset =
> > + rte_mbuf_dynfield_register(&flow_id_dynfield_desc);
>
>
> Since this path used in fastpath, could you move flow_id_dynfield_offset to
> test_order's initial entry as that cache will be warm always.
I don't understand. Are you talking about the offset value?
This field will be in the second part of mbuf, as udata64 was.
Or are you talking about the storage of the offset variable?
> > + if (flow_id_dynfield_offset < 0) {
> > + evt_err("failed to register mbuf field");
> > + return -rte_errno;
> > + }
> > +
> > test_order = rte_zmalloc_socket(test->name, sizeof(struct test_order),
> > RTE_CACHE_LINE_SIZE, opt->socket_id);
>
> See above
You mean the offset should be stored in this struct test_order?
On Wed, Oct 28, 2020 at 1:13 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 28/10/2020 05:54, Jerin Jacob:
> > On Wed, Oct 28, 2020 at 2:35 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > The order test stored the flow ID in the deprecated mbuf field udata64.
> > > It is moved to a dynamic field in order to allow removal of udata64.
> > >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > > + static const struct rte_mbuf_dynfield flow_id_dynfield_desc = {
> > > + .name = "test_event_dynfield_flow_id",
> > > + .size = sizeof(flow_id_t),
> > > + .align = __alignof__(flow_id_t),
> > > + };
> > > + flow_id_dynfield_offset =
> > > + rte_mbuf_dynfield_register(&flow_id_dynfield_desc);
> >
> >
> > Since this path used in fastpath, could you move flow_id_dynfield_offset to
> > test_order's initial entry as that cache will be warm always.
>
> I don't understand. Are you talking about the offset value?
> This field will be in the second part of mbuf, as udata64 was.
> Or are you talking about the storage of the offset variable?
Yes. Storage of the offset variable.
>
> > > + if (flow_id_dynfield_offset < 0) {
> > > + evt_err("failed to register mbuf field");
> > > + return -rte_errno;
> > > + }
> > > +
> > > test_order = rte_zmalloc_socket(test->name, sizeof(struct test_order),
> > > RTE_CACHE_LINE_SIZE, opt->socket_id);
> >
> > See above
>
> You mean the offset should be stored in this struct test_order?
Yes. Instead of a global variable flow_id_dynfield_offset
>
>
>
@@ -35,7 +35,7 @@ order_atq_worker(void *arg, const bool flow_id_cap)
}
if (!flow_id_cap)
- ev.flow_id = ev.mbuf->udata64;
+ flow_id_copy_from_mbuf(&ev);
if (ev.sub_event_type == 0) { /* stage 0 from producer */
order_atq_process_stage_0(&ev);
@@ -72,7 +72,7 @@ order_atq_worker_burst(void *arg, const bool flow_id_cap)
for (i = 0; i < nb_rx; i++) {
if (!flow_id_cap)
- ev[i].flow_id = ev[i].mbuf->udata64;
+ flow_id_copy_from_mbuf(&ev[i]);
if (ev[i].sub_event_type == 0) { /*stage 0 */
order_atq_process_stage_0(&ev[i]);
@@ -4,6 +4,8 @@
#include "test_order_common.h"
+int flow_id_dynfield_offset = -1;
+
int
order_test_result(struct evt_test *test, struct evt_options *opt)
{
@@ -46,13 +48,10 @@ order_producer(void *arg)
if (m == NULL)
continue;
- const uint32_t flow = (uintptr_t)m % nb_flows;
+ const flow_id_t flow = (uintptr_t)m % nb_flows;
/* Maintain seq number per flow */
m->seqn = producer_flow_seq[flow]++;
- m->udata64 = flow;
-
- ev.flow_id = flow;
- ev.mbuf = m;
+ flow_id_save(flow, m, &ev);
while (rte_event_enqueue_burst(dev_id, port, &ev, 1) != 1) {
if (t->err)
@@ -139,6 +138,18 @@ order_test_setup(struct evt_test *test, struct evt_options *opt)
{
void *test_order;
+ static const struct rte_mbuf_dynfield flow_id_dynfield_desc = {
+ .name = "test_event_dynfield_flow_id",
+ .size = sizeof(flow_id_t),
+ .align = __alignof__(flow_id_t),
+ };
+ flow_id_dynfield_offset =
+ rte_mbuf_dynfield_register(&flow_id_dynfield_desc);
+ if (flow_id_dynfield_offset < 0) {
+ evt_err("failed to register mbuf field");
+ return -rte_errno;
+ }
+
test_order = rte_zmalloc_socket(test->name, sizeof(struct test_order),
RTE_CACHE_LINE_SIZE, opt->socket_id);
if (test_order == NULL) {
@@ -13,6 +13,7 @@
#include <rte_lcore.h>
#include <rte_malloc.h>
#include <rte_mbuf.h>
+#include <rte_mbuf_dyn.h>
#include "evt_common.h"
#include "evt_options.h"
@@ -20,6 +21,24 @@
#define BURST_SIZE 16
+typedef uint32_t flow_id_t;
+extern int flow_id_dynfield_offset;
+
+static inline void
+flow_id_copy_from_mbuf(struct rte_event *event)
+{
+ event->flow_id = *RTE_MBUF_DYNFIELD(event->mbuf,
+ flow_id_dynfield_offset, flow_id_t *);
+}
+
+static inline void
+flow_id_save(flow_id_t flow, struct rte_mbuf *mbuf, struct rte_event *event)
+{
+ *RTE_MBUF_DYNFIELD(mbuf, flow_id_dynfield_offset, flow_id_t *) = flow;
+ event->flow_id = flow;
+ event->mbuf = mbuf;
+}
+
struct test_order;
struct worker_data {
@@ -35,7 +35,7 @@ order_queue_worker(void *arg, const bool flow_id_cap)
}
if (!flow_id_cap)
- ev.flow_id = ev.mbuf->udata64;
+ flow_id_copy_from_mbuf(&ev);
if (ev.queue_id == 0) { /* from ordered queue */
order_queue_process_stage_0(&ev);
@@ -73,7 +73,7 @@ order_queue_worker_burst(void *arg, const bool flow_id_cap)
for (i = 0; i < nb_rx; i++) {
if (!flow_id_cap)
- ev[i].flow_id = ev[i].mbuf->udata64;
+ flow_id_copy_from_mbuf(&ev[i]);
if (ev[i].queue_id == 0) { /* from ordered queue */
order_queue_process_stage_0(&ev[i]);