From: Shreyansh Jain <shreyansh.jain@nxp.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>,
"Horton, Remy" <remy.horton@intel.com>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>,
"Zhang, Qi Z" <qi.z.zhang@intel.com>,
"Xing, Beilei" <beilei.xing@intel.com>,
Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC PATCH v1 1/4] ethdev: add support for PMD-tuned Tx/Rx parameters
Date: Wed, 21 Mar 2018 12:21:31 +0530 [thread overview]
Message-ID: <CAJ5mUsVZe7gjvHVRmqpzOni8r_+Dsun23M3j7trMj0GfF+Nrsg@mail.gmail.com> (raw)
In-Reply-To: <8af6d6ea-34d8-3b0d-cd34-c11ebb2a8207@intel.com>
On Tue, Mar 20, 2018 at 8:24 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 3/16/2018 1:54 PM, Shreyansh Jain wrote:
>> On Thu, Mar 15, 2018 at 8:27 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
[...]
>>> Hi Remy, Shreyansh,
>>>
>>> What do you think about using a variable name consistent with existing
>>> "default_[rt]xconf" in dev_info?
>>
>> It just turned out to be much more complex than I initially thought :)
>> Is this what the above conversation merging at (for Rx, as example):
>>
>> 1. 'default_rx_size_conf' is added in rte_eth_dev_info (and this
>> includes I/O params like burst size, besides configure time nb_queue,
>> nb_desc etc). Driver would return these values filled in when
>> info_get() is called.
>>
>> 2a. If an application needs the defaults, it would perform info_get()
>> and get the values. then, use the values in configuration APIs
>> (rx_queue_setup for nb_rx_desc, eth_dev_dev_configure for
>> nb_rx_queues).
>> For rx_burst calls, it would use the burst_size fields obtained from info_get().
>> This is good enough for configuration and datapath (rx_burst).
>>
>> OR, another case
>>
>> 2b. Application wants to use default vaules provided by driver without
>> calling info_get. In which case, it would call
>> rx_queue_setup(nb_rx_desc=0..) or eth_dev_configure(nb_rx_queue=0,
>> nb_tx_queue=0). The implementation would query the value from
>> 'default_rx_size_conf' through info_get() and use those values.
>> Though, in this case, rte_eth_rx_burst(burst=0) might not work for
>> picking up the default within rte_ethdev.h.
>
> In Bruce's suggestion where ethdev keep defaults is changed.
> Initial suggestion was rte_eth_dev_info_get() filling default data, now defaults
> will be defined in functions like rte_eth_rx_queue_setup().
>
> This is a little different from filling defaults in rte_eth_dev_info_get():
> - Application can know where the defaults are coming from because dev_info
> fields are only modified by PMD. Application still prefer to use ethdev defaults.
>
> - The default values in ethdev library provided in function related to that
> data, instead of separate rte_eth_dev_info_get() function.
It seems we both are on same page (almost) - just that I couldn't
articulate my comments properly earlier, maybe.
rte_eth_dev_info_get is only a method to get defaults set by PMDs.
dev_info_get is not setting defaults by itself. I get this.
>
>
> What application can do:
> - Application can call rte_eth_dev_info_get() and can learn if PMD provided
> defaults or not.
> - If PMD doesn't provided any default values application can prefer to use
> application defined values. This may be an option for the application looking
> for most optimized values.
> - Although PMD doesn't provide any defaults, application still can use defaults
> provided by ethdev by providing '0' as arguments.
Yes, agree - and only comment I added previously in this case is that
this is not applicable for burst APIs. So, optimal [rt]x burst size
cannot be 'defaulted' to EAL layer. Other values like ring size, queue
count can be delegated to EAL for overwriting if passed as '0'.
>
>
> So how related ethdev functions will be updated:
> if argument != 0
> use argument
> else
> dev_info_get()
> if dev_info->argument != 0
> use dev_info->argument
> else
> use function_prov
Perfect, but only for eth_dev_configure and eth_[rt]x_queue_setup functions -
and that is OK with me.
>
>>
>> :Four observations:
>> A). For burst size (or any other I/O time value added in future),
>> values would have to be explicitly used by application - always. If
>> value reported by info_get() is '0' (see (B) below), application to
>> use its own judgement. No default override by lib_eal.
>> IMO, This is good enough assumption.
>
> This is no more true after Bruce's comment.
> If application provides any values it will overwrite everything else,
> application has the final word.
> But application may prefer to use provided default values.
I am not sure what has changed with Bruce's comment - but I agree with
what you are stating.
>
>>
>> B). '0' as an indicator for 'no-default-value-available-from-driver'
>> is still an open point. It is good enough for current proposed
>> parameters, but may be a valid numerical value in future.
>> IMO, this can be ignored for now.
>
> Agree that we can ignore it for now.
>
>>
>> C) Unlike the original proposal, this would add two separate members
>> to rte_eth_dev_info - one each for Rx and Tx. They both are still
>> expected to be populated through the info_get() implementation but not
>> by lib_eal.
>> IMO, doesn't matter.
>
> There won't be new members, which ones are you talking about?
original proposal: (ignore change of names, please)
rte_eth_dev_preferred_info {
rx_burst_size
tx_burst_size
rx_ring_size
tx_ring_size
...
}
And this is what I think last few comments intended:
rte_eth_rxpreferred {
...
rx_burst_size
rx_ring_size
...
}
rte_eth_txpreferred {
...
tx_burst_size
tx_ring_size
...
}
both the above added rte_eth_dev_info{}
This is what I meant when I stated "...this would add two separate
members to rte_eth_dev_info - one each for Rx and Tx..."
[...]
-
Shreyansh
next prev parent reply other threads:[~2018-03-21 6:52 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-07 12:08 [dpdk-dev] [RFC PATCH v1 0/4] ethdev: add per-PMD tuning of RxTx parmeters Remy Horton
2018-03-07 12:08 ` [dpdk-dev] [RFC PATCH v1 1/4] ethdev: add support for PMD-tuned Tx/Rx parameters Remy Horton
2018-03-14 12:28 ` Shreyansh Jain
2018-03-14 14:09 ` Remy Horton
2018-03-14 14:43 ` Ferruh Yigit
2018-03-14 15:10 ` Shreyansh Jain
2018-03-15 9:02 ` Remy Horton
2018-03-14 15:48 ` Remy Horton
2018-03-14 16:42 ` Ferruh Yigit
2018-03-14 17:23 ` Shreyansh Jain
2018-03-14 17:52 ` Ferruh Yigit
2018-03-14 18:53 ` Ananyev, Konstantin
2018-03-14 21:02 ` Ferruh Yigit
2018-03-14 21:36 ` Bruce Richardson
2018-03-15 13:57 ` Ferruh Yigit
2018-03-15 14:39 ` Bruce Richardson
2018-03-15 14:57 ` Ferruh Yigit
2018-03-16 13:54 ` Shreyansh Jain
2018-03-16 14:18 ` Bruce Richardson
2018-03-16 15:36 ` Remy Horton
2018-03-20 15:03 ` Ferruh Yigit
2018-03-21 10:14 ` Remy Horton
2018-03-21 13:56 ` Ferruh Yigit
2018-03-20 14:54 ` Ferruh Yigit
2018-03-21 6:51 ` Shreyansh Jain [this message]
2018-03-21 10:02 ` Ferruh Yigit
2018-03-21 10:45 ` Shreyansh Jain
2018-03-15 12:51 ` Ananyev, Konstantin
2018-03-15 13:57 ` Ferruh Yigit
2018-03-15 14:42 ` Bruce Richardson
2018-03-07 12:08 ` [dpdk-dev] [RFC PATCH v1 2/4] net/e1000: add TxRx tuning parameters Remy Horton
2018-03-07 12:08 ` [dpdk-dev] [RFC PATCH v1 3/4] net/i40e: " Remy Horton
2018-03-07 12:08 ` [dpdk-dev] [RFC PATCH v1 4/4] testpmd: make use of per-PMD TxRx parameters Remy Horton
2018-03-21 14:27 ` [dpdk-dev] [PATCH v2 0/4] ethdev: add per-PMD tuning of RxTx parmeters Remy Horton
2018-03-21 14:27 ` [dpdk-dev] [PATCH v2 1/4] ethdev: add support for PMD-tuned Tx/Rx parameters Remy Horton
2018-03-28 7:11 ` Shreyansh Jain
2018-03-30 15:40 ` Thomas Monjalon
2018-03-30 15:57 ` Thomas Monjalon
2018-03-31 0:46 ` Thomas Monjalon
2018-03-21 14:27 ` [dpdk-dev] [PATCH v2 2/4] net/e1000: add TxRx tuning parameters Remy Horton
2018-03-21 14:27 ` [dpdk-dev] [PATCH v2 3/4] net/i40e: " Remy Horton
2018-03-21 14:27 ` [dpdk-dev] [PATCH v2 4/4] testpmd: make use of per-PMD TxRx parameters Remy Horton
2018-03-28 7:18 ` Shreyansh Jain
2018-04-03 11:00 ` Remy Horton
2018-03-31 0:01 ` Thomas Monjalon
2018-04-03 8:49 ` Remy Horton
2018-03-27 18:43 ` [dpdk-dev] [PATCH v2 0/4] ethdev: add per-PMD tuning of RxTx parmeters Ferruh Yigit
2018-03-30 10:34 ` Ferruh Yigit
2018-03-31 0:05 ` Thomas Monjalon
2018-04-04 17:17 ` [dpdk-dev] [PATCH v3 " Remy Horton
2018-04-04 17:17 ` [dpdk-dev] [PATCH v3 1/4] ethdev: add support for PMD-tuned Tx/Rx parameters Remy Horton
2018-04-04 18:56 ` De Lara Guarch, Pablo
2018-04-05 10:16 ` Thomas Monjalon
2018-04-04 17:17 ` [dpdk-dev] [PATCH v3 2/4] net/e1000: add TxRx tuning parameters Remy Horton
2018-04-04 17:17 ` [dpdk-dev] [PATCH v3 3/4] net/i40e: " Remy Horton
2018-04-04 17:17 ` [dpdk-dev] [PATCH v3 4/4] testpmd: make use of per-PMD TxRx parameters Remy Horton
2018-04-06 14:49 ` [dpdk-dev] [PATCH v5 0/4] ethdev: add per-PMD tuning of RxTx parmeters Remy Horton
2018-04-06 14:49 ` [dpdk-dev] [PATCH v5 1/4] ethdev: add support for PMD-tuned Tx/Rx parameters Remy Horton
2018-04-06 14:50 ` [dpdk-dev] [PATCH v5 2/4] net/e1000: add TxRx tuning parameters Remy Horton
2018-04-06 14:50 ` [dpdk-dev] [PATCH v5 3/4] net/i40e: " Remy Horton
2018-04-06 14:50 ` [dpdk-dev] [PATCH v5 4/4] testpmd: make use of per-PMD TxRx parameters Remy Horton
2018-04-09 12:55 ` Shreyansh Jain
2018-04-09 14:38 ` Remy Horton
2018-04-10 4:18 ` Shreyansh Jain
2018-04-10 6:09 ` Remy Horton
2018-04-10 6:39 ` Shreyansh Jain
2018-04-06 17:01 ` [dpdk-dev] [PATCH v5 0/4] ethdev: add per-PMD tuning of RxTx parmeters Ferruh Yigit
2018-04-10 9:43 ` [dpdk-dev] [PATCH v6 " Remy Horton
2018-04-10 9:43 ` [dpdk-dev] [PATCH v6 1/4] ethdev: add support for PMD-tuned Tx/Rx parameters Remy Horton
2018-04-10 9:43 ` [dpdk-dev] [PATCH v6 2/4] net/e1000: add TxRx tuning parameters Remy Horton
2018-04-10 9:43 ` [dpdk-dev] [PATCH v6 3/4] net/i40e: " Remy Horton
2018-04-10 9:43 ` [dpdk-dev] [PATCH v6 4/4] testpmd: make use of per-PMD TxRx parameters Remy Horton
2018-04-10 12:57 ` [dpdk-dev] [PATCH v6 0/4] ethdev: add per-PMD tuning of RxTx parmeters Thomas Monjalon
2018-04-10 18:56 ` Ferruh Yigit
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=CAJ5mUsVZe7gjvHVRmqpzOni8r_+Dsun23M3j7trMj0GfF+Nrsg@mail.gmail.com \
--to=shreyansh.jain@nxp.com \
--cc=beilei.xing@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=jingjing.wu@intel.com \
--cc=konstantin.ananyev@intel.com \
--cc=qi.z.zhang@intel.com \
--cc=remy.horton@intel.com \
--cc=thomas@monjalon.net \
--cc=wenzhuo.lu@intel.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).