Re: [dpdk-dev] [PATCH] ixgbe: fix compilation issue when RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC is disabled

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 19 Mar 2015 12:24:38 +0000
Pablo de Lara <pablo.de.lara.guarch@intel.com> wrote:

> If RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC is disabled in the config file,
> RTE_PMD_IXGBE_RX_MAX_BURST macro and ixgbe_recv_pkts_bulk_alloc function are not declared,
> and some parts of the ixgbe code were still trying to use them.
> 
> This patch fixes the problem by including previous conditionals
> that were removed in 01fa1d6 ("ixgbe: unify Rx setup").
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 8bede22..011e97c 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -2075,7 +2075,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct ixgbe_rx_queue *rxq)
>  
>  /* Reset dynamic ixgbe_rx_queue fields back to defaults */
>  static void
> -ixgbe_reset_rx_queue(struct ixgbe_hw *hw, struct ixgbe_rx_queue *rxq)
> +ixgbe_reset_rx_queue(struct ixgbe_hw *hw __rte_unused, struct ixgbe_rx_queue *rxq)
>  {
>  	static const union ixgbe_adv_rx_desc zeroed_desc = { .read = {
>  			.pkt_addr = 0}};
> @@ -2092,9 +2092,11 @@ ixgbe_reset_rx_queue(struct ixgbe_hw *hw, struct ixgbe_rx_queue *rxq)
>  	 * constraints here to see if we need to zero out memory after the end
>  	 * of the H/W descriptor ring.
>  	 */
> +#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
>  	if (hw->rx_bulk_alloc_allowed)
>  		/* zero out extra memory */
>  		len += RTE_PMD_IXGBE_RX_MAX_BURST;
> +#endif
>  
>  	/*
>  	 * Zero out HW ring memory. Zero out extra memory at the end of
> @@ -2238,8 +2240,10 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  	 * function does not access an invalid memory region.
>  	 */
>  	len = nb_desc;
> +#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
>  	if (hw->rx_bulk_alloc_allowed)
>  		len += RTE_PMD_IXGBE_RX_MAX_BURST;
> +#endif
>  
>  	rxq->sw_ring = rte_zmalloc_socket("rxq->sw_ring",
>  					  sizeof(struct ixgbe_rx_entry) * len,
> @@ -3544,14 +3548,19 @@ void ixgbe_set_rx_function(struct rte_eth_dev *dev)
>  				   "burst size no less than 32.");
>  
>  		dev->rx_pkt_burst = ixgbe_recv_pkts_vec;
> -	} else if (hw->rx_bulk_alloc_allowed) {
> +
> +	}
> +#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
> +	else if (hw->rx_bulk_alloc_allowed) {
>  		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions are "
>  				    "satisfied. Rx Burst Bulk Alloc function "
>  				    "will be used on port=%d.",
>  			     dev->data->port_id);
>  
>  		dev->rx_pkt_burst = ixgbe_recv_pkts_bulk_alloc;
> -	} else {
> +	}
> +#endif
> +	else {
>  		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions are not "
>  				    "satisfied, or Scattered Rx is requested, "
>  				    "or RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC "

This code needs some 'de-ifdef' work. It is hard to maintain as is.
I prefer having one inline function, and getting rid of all the #ifdef's
and just let the compiler eliminate dead code.