14/05/2018 14:00, Wei Dai:
> --- a/doc/guides/prog_guide/poll_mode_drv.rst
> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
> @@ -303,12 +303,12 @@ In the DPDK offload API, offloads are divided into
> per-port and per-queue offloa
> * 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.
> * Any offloading is per-queue or pure per-port type, but can't be both types
> at same devices.
> -* A per-port offloading 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.
> +* Port capabilities = pre-queue capabilities + pure per-port capabilities.
s/pre/per/
> +* Any supported offloading can be enabled on all queues.
>
> 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.
> +The ``dev_info->[rt]x_offload_capa`` returned from
> ``rte_eth_dev_info_get()`` includes all pure per-port and per-queue
> offloading capabilities.
OK
> @@ -1556,29 +1558,29 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t
> rx_queue_id,
> * The local_conf.offloads input to underlying PMD only carries
> * those offloadings which are only enabled on this queue and
> * not enabled on all queues.
> - * The underlying PMD must be aware of this point.
> */
OK
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1067,13 +1067,18 @@ struct rte_eth_dev_info {
> uint16_t max_vfs; /**< Maximum number of VFs. */
> uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
> uint64_t rx_offload_capa;
> - /**< All RX offload capabilities including all per queue ones */
> + /**<
> + * All RX offload capabilities including all per-queue ones.
> + * Any flag in [rt]x_offload_capa and [rt]x_queue_offload_capa
> + * of this structure needn't be repeated in rte_eth_[rt]x_queue_setup().
It is confusing. Better to remove this sentence about queue_setup
in port capa comment.
> + * A flag enabled at port level can't be disabled at queue level.
This one too: it is a comment about port capa, not queue setup.
> @@ -1554,9 +1559,7 @@ const char * __rte_experimental
> rte_eth_dev_tx_offload_name(uint64_t offload);
> * the [rt]x_offload_capa returned from rte_eth_dev_infos_get().
> * Any type of device supported offloading set in the input argument
> * eth_conf->[rt]xmode.offloads to rte_eth_dev_configure() is enabled
> - * on all [RT]x queues and it can't be disabled no matter whether
> - * it is cleared or set in the input argument [rt]x_conf->offloads
> - * to rte_eth_[rt]x_queue_setup().
> + * on all queues and it can't be disabled in
> rte_eth_[rt]x_queue_setup().
OK
Missing: we must explain the "no repeat need" and
"no disable port offload on queue" constraint.
In the last review, I was suggesting such sentences:
No need to repeat flags already enabled at port level.
A flag enabled at port level, cannot be disabled at queue level.
I think it should go in queue setup comments.
Opinion?
