From: "Rao, Nikhil" <nikhil.rao@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Cc: bruce.richardson@intel.com, gage.eads@intel.com, dev@dpdk.org,
thomas@monjalon.net, harry.van.haaren@intel.com,
hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
narender.vangati@intel.com, erik.g.carrillo@intel.com,
abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
Subject: Re: [dpdk-dev] [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter
Date: Sun, 24 Sep 2017 17:44:06 +0530 [thread overview]
Message-ID: <d24a501c-a193-f13a-9c12-20f78d7ea27d@intel.com> (raw)
In-Reply-To: <20170921154608.GC20126@jerin>
On 9/21/2017 9:16 PM, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Fri, 22 Sep 2017 02:47:11 +0530
>> From: Nikhil Rao <nikhil.rao@intel.com>
>> To: jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com
>> CC: gage.eads@intel.com, dev@dpdk.org, thomas@monjalon.net,
>> harry.van.haaren@intel.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com,
>> narender.vangati@intel.com, erik.g.carrillo@intel.com,
>> abhinandan.gujjar@intel.com, santosh.shukla@caviumnetworks.com
>> Subject: [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for
>> rte_event_eth_rx_adapter
>> X-Mailer: git-send-email 2.7.4
>>
>> The caps API allows application to get information
>> needed to configure the ethernet receive adapter for the eventdev and
>> ethdev pair.
>>
>> The PMD callbacks are used by the rte_event_eth_rx_xxx() APIs to
>> configure and control the ethernet receive adapter if packet transfers
>> from the ethdev to eventdev is implemented in hardware.
>
> IMO, it better to split the cap PMD API and cap normative API into one patch and
> remaining PMD API to another patch.
OK.
>
>>
>> For e.g., the ethdev, eventdev pairing maybe such that all of the
>> Eth Rx queues can only be connected to a single event queue, in
>> which case the application is required to pass in -1 as the queue id
>> when adding a receive queue to the adapter.
>>
>> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
>> ---
>> lib/librte_eventdev/rte_eventdev.h | 33 +++++
>> lib/librte_eventdev/rte_eventdev_pmd.h | 173 +++++++++++++++++++++++++++
>> lib/librte_eventdev/rte_eventdev.c | 24 ++++
>> lib/librte_eventdev/rte_eventdev_version.map | 8 ++
>> 4 files changed, 238 insertions(+)
>>
>> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
>> index 128bc5221..a8bebac01 100644
>> --- a/lib/librte_eventdev/rte_eventdev.h
>> +++ b/lib/librte_eventdev/rte_eventdev.h
>> @@ -990,6 +990,39 @@ struct rte_event {
>> };
>> };
>>
>> +/* Ethdev Rx adapter capability bitmap flags */
>> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT 0x1
>> +/**< Eventdev can send packets to ethdev using internal event port */
>> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ 0x2
>> +/**< Ethdev Rx queues can be connected to single event queue */
>
> I think, Its is more of limitation. Since we are expressing it as
> capability. How about changing it as RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ
> (same as exiting !RTE_EVENT_ETH_RX_ADAPTER_CAP_SINGLE_EVENTQ and SW driver has this capability)
> i.e Ethdev Rx queues of single ethdev port can be connected to multiple
> event queue.
>
OK. I agree that the MULTI_EVENTQ is better suited to be expressed as a
capability.
>> +#define RTE_EVENT_ETH_RX_ADAPTER_CAP_FLOW_ID 0x4
>> +/**< Ethdev Rx adapter can set flow ID for event queue, if this flag
>> + * is unset, the application needs to provide a flow id when adding
>
> Looking at implementation, If I understand it correctly, it not application
> "needs" to provide instead, it is application can provide. If so, I think,
> making it as RTE_EVENT_ETH_RX_ADAPTER_CAP_SET_FLOW_ID or
> RTE_EVENT_ETH_RX_ADAPTER_CAP_OVERRIDE_FLOW_ID makes more sense.
>
If the FLOW_ID cap is not set, it is required for the application to
provide it, else the application optionally can provide it but the
feature of the application being able to provide (override) the flag
should be a separate flag.
If it's only the override behavior that is required, we can rename the
flag to OVERRIDE_FLOW_ID.
> something like,
>
> #define RTE_EVENT_ETH_RX_ADAPTER_CAP_SET_FLOW_ID 0x4
> /**< If this flag is set then the application can override the default
> * hash based flow id generated by the Rx adapter when the event
> * enqueues to the event queue.
> *
> * @see rte_event_eth_rx_adapter_queue_conf::rx_queue_flags
> * @see rte_event_eth_rx_adapter_queue_conf::ev::flow_id
> */
>
>>
>> +int
>> +rte_event_eth_rx_adapter_caps_get(uint8_t dev_id, uint8_t eth_port_id,
>> + uint32_t *caps)
>> +{
>> + struct rte_eventdev *dev;
>> +
>> + RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(eth_port_id, -EINVAL);
>> +
>> + dev = &rte_eventdevs[dev_id];
>> +
>> + if (caps == NULL)
>> + return -EINVAL;
>> + *caps = 0;
>> +
>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_adapter_caps_get,
>> + -ENOTSUP);
>
> How about not returning the -ENOTSUP. Instead just call
> eth_rx_adapter_caps_get if it not NULL. By this way if a PMD does not
> have any cap, it does not need to implement this hook.
OK.
>
>> + return (*dev->dev_ops->eth_rx_adapter_caps_get)
>> + (dev,
>> + &rte_eth_devices[eth_port_id],
>> + caps);
>> +}
>> +
>> static inline int
>> rte_event_dev_queue_config(struct rte_eventdev *dev, uint8_t nb_queues)
>> {
>> diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
>> index 4c48e5f0a..996b361a5 100644
>> --- a/lib/librte_eventdev/rte_eventdev_version.map
>> +++ b/lib/librte_eventdev/rte_eventdev_version.map
>> @@ -51,3 +51,11 @@ DPDK_17.08 {
>> rte_event_ring_init;
>> rte_event_ring_lookup;
>> } DPDK_17.05;
>> +
>> +
> Additional line feed.
>
>> +DPDK_17.11 {
>> + global:
>> +
>> + rte_event_eth_rx_adapter_caps_get;
>> +
>> +} DPDK_17.08;
>> --
>> 2.14.1.145.gb3622a4
>>
>
next prev parent reply other threads:[~2017-09-24 12:14 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-21 21:17 [dpdk-dev] [PATCH v4 0/4] eventdev: cover letter: ethernet Rx queue event adapter Nikhil Rao
2017-09-21 21:17 ` [dpdk-dev] [PATCH v4 1/4] eventdev: Add caps API and PMD callbacks for rte_event_eth_rx_adapter Nikhil Rao
2017-09-21 15:46 ` Jerin Jacob
2017-09-24 12:14 ` Rao, Nikhil [this message]
2017-10-02 8:48 ` Jerin Jacob
2017-09-21 21:17 ` [dpdk-dev] [PATCH v4 2/4] eventdev: Add ethernet Rx adapter caps function to eventdev SW PMD Nikhil Rao
2017-09-22 2:49 ` Jerin Jacob
2017-09-22 5:27 ` santosh
2017-09-21 21:17 ` [dpdk-dev] [PATCH v4 3/4] eventdev: Add eventdev ethernet Rx adapter Nikhil Rao
2017-09-21 15:43 ` Pavan Nikhilesh Bhagavatula
2017-09-23 11:35 ` Rao, Nikhil
2017-10-03 9:09 ` Pavan Nikhilesh Bhagavatula
2017-09-22 6:08 ` santosh
2017-10-02 10:20 ` Rao, Nikhil
2017-09-22 9:10 ` Jerin Jacob
2017-09-24 18:16 ` Rao, Nikhil
2017-09-25 2:59 ` Rao, Nikhil
2017-10-02 10:28 ` Rao, Nikhil
2017-10-02 10:39 ` Jerin Jacob
2017-10-05 8:54 ` Rao, Nikhil
2017-10-03 13:52 ` Jerin Jacob
2017-10-05 8:12 ` Rao, Nikhil
2017-09-21 21:17 ` [dpdk-dev] [PATCH v4 4/4] eventdev: Add tests for event eth Rx adapter APIs Nikhil Rao
2017-09-22 12:12 ` Jerin Jacob
2017-09-24 18:24 ` Rao, Nikhil
2017-10-02 10:31 ` Jerin Jacob
2017-10-04 11:28 ` Rao, Nikhil
2017-10-03 11:36 ` Pavan Nikhilesh Bhagavatula
2017-10-05 5:57 ` Rao, Nikhil
2017-10-05 8:08 ` Pavan Nikhilesh Bhagavatula
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=d24a501c-a193-f13a-9c12-20f78d7ea27d@intel.com \
--to=nikhil.rao@intel.com \
--cc=abhinandan.gujjar@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=erik.g.carrillo@intel.com \
--cc=gage.eads@intel.com \
--cc=harry.van.haaren@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=narender.vangati@intel.com \
--cc=nipun.gupta@nxp.com \
--cc=santosh.shukla@caviumnetworks.com \
--cc=thomas@monjalon.net \
/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).