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.
> 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?
> +It is certain that both per-queue and pure per-port offloading are per-port
> type.
I don't understand this sentence.
> 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.
> +
> + 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.
