From: santosh <santosh.shukla@caviumnetworks.com>
To: Olivier MATZ <olivier.matz@6wind.com>
Cc: dev@dpdk.org, thomas@monjalon.net,
jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com
Subject: Re: [dpdk-dev] [PATCH v4 2/2] ethdev: get the supported pools for a port
Date: Fri, 29 Sep 2017 12:21:28 +0100 [thread overview]
Message-ID: <12be7257-4512-339f-63b1-c7926fff59e7@caviumnetworks.com> (raw)
In-Reply-To: <ceb160fa-29d6-50e8-c780-8eb182da92e9@caviumnetworks.com>
On Friday 29 September 2017 11:16 AM, santosh wrote:
> Hi Olivier,
>
>
> On Friday 29 September 2017 09:32 AM, Olivier MATZ wrote:
>> On Mon, Sep 25, 2017 at 10:52:31PM +0100, santosh wrote:
>>> Hi Olivier,
>>>
>>>
>>> On Monday 25 September 2017 08:37 AM, Olivier MATZ wrote:
>>>
>>>>> +{
>>>>> + struct rte_eth_dev *dev;
>>>>> + const char *tmp;
>>>>> +
>>>>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>>>> +
>>>>> + if (pool == NULL)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + dev = &rte_eth_devices[port_id];
>>>>> +
>>>>> + if (*dev->dev_ops->pools_ops_supported == NULL) {
>>>>> + tmp = rte_eal_mbuf_default_mempool_ops();
>>>>> + if (!strcmp(tmp, pool))
>>>>> + return 0;
>>>>> + else
>>>>> + return -ENOTSUP;
>>>> I don't understand why we are comparing with
>>>> rte_eal_mbuf_default_mempool_ops().
>>>>
>>>> It means that the result of the function would be influenced
>>>> by the parameter given by the user.
>>> But that will be only for ops not supported case and in that case,
>>> function _must_ make sure that if inputted param is _default_ops_name
>>> then function should return ops supported correct info (whether
>>> returning '0' : Best ops or '1': ops does support
>>> , this part is arguable.. meaning One can say that default_ops ='handle-name' is best possible handle Or
>>> one of handle which platform supports).
>>>
>>>> I think that a PMD that does not implement ->pools_ops_supported
>>>> should always return 1 (mempool is supported).
>> I don't agree.
>> The result of this API (mempool ops supported or not by a PMD)
>> should not depend on what user asks for.
>>
>>> Return 1 says: PMD support this ops..
>>>
>>> So if ops is not supported and func returns with 1, then which ops application will use?
>>> If that ops is default_ops.. then How application will distinguish when to use default ops or
>>> param ops?.. as because in both cases func will return with value 1.
>>>
>>> The approach in the patch takes care of that condition and func will return -ENOTSUP
>>> if (ops not support || inputted param not matching with default ops) otherwise will return
>>> 0 or 1.
>>>
>>> At application side;
>>> For error case: In case of -ENOTSUP, its upto application to use _default_ops or exit.
>>> For good case: 0 or 1 case, func gaurantee that handle is either best handle for pool or pool supports
>>> that handle.. However in your suggestion if ops not supported case returns 1 then application is not
>>> sure which ops to use.. default_ops Or input ops given to func.
>>>
>>> make sense?
>> My proposition is:
>>
>> - what a PMD returns does not depend on used parameter:
>> - 0: best support
>> - 1: support
>> - -ENOTSUP: not supported
>>
>> - if a PMD does not implement the _ops_supported() API, assume all pools
>> are supported (returns 1)
> If API returns 1 for PMD does not implement _ops_supported() case,
> then do we really need -ENOTSUP?
>
> If so then in which case API will return -ENOTSUP error, wondering.
To summarize return value of API (specially for error case):
- 0: best support
- 1: pool support ops or _that_ PMD did not implement _ops_supported().
- -ENODEV: Invalid port id.
- -EINVAL: pool is null.
So to me, We don't need -ENOTSUP error code as return value 1 per your input,
address both case ie.. (Pool does support this ops Or _that_ PMD does not implement _ops_supported()).
is above API return value and their case explanation make sense to you?
If not then could you pl. be more explicit on your error code case detailing and
suggest more about return value '1' case?
As per me: Return value 1 is wearing two hats,
addressing two cases. Correct me if I misunderstood your proposition?
Thanks.
next prev parent reply other threads:[~2017-09-29 11:21 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-01 8:05 [dpdk-dev] [PATCH 0/2] Allow application set mempool handle Santosh Shukla
2017-06-01 8:05 ` [dpdk-dev] [PATCH 1/2] eal: Introducing option to " Santosh Shukla
2017-06-30 14:12 ` Olivier Matz
2017-07-04 12:33 ` santosh
2017-06-01 8:05 ` [dpdk-dev] [PATCH 2/2] ether/ethdev: Allow pmd to advertise preferred pool capability Santosh Shukla
2017-06-30 14:13 ` Olivier Matz
2017-07-04 12:39 ` santosh
2017-07-04 13:07 ` Olivier Matz
2017-07-04 14:12 ` Jerin Jacob
2017-06-19 11:52 ` [dpdk-dev] [PATCH 0/2] Allow application set mempool handle Hemant Agrawal
2017-06-19 13:01 ` Jerin Jacob
2017-06-20 10:37 ` Hemant Agrawal
2017-06-20 14:04 ` Jerin Jacob
2017-06-30 14:12 ` Olivier Matz
2017-07-04 12:25 ` santosh
2017-07-04 15:59 ` Olivier Matz
2017-07-05 7:48 ` santosh
2017-07-20 7:06 ` [dpdk-dev] [PATCH v2 0/2] Dynamically configure " Santosh Shukla
2017-07-20 7:06 ` [dpdk-dev] [PATCH v2 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-08-15 8:07 ` [dpdk-dev] [PATCH v3 0/2] Dynamically configure mempool handle Santosh Shukla
2017-08-15 8:07 ` [dpdk-dev] [PATCH v3 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-09-04 11:46 ` Olivier MATZ
2017-09-07 9:25 ` Hemant Agrawal
2017-08-15 8:07 ` [dpdk-dev] [PATCH v3 2/2] ethdev: allow pmd to advertise " Santosh Shukla
2017-09-04 12:11 ` Olivier MATZ
2017-09-04 13:14 ` santosh
2017-09-07 9:21 ` Hemant Agrawal
2017-09-07 10:06 ` santosh
2017-09-07 10:11 ` santosh
2017-09-07 11:08 ` Hemant Agrawal
2017-09-11 9:33 ` Olivier MATZ
2017-09-11 12:40 ` santosh
2017-09-11 13:00 ` Olivier MATZ
2017-09-04 9:41 ` [dpdk-dev] [PATCH v3 0/2] Dynamically configure mempool handle Sergio Gonzalez Monroy
2017-09-04 13:20 ` santosh
2017-09-04 13:34 ` Olivier MATZ
2017-09-04 14:24 ` Sergio Gonzalez Monroy
2017-09-05 7:47 ` Olivier MATZ
2017-09-05 8:11 ` Jerin Jacob
2017-09-11 15:18 ` [dpdk-dev] [PATCH v4 " Santosh Shukla
2017-09-11 15:18 ` [dpdk-dev] [PATCH v4 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-09-25 7:28 ` Olivier MATZ
2017-09-25 21:23 ` santosh
2017-09-11 15:18 ` [dpdk-dev] [PATCH v4 2/2] ethdev: get the supported pools for a port Santosh Shukla
2017-09-25 7:37 ` Olivier MATZ
2017-09-25 21:52 ` santosh
2017-09-29 5:00 ` santosh
2017-09-29 8:32 ` Olivier MATZ
2017-09-29 10:16 ` santosh
2017-09-29 11:21 ` santosh [this message]
2017-09-29 11:23 ` Olivier MATZ
2017-09-29 11:31 ` santosh
2017-09-13 10:00 ` [dpdk-dev] [PATCH v4 0/2] Dynamically configure mempool handle santosh
2017-09-19 8:28 ` santosh
2017-09-25 7:24 ` Olivier MATZ
2017-09-25 21:58 ` santosh
2017-10-01 9:14 ` [dpdk-dev] [PATCH v5 " Santosh Shukla
2017-10-01 9:14 ` [dpdk-dev] [PATCH v5 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-10-02 14:29 ` Olivier MATZ
2017-10-06 0:29 ` Thomas Monjalon
2017-10-06 3:31 ` santosh
2017-10-06 8:39 ` Thomas Monjalon
2017-10-06 7:45 ` [dpdk-dev] [PATCH v6 0/2] Dynamically configure mempool handle Santosh Shukla
2017-10-06 7:45 ` [dpdk-dev] [PATCH v6 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-10-06 7:45 ` [dpdk-dev] [PATCH v6 2/2] ethdev: get the supported pool for a port Santosh Shukla
2017-10-06 18:58 ` [dpdk-dev] [PATCH v6 0/2] Dynamically configure mempool handle Thomas Monjalon
2017-10-01 9:14 ` [dpdk-dev] [PATCH v5 2/2] ethdev: get the supported pool for a port Santosh Shukla
2017-10-02 14:31 ` Olivier MATZ
2017-10-06 0:30 ` Thomas Monjalon
2017-10-06 3:32 ` santosh
2017-10-02 8:37 ` [dpdk-dev] [PATCH v5 0/2] Dynamically configure mempool handle santosh
2017-07-20 7:06 ` [dpdk-dev] [PATCH v2 2/2] ethdev: allow pmd to advertise pool handle Santosh Shukla
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=12be7257-4512-339f-63b1-c7926fff59e7@caviumnetworks.com \
--to=santosh.shukla@caviumnetworks.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=olivier.matz@6wind.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).