Re: [dpdk-dev] [PATCH v2 3/4] app/test-eventdev: add Tx adapter support

["Rao, Nikhil" <nikhil.rao@intel.com>]

On 9/5/2018 2:24 PM, Pavan Nikhilesh wrote:
> On Wed, Sep 05, 2018 at 12:24:18PM +0530, Rao, Nikhil wrote:
>> On 9/4/2018 7:42 PM, Pavan Nikhilesh wrote:
>>> Convert existing Tx service based pipeline to Tx adapter based APIs and
>>> simplify worker functions.
>>>
>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
>>> ---
>>>    app/test-eventdev/test_pipeline_atq.c    | 269 ++++++++++++-----------
>>>    app/test-eventdev/test_pipeline_common.c | 206 +++++------------
>>>    app/test-eventdev/test_pipeline_common.h |  62 +++---
>>>    app/test-eventdev/test_pipeline_queue.c  | 241 ++++++++++----------
>>>    4 files changed, 367 insertions(+), 411 deletions(-)
>>>
>>> diff --git a/app/test-eventdev/test_pipeline_common.c b/app/test-eventdev/test_pipeline_common.c
>>> index 832ab8b6e..ab407dbbb 100644
>>> --- a/app/test-eventdev/test_pipeline_common.c
>>> +++ b/app/test-eventdev/test_pipeline_common.c
>>> @@ -5,58 +5,6 @@
>>>
>>>
>>> @@ -215,7 +160,6 @@ pipeline_ethdev_setup(struct evt_test *test, struct evt_options *opt)
>>>    {
>>>        uint16_t i;
>>>        uint8_t nb_queues = 1;
>>> -     uint8_t mt_state = 0;
>>>        struct test_pipeline *t = evt_test_priv(test);
>>>        struct rte_eth_rxconf rx_conf;
>>>        struct rte_eth_conf port_conf = {
>>> @@ -238,13 +182,21 @@ pipeline_ethdev_setup(struct evt_test *test, struct evt_options *opt)
>>>                return -ENODEV;
>>>        }
>>>
>>> +     t->internal_port = 0;
>>>        RTE_ETH_FOREACH_DEV(i) {
>>>                struct rte_eth_dev_info dev_info;
>>>                struct rte_eth_conf local_port_conf = port_conf;
>>> +             uint32_t caps = 0;
>>> +
>>> +             rte_event_eth_tx_adapter_caps_get(opt->dev_id, i, &caps);
>>> +             if ((caps & RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT)) {
>>> +                     t->internal_port = 1;
>>> +             } else if (t->internal_port == 1) {
>>> +                     evt_err("Eventdev can't use %d port", i);
>>> +                     return -EINVAL;
>>> +             }
>>>
>> Shouldn't this function also return -EINVAL for the case where
>> internal_port = 0 for i = 0, and internal_port = 1 for i = 1 ?
> 
> I think it would be better to force all the ports to use the non-internal cap
> mode when we detect that one of the port doesn't have internal port capability
> rather than exiting. This was the behaviour previously it will leave room to
> support both the pipeline models in future.

Agreed.

Nikhil