From: Pavan Nikhilesh Bhagavatula <pbhagavatula@caviumnetworks.com>
To: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
Cc: dev@dpdk.org, nipun.gupta@nxp.com, hemant.agrawal@nxp.com,
nikhil.rao@intel.com, jerin.jacobkollanukkaran@cavium.com
Subject: Re: [dpdk-dev] [RFC PATCH v4 2/4] eventtimer: add common code
Date: Wed, 29 Nov 2017 10:49:25 +0530 [thread overview]
Message-ID: <20171129051924.o2q7z6l7zeewd7kf@Pavan-LT> (raw)
In-Reply-To: <1511890808-6072-3-git-send-email-erik.g.carrillo@intel.com>
Hi Gabriel,
Comments inline
On Tue, Nov 28, 2017 at 11:40:06AM -0600, Erik Gabriel Carrillo wrote:
<snip>
> +struct rte_event_timer_adapter *
> +rte_event_timer_adapter_create(const struct rte_event_timer_adapter_conf *conf)
> +{
> + /* default port conf values */
> + struct rte_event_port_conf port_conf = {
> + .new_event_threshold = 128,
> + .dequeue_depth = 32,
> + .enqueue_depth = 32
> + };
Instead of harcoding get the port conf values from
rte_event_port_default_conf_get().
> +
> + return rte_event_timer_adapter_create_ext(conf, default_port_conf_cb,
> + &port_conf);
> +}
> +
> +struct rte_event_timer_adapter *
> +rte_event_timer_adapter_create_ext(
> + const struct rte_event_timer_adapter_conf *conf,
> + rte_event_timer_adapter_port_conf_cb_t conf_cb,
> + void *conf_arg)
> +{
> + uint16_t adapter_id;
> + struct rte_event_timer_adapter *adapter;
> + const struct rte_memzone *mz;
> + char mz_name[DATA_MZ_NAME_MAX_LEN];
> + int n, ret;
> + struct rte_eventdev *dev;
> +
> + if (conf == NULL) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + /* Check eventdev ID */
> + if (!rte_event_pmd_is_valid_dev(conf->event_dev_id)) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> + dev = &rte_eventdevs[conf->event_dev_id];
> +
> + adapter_id = conf->timer_adapter_id;
> +
> + /* Check adapter ID not already allocated */
> + adapter = &adapters[adapter_id];
> + if (adapter->allocated) {
> + rte_errno = -EEXIST;
> + return NULL;
> + }
> +
> + /* Create shared data area. */
> + n = snprintf(mz_name, sizeof(mz_name), DATA_MZ_NAME_FORMAT, adapter_id);
> + if (n >= (int)sizeof(mz_name)) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> + mz = rte_memzone_reserve(mz_name,
> + sizeof(struct rte_event_timer_adapter_data),
> + conf->socket_id, 0);
> + if (mz == NULL)
> + /* rte_errno set by rte_memzone_reserve */
> + return NULL;
> +
> + adapter->data = mz->addr;
> + memset(adapter->data, 0, sizeof(struct rte_event_timer_adapter_data));
> +
> + adapter->data->mz = mz;
> + adapter->data->event_dev_id = conf->event_dev_id;
> + adapter->data->id = adapter_id;
> + adapter->data->socket_id = conf->socket_id;
> + adapter->data->conf = *conf; /* copy conf structure */
> +
> + /* Query eventdev PMD for timer adapter capabilities and ops */
> + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + &adapter->data->caps,
> + &adapter->ops);
The underlying driver needs info about the adapter flags i.e.
RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES and RTE_EVENT_TIMER_ADAPTER_F_SP_PUT so we
need to pass those too conf->flags.
> + if (ret < 0) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + if (!(adapter->data->caps &
> + RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)) {
> + FUNC_PTR_OR_NULL_RET_WITH_ERRNO(conf_cb, -EINVAL);
> + ret = conf_cb(adapter->data->id, adapter->data->event_dev_id,
> + &adapter->data->event_port_id, conf_arg);
> + if (ret < 0) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> + }
> +
> + /* Allow driver to do some setup */
> + FUNC_PTR_OR_NULL_RET_WITH_ERRNO(adapter->ops->init, -ENOTSUP);
> + ret = adapter->ops->init(adapter);
> + if (ret < 0) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + /* Set fast-path function pointers */
> + adapter->arm_burst = adapter->ops->arm_burst;
> + adapter->arm_tmo_tick_burst = adapter->ops->arm_tmo_tick_burst;
> + adapter->cancel_burst = adapter->ops->cancel_burst;
> +
> + adapter->allocated = 1;
> +
> + return adapter;
> +}
> +
> +struct rte_event_timer_adapter *
> +rte_event_timer_adapter_lookup(uint16_t adapter_id)
> +{
> + char name[DATA_MZ_NAME_MAX_LEN];
> + const struct rte_memzone *mz;
> + struct rte_event_timer_adapter_data *data;
> + struct rte_event_timer_adapter *adapter;
> + int ret;
> + struct rte_eventdev *dev;
> +
> + if (adapters[adapter_id].allocated)
> + return &adapters[adapter_id]; /* Adapter is already loaded */
> +
> + snprintf(name, DATA_MZ_NAME_MAX_LEN, DATA_MZ_NAME_FORMAT, adapter_id);
> + mz = rte_memzone_lookup(name);
> + if (mz == NULL) {
> + rte_errno = -ENOENT;
> + return NULL;
> + }
> +
> + data = mz->addr;
> +
> + adapter = &adapters[data->id];
> + adapter->data = data;
> +
> + dev = &rte_eventdevs[adapter->data->event_dev_id];
> +
> + /* Query eventdev PMD for timer adapter capabilities and ops */
> + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + &adapter->data->caps,
> + &adapter->ops);
Same as above.
> + if (ret < 0) {
> + rte_errno = -EINVAL;
> + return NULL;
> + }
> +
> + /* Set fast-path function pointers */
> + adapter->arm_burst = adapter->ops->arm_burst;
> + adapter->arm_tmo_tick_burst = adapter->ops->arm_tmo_tick_burst;
> + adapter->cancel_burst = adapter->ops->cancel_burst;
> +
> + adapter->allocated = 1;
> +
> + return adapter;
> +}
> +
<snip>
> +int
> +rte_event_timer_arm_burst(const struct rte_event_timer_adapter *adapter,
> + struct rte_event_timer **event_timers,
> + uint16_t nb_event_timers)
> +{
> + int ret;
> +
> + ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> + FUNC_PTR_OR_ERR_RET(adapter->arm_burst, -EINVAL);
> +
> + if (!adapter->data->started)
> + return -EAGAIN;
These checks are datapath heavy so enable them under debug compilation.
> +
> + ret = adapter->arm_burst(adapter, event_timers, nb_event_timers);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
when burst is called we need to return the number of timers successfully set
and free the failures. Return the result directly.
> +}
> +
> +int
> +rte_event_timer_arm_tmo_tick_burst(
> + const struct rte_event_timer_adapter *adapter,
> + struct rte_event_timer **event_timers,
> + const uint64_t timeout_ticks,
> + const uint16_t nb_event_timers)
> +{
> + int ret;
> +
> + ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> + FUNC_PTR_OR_ERR_RET(adapter->arm_tmo_tick_burst, -EINVAL);
> +
> + if (!adapter->data->started)
> + return -EAGAIN;
> +
> + for (int i = 0; i < nb_event_timers; i++)
> + event_timers[i]->timeout_ticks = timeout_ticks;
> +
> + ret = adapter->arm_tmo_tick_burst(adapter, event_timers, timeout_ticks,
> + nb_event_timers);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
Same as above.
> +}
> +
> +int
> +rte_event_timer_cancel_burst(const struct rte_event_timer_adapter *adapter,
> + struct rte_event_timer **event_timers,
> + uint16_t nb_event_timers)
> +{
> + int ret;
> +
> + ADAPTER_VALID_OR_ERR_RET(adapter, -EINVAL);
> + FUNC_PTR_OR_ERR_RET(adapter->cancel_burst, -EINVAL);
> +
> + if (!adapter->data->started)
> + return -EAGAIN;
> +
> + ret = adapter->cancel_burst(adapter, event_timers, nb_event_timers);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
Same as above.
> +}
> diff --git a/lib/librte_eventdev/rte_event_timer_adapter_driver.h b/lib/librte_eventdev/rte_event_timer_adapter_driver.h
Consider naming this _pmd.h for consistency
> new file mode 100644
> index 0000000..485fad1
> --- /dev/null
> +++ b/lib/librte_eventdev/rte_event_timer_adapter_driver.h
> @@ -0,0 +1,159 @@
Regards,
Pavan.
next prev parent reply other threads:[~2017-11-29 5:19 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-17 16:11 [dpdk-dev] [RFC PATCH 0/1] eventtimer: introduce event timer wheel Jerin Jacob
2017-08-17 16:11 ` [dpdk-dev] [RFC PATCH 1/1] " Jerin Jacob
2017-08-23 22:57 ` [dpdk-dev] [RFC PATCH 0/1] " Carrillo, Erik G
2017-08-25 10:25 ` Jerin Jacob
2017-08-29 15:02 ` Thomas Monjalon
2017-08-29 15:41 ` Jerin Jacob
2017-08-29 15:48 ` Thomas Monjalon
2017-08-29 16:07 ` Jerin Jacob
2017-09-22 15:17 ` [dpdk-dev] [RFC PATCH v2 0/1] eventtimer: introduce event timer adapter Erik Gabriel Carrillo
2017-09-22 15:17 ` [dpdk-dev] [RFC PATCH v2 1/1] " Erik Gabriel Carrillo
2017-10-03 14:37 ` [dpdk-dev] [RFC PATCH v2 0/1] " Jerin Jacob
2017-10-09 20:30 ` Carrillo, Erik G
2017-10-16 12:04 ` Pavan Nikhilesh Bhagavatula
2017-10-16 12:37 ` Pavan Nikhilesh Bhagavatula
2017-10-18 21:48 ` Carrillo, Erik G
2017-10-26 15:45 ` Pavan Nikhilesh Bhagavatula
2017-11-20 22:35 ` [dpdk-dev] [RFC PATCH v3 " Erik Gabriel Carrillo
2017-11-20 22:35 ` [dpdk-dev] [RFC PATCH v3 1/1] " Erik Gabriel Carrillo
2017-11-23 4:37 ` Pavan Nikhilesh Bhagavatula
2017-11-27 14:47 ` Carrillo, Erik G
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 0/4] " Erik Gabriel Carrillo
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 1/4] " Erik Gabriel Carrillo
2017-11-29 10:29 ` Pavan Nikhilesh Bhagavatula
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 2/4] eventtimer: add common code Erik Gabriel Carrillo
2017-11-29 5:19 ` Pavan Nikhilesh Bhagavatula [this message]
2017-11-30 20:59 ` Carrillo, Erik G
2017-12-01 5:13 ` Pavan Nikhilesh Bhagavatula
2017-12-01 20:19 ` Carrillo, Erik G
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 3/4] eventtimer: add default software implementation stub Erik Gabriel Carrillo
2017-11-29 10:34 ` Pavan Nikhilesh Bhagavatula
2017-11-30 23:56 ` Carrillo, Erik G
2017-12-01 5:15 ` Pavan Nikhilesh Bhagavatula
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 4/4] test: add event timer adapter auto-test Erik Gabriel Carrillo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171129051924.o2q7z6l7zeewd7kf@Pavan-LT \
--to=pbhagavatula@caviumnetworks.com \
--cc=dev@dpdk.org \
--cc=erik.g.carrillo@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacobkollanukkaran@cavium.com \
--cc=nikhil.rao@intel.com \
--cc=nipun.gupta@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).