Re: [dpdk-dev] [PATCH] examples/l3fwd: adding event queue support

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Apr 12, 2018 at 06:09:04AM +0000, Sunil Kumar Kori wrote:
> Gentle reminder to review the RFC.
> 
> Regards
> Sunil Kumar
> 

Hi,

sorry for the delay in review.

/Bruce

> -----Original Message-----
> From: Sunil Kumar Kori [mailto:sunil.kori@nxp.com] 
> Sent: Monday, March 19, 2018 7:15 PM
> To: dev@dpdk.org
> Cc: Sunil Kumar Kori <sunil.kori@nxp.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
> Subject: [PATCH] examples/l3fwd: adding event queue support
> 
> This patch set to add the support for eventdev based queue mode support to the l3fwd application.
> 1. Eventdev support with parallel queue
> 2. Eventdev support with atomic queue
> 
> This patch adds
> - New command line parameter is added named as "dequeue-mode" which
>   identifies dequeue method i.e. dequeue via eventdev or polling
>   (default is polling)
> . If dequeue mode is via:
>          a. eventdev: New parameters are added -e, -a, -l  to cater
> 	    eventdev config, adapter config and link configuration
>             respectively. "--config" option will be invalid in this case.
>          b. poll mode: It will work as of existing way and option for
> 	    eventdev parameters(-e, -a, -l) will be invalid.
> 
> - Functions are added in l3fwd_em.c and l3fwd_lpm.c for packet I/O
>   operation
> 
> The main purpose of this RFC is get comments on the approach.
> This is a *not tested* code.
> 
> Signed-off-by: Sunil Kumar Kori <sunil.kori@nxp.com>
> ---
>  examples/l3fwd/Makefile         |   2 +-
>  examples/l3fwd/l3fwd.h          |  21 ++
>  examples/l3fwd/l3fwd_em.c       | 100 ++++++++
>  examples/l3fwd/l3fwd_eventdev.c | 541 ++++++++++++++++++++++++++++++++++++++++
>  examples/l3fwd/l3fwd_eventdev.h |  85 +++++++
>  examples/l3fwd/l3fwd_lpm.c      | 100 ++++++++
>  examples/l3fwd/main.c           | 318 +++++++++++++++++++----
>  examples/l3fwd/meson.build      |   2 +-
>  8 files changed, 1120 insertions(+), 49 deletions(-)  create mode 100644 examples/l3fwd/l3fwd_eventdev.c  create mode 100644 examples/l3fwd/l3fwd_eventdev.h
> 

My initial impression is that this seems like an awful lot of new code just
to support reading from an eventdev rather than from an ethdev. Looking at
the datapath main function loop, is the only difference there that
rte_eth_rx_burst has been changed to rte_eventdev_dequeue_burst or are
there more significant changes than that?

If this is the case, is this scale of changes really needed to this app?
What about the other examples, how many of them will need to be similarly
updated?

I'm also wondering if it would help, or be useful, to have a vdev type
which wraps an eventdev queue as an ethdev. That would eliminate the need
for the datapath code, and may help abstract away some parts of the setup.
It would also help with re-use if you anticipate wanting to make a similar
change to other apps.

/Bruce