Re: [dpdk-dev] [PATCH 1/2] eventdev: implement ABI change

["Kinsella, Ray" <mdr@ashroe.eu>]

On 28/09/2020 06:53, Jerin Jacob wrote:
> On Thu, Sep 24, 2020 at 3:25 PM Sunil Kumar Kori <skori@marvell.com> wrote:
>>
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Timothy McDaniel
>>> Sent: Friday, September 11, 2020 10:28 PM
>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>>> mattias.ronnblom@ericsson.com; liang.j.ma@intel.com;
>>> peter.mccarthy@intel.com; nipun.gupta@nxp.com; Pavan Nikhilesh
>>> Bhagavatula <pbhagavatula@marvell.com>; dev@dpdk.org;
>>> erik.g.carrillo@intel.com; gage.eads@intel.com; harry.van.haaren@intel.com;
>>> hemant.agrawal@nxp.com; bruce.richardson@intel.com
>>> Subject: [dpdk-dev] [PATCH 1/2] eventdev: implement ABI change
> 
> Please change subject to "eventdev: express DLB PMD constraints" or
> something similar.
> 
>>>
>>> This commit implements the eventdev ABI changes required by the DLB PMD.
>>>
>>> The DLB hardware does not conform exactly to the eventdev interface.
>>> 1) It has a limit on the number of queues that may be linked to a port.
>>> 2) Some ports are further restricted to a maximum of 1 linked queue.
>>> 3) It does not (currently) have the ability to carry the flow_id as part of the
>>> event (QE) payload.
>>>
>>> Due to the above, we would like to propose the following enhancements.
>>>
>>> 1) Add new fields to the rte_event_dev_info struct. These fields allow the
>>> device to advertise its capabilities so that applications can take the
>>> appropriate actions based on those capabilities.
>>>
>>> 2) Add a new field to the rte_event_dev_config struct. This field allows the
>>> application to specify how many of its ports are limited to a single link, or will
>>> be used in single link mode.
>>>
>>> 3) Replace the dedicated implicit_release_disabled field with a bit field of
>>> explicit port capabilities. The implicit_release_disable functionality is assigned
>>> to one bit, and a port-is-single-link-only attribute is assigned to another, with
>>> the remaining bits available for future assignment.
>>>
>>> Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
>>> ---
>>> drivers/event/dpaa/dpaa_eventdev.c             |  3 +-
>>> drivers/event/dpaa2/dpaa2_eventdev.c           |  5 +-
>>> drivers/event/dsw/dsw_evdev.c                  |  3 +-
>>> drivers/event/octeontx/ssovf_evdev.c           |  5 +-
>>> drivers/event/octeontx2/otx2_evdev.c           |  3 +-
>>> drivers/event/opdl/opdl_evdev.c                |  3 +-
>>> drivers/event/skeleton/skeleton_eventdev.c     |  5 +-
>>> drivers/event/sw/sw_evdev.c                    |  8 ++--
>>> drivers/event/sw/sw_evdev_selftest.c           |  6 +--
>>> lib/librte_eventdev/rte_event_eth_tx_adapter.c |  2 +-
>>> lib/librte_eventdev/rte_eventdev.c             | 66 +++++++++++++++++++++++---
>>> lib/librte_eventdev/rte_eventdev.h             | 51 ++++++++++++++++----
>>> lib/librte_eventdev/rte_eventdev_pmd_pci.h     |  1 -
>>> lib/librte_eventdev/rte_eventdev_trace.h       |  7 +--
>>> lib/librte_eventdev/rte_eventdev_version.map   |  4 +-
> 
> Missed to remove the announced depreciation notice.
> 
> 
>>> 15 files changed, 134 insertions(+), 38 deletions(-)
>>>
>>> /**
>>>  * @internal
>>>  * Wrapper for use by pci drivers as a .remove function to detach a event diff
>>> --git a/lib/librte_eventdev/rte_eventdev_trace.h
>>> b/lib/librte_eventdev/rte_eventdev_trace.h
>>> index 4de6341..5ec43d8 100644
>>> --- a/lib/librte_eventdev/rte_eventdev_trace.h
>>> +++ b/lib/librte_eventdev/rte_eventdev_trace.h
>>> @@ -34,6 +34,7 @@ RTE_TRACE_POINT(
>>>       rte_trace_point_emit_u32(dev_conf-
>>>> nb_event_port_dequeue_depth);
>>>       rte_trace_point_emit_u32(dev_conf-
>>>> nb_event_port_enqueue_depth);
>>>       rte_trace_point_emit_u32(dev_conf->event_dev_cfg);
>>> +      rte_trace_point_emit_u8(dev_conf-
>>>> nb_single_link_event_port_queues);
>>>       rte_trace_point_emit_int(rc);
>>> )
>>>
>> I tried running testpmd with trace support with this patch and got following error:
>>
>> EAL: __rte_trace_point_emit_field():442 CTF field is too long
>> EAL: __rte_trace_point_register():468 missing rte_trace_emit_header() in register fn
>>
>> It is because of memory (384 bytes) for CTF gets exhausted during eventdev_configure.
>> Did you also observe the same issue ? If yes, Then please send a separate patch to increase
>> Maximum buffer size TRACE_CTF_FIELD_SIZE. Recommended size is 448 bytes.
> 
> Please check this and send the next version for this and common code
> change[1]. I would like to merge-common code change first and then the
> DLB drivers.
> 
> [1]
> http://patches.dpdk.org/patch/77466/
> 
> 
>>
>>> @@ -59,7 +60,7 @@ RTE_TRACE_POINT(
>>>       rte_trace_point_emit_i32(port_conf->new_event_threshold);
>>>       rte_trace_point_emit_u16(port_conf->dequeue_depth);
>>>       rte_trace_point_emit_u16(port_conf->enqueue_depth);
>>> -      rte_trace_point_emit_u8(port_conf->disable_implicit_release);
>>> +      rte_trace_point_emit_u32(port_conf->event_port_cfg);
>>>       rte_trace_point_emit_int(rc);
>>> )
>>>
>>> @@ -165,7 +166,7 @@ RTE_TRACE_POINT(
>>>       rte_trace_point_emit_i32(port_conf->new_event_threshold);
>>>       rte_trace_point_emit_u16(port_conf->dequeue_depth);
>>>       rte_trace_point_emit_u16(port_conf->enqueue_depth);
>>> -      rte_trace_point_emit_u8(port_conf->disable_implicit_release);
>>> +      rte_trace_point_emit_u32(port_conf->event_port_cfg);
>>>       rte_trace_point_emit_ptr(conf_cb);
>>>       rte_trace_point_emit_int(rc);
>>> )
>>> @@ -257,7 +258,7 @@ RTE_TRACE_POINT(
>>>       rte_trace_point_emit_i32(port_conf->new_event_threshold);
>>>       rte_trace_point_emit_u16(port_conf->dequeue_depth);
>>>       rte_trace_point_emit_u16(port_conf->enqueue_depth);
>>> -      rte_trace_point_emit_u8(port_conf->disable_implicit_release);
>>> +      rte_trace_point_emit_u32(port_conf->event_port_cfg);
>>> )
>>>
>>> RTE_TRACE_POINT(
>>> diff --git a/lib/librte_eventdev/rte_eventdev_version.map
>>> b/lib/librte_eventdev/rte_eventdev_version.map
>>> index 3d9d0ca..2846d04 100644
>>> --- a/lib/librte_eventdev/rte_eventdev_version.map
>>> +++ b/lib/librte_eventdev/rte_eventdev_version.map
>>> @@ -100,7 +100,6 @@ EXPERIMENTAL {
>>>       # added in 20.05
>>>       __rte_eventdev_trace_configure;
>>>       __rte_eventdev_trace_queue_setup;
>>> -      __rte_eventdev_trace_port_setup;
>>>       __rte_eventdev_trace_port_link;
>>>       __rte_eventdev_trace_port_unlink;
>>>       __rte_eventdev_trace_start;
>>> @@ -134,4 +133,7 @@ EXPERIMENTAL {
>>>       __rte_eventdev_trace_crypto_adapter_queue_pair_del;
>>>       __rte_eventdev_trace_crypto_adapter_start;
>>>       __rte_eventdev_trace_crypto_adapter_stop;
>>> +
>>> +      # changed in 20.11
>>> +      __rte_eventdev_trace_port_setup;
> 
> + Ray, Techboard
> 
> Trace is still experimental. So I assume we don't need any
> depreciation notice to change this.

Correct, no need for a depreciation notice. 
No need to change the map file either. 

>>> };
>>> --
>>> 2.6.4
>>