Re: [dpdk-dev] [PATCH v5 1/3] ethdev: introduce Rx queue offloads API

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 9/28/2017 7:54 PM, Shahaf Shuler wrote:
> Introduce a new API to configure Rx offloads.
> 
> In the new API, offloads are divided into per-port and per-queue
> offloads. The PMD reports capability for each of them.
> Offloads are enabled using the existing DEV_RX_OFFLOAD_* flags.
> To enable per-port offload, the offload should be set on both device
> configuration and queue configuration. To enable per-queue offload, the
> offloads can be set only on queue configuration.
> 
> Applications should set the ignore_offload_bitfield bit on rxmode
> structure in order to move to the new API.
> 
> The old Rx offloads API is kept for the meanwhile, in order to enable a
> smooth transition for PMDs and application to the new API.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>

<...>

> @@ -1102,8 +1193,18 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>  	if (rx_conf == NULL)
>  		rx_conf = &dev_info.default_rxconf;
>  
> +	local_conf = *rx_conf;
> +	if (dev->data->dev_conf.rxmode.ignore_offload_bitfield == 0) {
> +		/**
> +		 * Reflect port offloads to queue offloads in order for
> +		 * offloads to not be discarded.
> +		 */
> +		rte_eth_convert_rx_offload_bitfield(&dev->data->dev_conf.rxmode,
> +						    &local_conf.offloads);
> +	}

If an application switches to the new method, it will set "offloads" and
if underlying PMD doesn't support the new method it will just do nothing
with "offloads" variable but problem is application won't know PMD just
ignored them, it may think per queue offloads set.

Does it make sense to notify application that PMD doesn't understand
that new "offloads" flag?

> +
>  	ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
> -					      socket_id, rx_conf, mp);
> +					      socket_id, &local_conf, mp);
>  	if (!ret) {
>  		if (!dev->data->min_rx_buf_size ||
>  		    dev->data->min_rx_buf_size > mbp_buf_size)

<...>

>  /**
> @@ -691,6 +712,12 @@ struct rte_eth_rxconf {
>  	uint16_t rx_free_thresh; /**< Drives the freeing of RX descriptors. */
>  	uint8_t rx_drop_en; /**< Drop packets if no descriptors are available. */
>  	uint8_t rx_deferred_start; /**< Do not start queue with rte_eth_dev_start(). */
> +	/**
> +	 * Per-queue Rx offloads to be set using DEV_RX_OFFLOAD_* flags.
> +	 * Only offloads set on rx_queue_offload_capa or rx_offload_capa
> +	 * fields on rte_eth_dev_info structure are allowed to be set.
> +	 */

How application will use above "capa" flags to decide what to set? Since
"rx_queue_offload_capa" is new field introduced with this patch no PMD
implemented it yet, does it means no application will be able to use per
queue offloads yet?

> +	uint64_t offloads;
>  };
>  

<...>