Re: [PATCH 1/2] net/dpaa: Changes to support ethdev offload APIs

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

On 4/9/2018 2:19 PM, Sunil Kumar Kori wrote:
> Signed-off-by: Sunil Kumar Kori <sunil.kori@nxp.com>
> ---
>  drivers/net/dpaa/dpaa_ethdev.c | 46 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
> index db49364..efef62c 100644
> --- a/drivers/net/dpaa/dpaa_ethdev.c
> +++ b/drivers/net/dpaa/dpaa_ethdev.c
> @@ -95,6 +95,9 @@ static const struct rte_dpaa_xstats_name_off dpaa_xstats_strings[] = {
>  
>  static struct rte_dpaa_driver rte_dpaa_pmd;
>  
> +static void
> +dpaa_eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info);
> +
>  static inline void
>  dpaa_poll_queue_default_config(struct qm_mcc_initfq *opts)
>  {
> @@ -134,13 +137,42 @@ dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>  }
>  
>  static int
> -dpaa_eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
> +dpaa_eth_dev_configure(struct rte_eth_dev *dev)
>  {
>  	struct dpaa_if *dpaa_intf = dev->data->dev_private;
> +	struct rte_eth_conf *eth_conf = &dev->data->dev_conf;
> +	struct rte_eth_dev_info dev_info;
> +	uint64_t rx_offloads = eth_conf->rxmode.offloads;
> +	uint64_t tx_offloads = eth_conf->txmode.offloads;
>  
>  	PMD_INIT_FUNC_TRACE();
>  
> -	if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
> +	dpaa_eth_dev_info(dev, &dev_info);

It is up to you but you may prefer to keep [rt]x_offload_capa in a variable or
macro so that you can use here directly without need to call dev_info, but that
is also OK if you prefer.

> +	if (dev_info.rx_offload_capa != rx_offloads) {
> +		DPAA_PMD_ERR("Some Rx offloads are not supported "
> +			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
> +			rx_offloads, dev_info.rx_offload_capa);
> +		return -ENOTSUP;
> +	}
> +
> +	if (dev_info.tx_offload_capa != tx_offloads) {
> +		DPAA_PMD_ERR("Some Tx offloads are not supported "
> +			"requested 0x%" PRIx64 " supported 0x%" PRIx64,
> +			tx_offloads, dev_info.tx_offload_capa);
> +		return -ENOTSUP;
> +	}


dev_info.rx_offload_capa is your device's offload capability. User may prefer to
utilize or not any of these offloads. So you can't return if requested offloads
are not equal to capability, this part is wrong.

Only you need to be sure that user is not asking more than what is supported.

<...>