[dpdk-dev] [PATCH v10] ethdev: new Rx/Tx offloads API
Dai, Wei
wei.dai at intel.com
Thu May 10 13:27:17 CEST 2018
Hi, Thomas
Thanks for your feedback and guidance.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas at monjalon.net]
> Sent: Thursday, May 10, 2018 10:36 AM
> To: Dai, Wei <wei.dai at intel.com>
> Cc: dev at dpdk.org; Yigit, Ferruh <ferruh.yigit at intel.com>; Zhang, Qi Z
> <qi.z.zhang at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v10] ethdev: new Rx/Tx offloads API
>
> Hi,
>
> I am checking if this patch comply with goals discussed in the survey:
> http://dpdk.org/ml/archives/dev/2018-March/094459.html
>
> - Allow "forgetting" port offloads in queue offloads setup.
>
> - An offload enabled at port level, cannot be disabled at queue level.
>
> - Every queue capabilities must be reported as port capabilities.
>
> - A capability should be reported at queue level
> only if it can be enabled on queue when it is disabled on port level.
>
> I think some items must be updated in doxygen comments of rte_ethdev.h.
> Please could you try to do a v11 for doxygen? I will review it quickly.
>
> Examples:
>
> - in queue offloads:
> "No need to repeat flags already enabled at port level.
> A flag enabled at port level, cannot be disabled at queue level."
>
> - in port capabilities: "(include per-queue capabilities)"
>
> More comments below, thanks.
>
>
> 10/05/2018 02:56, Wei Dai:
> > This patch check if a input requested offloading is valid or not.
> > Any reuqested offloading must be supported in the device capabilities.
> > Any offloading is disabled by default if it is not set in the
> > parameter dev_conf->[rt]xmode.offloads to rte_eth_dev_configure( ) and
> > [rt]x_conf->offloads to rte_eth_[rt]x_queue_setup( ).
> > If any offloading is enabled in rte_eth_dev_configure( ) by
> > application, it is enabled on all queues no matter whether it is
> > per-queue or per-port type and no matter whether it is set or cleared
> > in [rt]x_conf->offloads to rte_eth_[rt]x_queue_setup( ).
> > If a per-queue offloading hasn't be enabled in rte_eth_dev_configure(
> > ), it can be enabled or disabled for individual queue in
> > ret_eth_[rt]x_queue_setup( ).
> > A new added offloading is the one which hasn't been enabled in
> > rte_eth_dev_configure( ) and is reuqested to be enabled in
> > rte_eth_[rt]x_queue_setup( ), it must be per-queue type, otherwise
> > triger an error log.
> > The underlying PMD must be aware that the requested offloadings to PMD
> > specific queue_setup( ) function only carries those new added
> > offloadings of per-queue type.
>
> Good summary.
> Please forget the whitespace inside the parens.
I will remove whitespaces inside the parens.
>
> > This patch can make above such checking in a common way in rte_ethdev
> > layer to avoid same checking in underlying PMD.
>
> Good
>
> > --- a/doc/guides/prog_guide/poll_mode_drv.rst
> > +++ b/doc/guides/prog_guide/poll_mode_drv.rst
> > @@ -297,16 +297,30 @@ Per-Port and Per-Queue Offloads
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > In the DPDK offload API, offloads are divided into per-port and per-queue
> offloads.
> > +A per-queue offloading can be enabled on a queue and disabled on
> another queue at the same time.
> > +A pure per-port offloading can't be enabled on a queue and disabled on
> another queue at the same time.
> > +A pure per-port offloading must be enabled or disabled on all queues at
> the same time.
> > +A per-port offloading can be enabled or disabled on all queues at the
> same time.
>
> What is the difference between pure per-port and per-port here?
>
Can I add following words to explain more:
A pure per-port offloads are those supported by device but not per-queue type.
A supported offload can be per-queue or pure per-port, but can't be both
types at same device.
Any offloadings are per-port type as it can be enabled or disabled on all queues
at the same time.
> > +It is certain that both per-queue and pure per-port offloading are per-port
> type.
>
> I don't understand this sentence.
>
Explained above, I just want to divide all offloading into two distinct types:
Per-queue and pure per-port.
And per-queue + pure per-port = per-port.
> > The different offloads capabilities can be queried using
> ``rte_eth_dev_info_get()``.
> > +The dev_info->[rt]x_queue_offload_capa returned from
> ``rte_eth_dev_info_get()`` includes all per-queue offloading capabilities.
> > +The dev_info->[rt]x_offload_capa returned from
> ``rte_eth_dev_info_get()`` includes all per-port and per-queue offloading
> capabilities.
>
> Yes
>
> > +Any requested offloading by application must be within the device
> capabilities.
>
> Yes
>
> > +Any offloading is disabled by default if it is not set in the
> > +parameter
>
> Yes
>
> > +dev_conf->[rt]xmode.offloads to ``rte_eth_dev_configure( )`` and
> > +[rt]x_conf->offloads to ``rte_eth_[rt]x_queue_setup( )``.
> > +If any offloading is enabled in ``rte_eth_dev_configure( )`` by
> > +application, it is enabled on all queues no matter whether it is
> > +per-queue or per-port type and no matter whether it is set or cleared
> > +in [rt]x_conf->offloads to ``rte_eth_[rt]x_queue_setup( )``.
> > +If a per-queue offloading hasn't been enabled in
> > +``rte_eth_dev_configure( )``, it can be enabled or disabled in
> ``rte_eth_[rt]x_queue_setup( )`` for individual queue.
>
> Yes
>
> > +A new added offloads in [rt]x_conf->offloads to
> > +``rte_eth_[rt]x_queue_setup( )`` input by application is the one
> > +which hasn't been enabled in ``rte_eth_dev_configure( )`` and is requested
> to be enabled in ``rte_eth_[rt]x_queue_setup( )``, it must be per-queue type,
> otherwise return error.
>
> Yes
>
>
> > --- a/doc/guides/rel_notes/release_18_05.rst
> > +++ b/doc/guides/rel_notes/release_18_05.rst
> > +* **ethdev: changes to offload API**
>
> No need of bold formatting of title in API changes.
>
Will revise it as you guide.
> > +
> > + A pure per-port offloading isn't requested to be repeated in
> [rt]x_conf->offloads to
> > + ``rte_eth_[rt]x_queue_setup( )``. Now any offloading enabled in
> ``rte_eth_dev_configure( )``
> > + can't be disabled by ``rte_eth_[rt]x_queue_setup( )``. Any new added
> offloading which has
> > + not been enabled in ``rte_eth_dev_configure( )`` and is requested to
> be enabled in
> > + ``rte_eth_[rt]x_queue_setup( )`` must be per-queue type, otherwise
> return error.
>
>
>
More information about the dev
mailing list