[dpdk-dev] [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-ops API to return int

Shreyansh Jain shreyansh.jain at nxp.com
Fri May 18 12:59:56 CEST 2018


> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Andy Green
> Sent: Thursday, May 17, 2018 7:19 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v5 01/21] lib/librte_ethdev: change eth-dev-
> ops API to return int
> 
> Signed-off-by: Andy Green <andy at warmcat.com>
> ---
>  drivers/net/ark/ark_ethdev_rx.c     |    4 ++--
>  drivers/net/ark/ark_ethdev_rx.h     |    3 +--
>  drivers/net/avf/avf_rxtx.c          |    4 ++--
>  drivers/net/avf/avf_rxtx.h          |    2 +-
>  drivers/net/bnxt/bnxt_ethdev.c      |    5 +++--
>  drivers/net/dpaa/dpaa_ethdev.c      |    4 ++--
>  drivers/net/dpaa2/dpaa2_ethdev.c    |    6 +++---
>  drivers/net/e1000/e1000_ethdev.h    |    6 ++----
>  drivers/net/e1000/em_rxtx.c         |    4 ++--
>  drivers/net/e1000/igb_rxtx.c        |    4 ++--
>  drivers/net/enic/enic_ethdev.c      |    9 +++------
>  drivers/net/i40e/i40e_rxtx.c        |    4 ++--
>  drivers/net/i40e/i40e_rxtx.h        |    3 +--
>  drivers/net/ixgbe/ixgbe_ethdev.h    |    3 +--
>  drivers/net/ixgbe/ixgbe_rxtx.c      |    4 ++--
>  drivers/net/nfp/nfp_net.c           |    9 ++++-----
>  drivers/net/sfc/sfc_ethdev.c        |    4 ++--
>  drivers/net/thunderx/nicvf_ethdev.c |    2 +-
>  drivers/net/thunderx/nicvf_rxtx.c   |    4 ++--
>  drivers/net/thunderx/nicvf_rxtx.h   |    2 +-
>  drivers/net/vhost/rte_eth_vhost.c   |    4 ++--
>  examples/l3fwd-power/main.c         |    2 +-
>  lib/librte_ethdev/rte_ethdev_core.h |    4 ++--
>  23 files changed, 44 insertions(+), 52 deletions(-)
> 

[...]

>  	rxq = dev->data->rx_queues[rx_queue_id];
> diff --git a/drivers/net/dpaa/dpaa_ethdev.c
> b/drivers/net/dpaa/dpaa_ethdev.c
> index d014a11aa..70a5b4851 100644
> --- a/drivers/net/dpaa/dpaa_ethdev.c
> +++ b/drivers/net/dpaa/dpaa_ethdev.c
> @@ -725,7 +725,7 @@ static void dpaa_eth_tx_queue_release(void *txq
> __rte_unused)
>  	PMD_INIT_FUNC_TRACE();
>  }
> 
> -static uint32_t
> +static int
>  dpaa_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
>  {
>  	struct dpaa_if *dpaa_intf = dev->data->dev_private;
> @@ -738,7 +738,7 @@ dpaa_dev_rx_queue_count(struct rte_eth_dev *dev,
> uint16_t rx_queue_id)
>  		RTE_LOG(DEBUG, PMD, "RX frame count for q(%d) is %u\n",
>  			rx_queue_id, frm_cnt);
>  	}
> -	return frm_cnt;
> +	return (int)frm_cnt;
>  }
> 
>  static int dpaa_link_down(struct rte_eth_dev *dev)
> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c
> b/drivers/net/dpaa2/dpaa2_ethdev.c
> index 9297725d9..eb6245b83 100644
> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> @@ -604,7 +604,7 @@ dpaa2_dev_tx_queue_release(void *q __rte_unused)
>  	PMD_INIT_FUNC_TRACE();
>  }
> 
> -static uint32_t
> +static int
>  dpaa2_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t
> rx_queue_id)
>  {
>  	int32_t ret;
> @@ -612,7 +612,7 @@ dpaa2_dev_rx_queue_count(struct rte_eth_dev *dev,
> uint16_t rx_queue_id)
>  	struct dpaa2_queue *dpaa2_q;
>  	struct qbman_swp *swp;
>  	struct qbman_fq_query_np_rslt state;
> -	uint32_t frame_cnt = 0;
> +	int frame_cnt = 0;
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> @@ -628,7 +628,7 @@ dpaa2_dev_rx_queue_count(struct rte_eth_dev *dev,
> uint16_t rx_queue_id)
>  	dpaa2_q = (struct dpaa2_queue *)priv->rx_vq[rx_queue_id];
> 
>  	if (qbman_fq_query_state(swp, dpaa2_q->fqid, &state) == 0) {
> -		frame_cnt = qbman_fq_state_frame_count(&state);
> +		frame_cnt = (int)qbman_fq_state_frame_count(&state);
>  		DPAA2_PMD_DEBUG("RX frame count for q(%d) is %u",
>  				rx_queue_id, frame_cnt);
>  	}

This doesn't feel correct. A counter, especially the number of descriptors in a queue, doesn't have a negative value. So, 1) this is an unnatural return and 2) we litter the code with unnecessary typecast.

In fact, even in the above change, the debug messages continue to print unsigned values. So, another typecast would be required there.

I don't agree with this change - at least not until some strong gcc 8 warning reason is triggering this. Can you please point me to some conversation on mailing list which enforces this?


More information about the dev mailing list