Re: [RFC PATCH 2/4] ethdev: introduce Rx queue offloads API

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

On 8/7/2017 11:54 AM, Shahaf Shuler wrote:
> Introduce a new API to configure Rx offloads.
> 
> The new API will re-use existing DEV_RX_OFFLOAD_* flags
> to enable the different offloads. This will ease the process
> of adding a new Rx offloads, as no ABI breakage is involved.
> In addition, the offload configuration can be done per queue,
> instead of per port.

If a device doesn't have capability to set the offload per queue how
should it behave, I think it is good to define this.

> 
> The Rx queue offload API can be used only with devices which advertize
> the RTE_ETH_DEV_RXQ_OFFLOAD capability.
> 
> 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>

<...>

> @@ -357,7 +357,14 @@ struct rte_eth_rxmode {
>  		jumbo_frame      : 1, /**< Jumbo Frame Receipt enable. */
>  		hw_strip_crc     : 1, /**< Enable CRC stripping by hardware. */
>  		enable_scatter   : 1, /**< Enable scatter packets rx handler */
> -		enable_lro       : 1; /**< Enable LRO */
> +		enable_lro       : 1, /**< Enable LRO */
> +		ignore		 : 1;

what do you think making this variable more verbose, like
"ignore_rx_offloads"

"dev_conf.rxmode.ignore" doesn't say on its own what is ignored.

> +		/**
> +		 * When set the rxmode offloads should be ignored,
> +		 * instead the Rx offloads will be set on rte_eth_rxq_conf.
> +		 * This bit is temporary till rxmode Rx offloads API will
> +		 * be deprecated.
> +		 */
>  };

<...>

> +/** Device supports the rte_eth_rxq_conf offloads API */
> +#define RTE_ETH_DEV_RXQ_OFFLOAD 0x0010
Since this is temporary flag and with current implementation this is
local to library, should we put this into public header?

Later when all PMDs implemented this new method and we want to remove
the flag, can we remove them or do we have to keep them reserved for any
conflict for further new values?

I guess this should be part of missing pmd-ethdev interface file
(rte_ethdev_pmd.h ?).

>  
>  /**
>   * @internal
>