Re: [dpdk-dev] [PATCH v7 2/7] eventtimer: add common code

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Thu, 8 Mar 2018 15:54:01 -0600
> From: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> To: pbhagavatula@caviumnetworks.com
> CC: dev@dpdk.org, jerin.jacob@caviumnetworks.com, nipun.gupta@nxp.com,
>  hemant.agrawal@nxp.com
> Subject: [PATCH v7 2/7] eventtimer: add common code
> X-Mailer: git-send-email 1.7.10
> 
> This commit adds the logic that is shared by all event timer adapter
> drivers; the common code handles instance allocation and some
> initialization.
> 
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>

It fails to build at least on clang 5.0(GCC 7.3 builds fine)

/export/dpdk-next-eventdev/lib/librte_eventdev/rte_event_timer_adapter.c:280:10:
error: format specifies type 'unsigned short' but the argument has type
'uint8_t' (aka 'unsigned char') [-Werror,-Wformat]
                              adapter->data->id);
                              ^~~~~~~~~~~~~~~~~
/export/dpdk-next-eventdev/lib/librte_eventdev/rte_event_timer_adapter.c:36:58:
note: expanded from macro 'EVTIM_LOG_ERR'
#define EVTIM_LOG_ERR(...) EVTIM_LOG(ERR, evtim_logtype, __VA_ARGS__)
                                                         ^~~~~~~~~~~
/export/dpdk-next-eventdev/lib/librte_eventdev/rte_event_timer_adapter.c:34:43:
note: expanded from macro 'EVTIM_LOG'
                        "\n", __func__, __LINE__,
RTE_FMT_TAIL(__VA_ARGS__,)))
                                                               ^~~~~~~~~~~
/export/dpdk-next-eventdev/build/include/rte_common.h:382:32: note:
expanded from macro 'RTE_FMT_TAIL'
#define RTE_FMT_TAIL(fmt, ...) __VA_ARGS__
                               ^~~~~~~~~~~
/export/dpdk-next-eventdev/build/include/rte_common.h:380:39: note:
expanded from macro 'RTE_FMT'
#define RTE_FMT(fmt, ...) fmt "%.0s", __VA_ARGS__ ""
                                      ^~~~~~~~~~~
/export/dpdk-next-eventdev/lib/librte_eventdev/rte_event_timer_adapter.c:350:28:
error: format specifies type 'unsigned short' but the argument has type
'uint8_t' (aka 'unsigned char') [-Werror,-Wformat]
                              "before freeing", adapter->data->id);
                                                ^~~~~~~~~~~~~~~~~
/export/dpdk-next-eventdev/lib/librte_eventdev/rte_event_timer_adapter.c:36:58:
note: expanded from macro 'EVTIM_LOG_ERR'
#define EVTIM_LOG_ERR(...) EVTIM_LOG(ERR, evtim_logtype, __VA_ARGS__)
                                                         ^~~~~~~~~~~
/export/dpdk-next-eventdev/lib/librte_eventdev/rte_event_timer_adapter.c:34:43:
note: expanded from macro 'EVTIM_LOG'
                        "\n", __func__, __LINE__,
RTE_FMT_TAIL(__VA_ARGS__,)))
                                                               ^~~~~~~~~~~
/export/dpdk-next-eventdev/build/include/rte_common.h:382:32: note:
expanded from macro 'RTE_FMT_TAIL'
#define RTE_FMT_TAIL(fmt, ...) __VA_ARGS__
                               ^~~~~~~~~~~
/export/dpdk-next-eventdev/build/include/rte_common.h:380:39: note:
expanded from macro 'RTE_FMT'
#define RTE_FMT(fmt, ...) fmt "%.0s", __VA_ARGS__ ""


> ---
>  config/common_base                                |   1 +
>  drivers/event/sw/sw_evdev.c                       |  18 +
>  lib/librte_eventdev/Makefile                      |   2 +
>  lib/librte_eventdev/rte_event_timer_adapter.c     | 459 ++++++++++++++++++++++
>  lib/librte_eventdev/rte_event_timer_adapter_pmd.h | 150 +++++++
>  lib/librte_eventdev/rte_eventdev.h                |   3 +
>  lib/librte_eventdev/rte_eventdev_pmd.h            |  35 ++
>  lib/librte_eventdev/rte_eventdev_version.map      |  20 +
>  8 files changed, 688 insertions(+)
>  create mode 100644 lib/librte_eventdev/rte_event_timer_adapter.c
>  create mode 100644 lib/librte_eventdev/rte_event_timer_adapter_pmd.h
> 
> diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
> index 549b182..8b16e3f 100644
> --- a/lib/librte_eventdev/Makefile
> +++ b/lib/librte_eventdev/Makefile
> @@ -20,6 +20,7 @@ LDLIBS += -lrte_eal -lrte_ring -lrte_ethdev -lrte_hash
>  SRCS-y += rte_eventdev.c
>  SRCS-y += rte_event_ring.c
>  SRCS-y += rte_event_eth_rx_adapter.c
> +SRCS-y += rte_event_timer_adapter.c
>  
>  # export include files
>  SYMLINK-y-include += rte_eventdev.h
> @@ -29,6 +30,7 @@ SYMLINK-y-include += rte_eventdev_pmd_vdev.h
>  SYMLINK-y-include += rte_event_ring.h
>  SYMLINK-y-include += rte_event_eth_rx_adapter.h
>  SYMLINK-y-include += rte_event_timer_adapter.h
> +SYMLINK-y-include += rte_event_timer_adapter_pmd.h
>  
>  # versioning export map
>  EXPORT_MAP := rte_eventdev_version.map
> diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c b/lib/librte_eventdev/rte_event_timer_adapter.c
> new file mode 100644
> index 0000000..711d6b9
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_timer_adapter.c
> +void __rte_experimental
> +rte_event_timer_init(struct rte_event_timer *evtim)
> +{
> +	evtim->ev.op = RTE_EVENT_OP_NEW;
> +	evtim->ev.event_type = RTE_EVENT_TYPE_TIMER;
> +	evtim->state = RTE_EVENT_TIMER_NOT_ARMED;
> +}
> +
> +int __rte_experimental
> +rte_event_timer_arm_burst(const struct rte_event_timer_adapter *adapter,
> +			  struct rte_event_timer **evtims,
> +			  uint16_t nb_evtims)
> +{
> +#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> +	ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> +	FUNC_PTR_OR_ERR_RET(adapter->arm_burst, -EINVAL);
> +#endif
> +
> +	return adapter->arm_burst(adapter, evtims, nb_evtims);
> +}
> +
> +int __rte_experimental
> +rte_event_timer_arm_tmo_tick_burst(
> +			const struct rte_event_timer_adapter *adapter,
> +			struct rte_event_timer **evtims,
> +			const uint64_t timeout_ticks,
> +			const uint16_t nb_evtims)
> +{
> +#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> +	ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> +	FUNC_PTR_OR_ERR_RET(adapter->arm_tmo_tick_burst, -EINVAL);
> +#endif
> +
> +	return adapter->arm_tmo_tick_burst(adapter, evtims, timeout_ticks,
> +					   nb_evtims);
> +}
> +
> +int __rte_experimental
> +rte_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
> +			     struct rte_event_timer **evtims,
> +			     uint16_t nb_evtims)
> +{
> +#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> +	ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> +	FUNC_PTR_OR_ERR_RET(adapter->cancel_burst, -EINVAL);
> +#endif
> +
> +	return adapter->cancel_burst(adapter, evtims, nb_evtims);
> +}

Please move fastpath functions to header file as "static inline".
That would avoid function all overhead for fastpath functions.
Even though it makes header file bit ugly, We are following the
same procedure across other subsystems in DPDK to get performance advantage.


> +
> +#endif /* __RTE_EVENT_TIMER_ADAPTER_PMD_H__ */
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index f9ad71e..888bcf1 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -1046,6 +1046,9 @@ struct rte_event {
>   * @see struct rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
>   */
>  
> +#define RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT (1ULL << 1)

1ULL << 0 ??

> +/**< This flag is set when the timer mechanism is in HW. */
> +
> +
> +EXPERIMENTAL {
> +	global:
> +
> +	rte_event_timer_adapter_create;
> +	rte_event_timer_adapter_create_ext;
> +	rte_event_timer_adapter_free;
> +	rte_event_timer_adapter_get_info;
> +	rte_event_timer_adapter_lookup;
> +	rte_event_timer_adapter_service_id_get;
> +	rte_event_timer_adapter_service_id_get;

Duplicate entry

> +	rte_event_timer_adapter_start;
> +	rte_event_timer_adapter_stats_get;
> +	rte_event_timer_adapter_stats_reset;
> +	rte_event_timer_adapter_stop;
> +	rte_event_timer_init;
> +	rte_event_timer_arm_burst;
> +	rte_event_timer_arm_tmo_tick_burst;
> +	rte_event_timer_cancel_burst;
> +} DPDK_18.02;
> -- 
> 2.6.4
>