Hi Abhinandan, Please see my comments inline.
Thanks Ankur >-----Original Message----- >From: Ankur Dwivedi >Sent: Friday, September 18, 2020 8:34 PM >To: 'Gujjar, Abhinandan S' <abhinandan.guj...@intel.com>; dev@dpdk.org >Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Anoob Joseph ><ano...@marvell.com> >Subject: RE: [PATCH 1/3] test/event_crypto_adapter: return error with >unsupported mode > >Hi Abhinandan, > >Please see my comments inline. > >Thanks >Ankur > >>-----Original Message----- >>From: Gujjar, Abhinandan S <abhinandan.guj...@intel.com> >>Sent: Friday, September 18, 2020 8:13 PM >>To: Ankur Dwivedi <adwiv...@marvell.com>; dev@dpdk.org >>Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Anoob Joseph >><ano...@marvell.com> >>Subject: [EXT] RE: [PATCH 1/3] test/event_crypto_adapter: return error >>with unsupported mode >> >>External Email >> >>---------------------------------------------------------------------- >>Hi Ankur, >> >>Please see comments inline. >> >>Regards >>Abhinandan >> >>> -----Original Message----- >>> From: Ankur Dwivedi <adwiv...@marvell.com> >>> Sent: Friday, September 18, 2020 8:05 PM >>> To: Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; dev@dpdk.org >>> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Anoob Joseph >>> <ano...@marvell.com> >>> Subject: RE: [PATCH 1/3] test/event_crypto_adapter: return error with >>> unsupported mode >>> >>> Hi Abhinandan, >>> >>> Please see my comments inline. >>> >>> Thanks >>> Ankur >>> >>> >-----Original Message----- >>> >From: Gujjar, Abhinandan S <abhinandan.guj...@intel.com> >>> >Sent: Friday, September 18, 2020 12:41 PM >>> >To: Ankur Dwivedi <adwiv...@marvell.com>; dev@dpdk.org >>> >Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Anoob Joseph >>> ><ano...@marvell.com> >>> >Subject: [EXT] RE: [PATCH 1/3] test/event_crypto_adapter: return >>> >error with unsupported mode >>> > >>> >External Email >>> > >>> >-------------------------------------------------------------------- >>> >- >>> >- >>> >Hi Ankur, >>> > >>> >This patch requires minor change. Please find the comment inline. >>> > >>> >> -----Original Message----- >>> >> From: Gujjar, Abhinandan S >>> >> Sent: Friday, September 11, 2020 1:07 PM >>> >> To: Ankur Dwivedi <adwiv...@marvell.com>; dev@dpdk.org >>> >> Cc: jer...@marvell.com; ano...@marvell.com >>> >> Subject: RE: [PATCH 1/3] test/event_crypto_adapter: return error >>> >> with unsupported mode >>> >> >>> >> Acked-by: abhinandan.guj...@intel.com (For series) >>> >> >>> >> > -----Original Message----- >>> >> > From: Ankur Dwivedi <adwiv...@marvell.com> >>> >> > Sent: Tuesday, September 8, 2020 2:46 PM >>> >> > To: dev@dpdk.org >>> >> > Cc: Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; >>> >> > jer...@marvell.com; ano...@marvell.com; Ankur Dwivedi >>> >> > <adwiv...@marvell.com> >>> >> > Subject: [PATCH 1/3] test/event_crypto_adapter: return error >>> >> > with unsupported mode >>> >> > >>> >> > The capability of a event device should be checked before >>> >> > creating a event crypto adapter in a particular mode. The test >>> >> > case returns error if the mode is not supported. >>> >> > >>> >> > Signed-off-by: Ankur Dwivedi <adwiv...@marvell.com> >>> >> > --- >>> >> > app/test/test_event_crypto_adapter.c | 30 >>> >> > ++++++++++++++++++---------- >>> >> > 1 file changed, 20 insertions(+), 10 deletions(-) >>> >> > >>> >> > diff --git a/app/test/test_event_crypto_adapter.c >>> >> > b/app/test/test_event_crypto_adapter.c >>> >> > index 8d42462d8..930c2a9bc 100644 >>> >> > --- a/app/test/test_event_crypto_adapter.c >>> >> > +++ b/app/test/test_event_crypto_adapter.c >>> >> > @@ -750,15 +750,23 @@ configure_event_crypto_adapter(enum >>> >> > rte_event_crypto_adapter_mode mode) >>> >> > uint32_t cap; >>> >> > int ret; >>> >> > >>> >> > + ret = rte_event_crypto_adapter_caps_get(evdev, >TEST_CDEV_ID, >>> >> > &cap); >>> >> > + TEST_ASSERT_SUCCESS(ret, "Failed to get adapter >>> >> > +capabilities\n"); >>> >> > + >>> >> > + if ((mode == RTE_EVENT_CRYPTO_ADAPTER_OP_FORWARD) >&& >>> >> > + !(cap & >>> >> > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD)) >>> >> > + return -ENOTSUP; >>> >> > + >>> >> > + if ((mode == RTE_EVENT_CRYPTO_ADAPTER_OP_NEW) && >>> >> > + !(cap & >>> >> > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_NEW)) >>> >> > + return -ENOTSUP; >>> >> > + >>> >SW adapter does not have *_INTERNAL_PORT_* caps. Just noticed test >>> >was failing here. >>> >Could you please bypass this check for "event_sw" and repost the patch? >>> [Ankur] I was thinking of handling this by reading >>> rte_eventdevs[evdev].data->name. >>> If the name matches event_sw0(which is the default device name), I >>> will bypass if checks. >>> Can the event_sw device name be anything else? >>> Or, if there is a better way of handling this please let me know. >>Yes. You can also use rte_event_dev_info_get(evdev, &info) and check >>info.driver_name for "event_sw". >[Ankur] Thanks for the suggestion. I will try this and make the required >changes in v2. [Ankur] I thought of an alternative to implement the above. If the capability does not have any *_INTERNAL_PORT_* caps, it will be a SW PMD. Is this correct? In that case capability will be used to skip the if checks instead of the pmd specific name comparison. Please let me know your thoughts on this. >> >>> > >>> >> > /* Create adapter with default port creation callback */ >>> >> > ret = rte_event_crypto_adapter_create(TEST_ADAPTER_ID, >>> >> > - TEST_CDEV_ID, >>> >> > + evdev, >>> >> > &conf, mode); >>> >> > TEST_ASSERT_SUCCESS(ret, "Failed to create event crypto >>> >> adapter\n"); >>> >> > >>> >> > - ret = rte_event_crypto_adapter_caps_get(TEST_ADAPTER_ID, >>> >> > evdev, &cap); >>> >> > - TEST_ASSERT_SUCCESS(ret, "Failed to get adapter >capabilities\n"); >>> >> > - >>> >> > if (cap & >>> >> > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) { >>> >> > ret = >>> >> > rte_event_crypto_adapter_queue_pair_add(TEST_ADAPTER_ID, >>> >> > TEST_CDEV_ID, TEST_CDEV_QP_ID, >>> &response_info); @@ -813,6 >>> >> > +821,8 @@ test_crypto_adapter_conf(enum >>> >> > rte_event_crypto_adapter_mode mode) >>> >> > TEST_ASSERT(ret >= 0, "Failed to link queue >%d " >>> >> > "port=%u\n", qid, >>> >> > > params.crypto_event_port_id); >>> >> > + } else { >>> >> > + return ret; >>> >> > } >>> >> > crypto_adapter_setup_done = 1; >>> >> > } >>> >> > @@ -845,24 +855,24 @@ static int >>> >> > test_crypto_adapter_conf_op_forward_mode(void) >>> >> > { >>> >> > enum rte_event_crypto_adapter_mode mode; >>> >> > + int ret; >>> >> > >>> >> > mode = RTE_EVENT_CRYPTO_ADAPTER_OP_FORWARD; >>> >> > - TEST_ASSERT_SUCCESS(test_crypto_adapter_conf(mode), >>> >> > - "Failed to config crypto adapter"); >>> >> > + ret = test_crypto_adapter_conf(mode); >>> >> > >>> >> > - return TEST_SUCCESS; >>> >> > + return ret; >>> >> > } >>> >> > >>> >> > static int >>> >> > test_crypto_adapter_conf_op_new_mode(void) >>> >> > { >>> >> > enum rte_event_crypto_adapter_mode mode; >>> >> > + int ret; >>> >> > >>> >> > mode = RTE_EVENT_CRYPTO_ADAPTER_OP_NEW; >>> >> > - TEST_ASSERT_SUCCESS(test_crypto_adapter_conf(mode), >>> >> > - "Failed to config crypto adapter"); >>> >> > + ret = test_crypto_adapter_conf(mode); >>> >> > >>> >> > - return TEST_SUCCESS; >>> >> > + return ret; >>> >> > } >>> >> > >>> >> > >>> >> > -- >>> >> > 2.28.0