Re: [dpdk-dev] [PATCH 1/1] eal: Don't fail secondary if primary is missing tailqs

[Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>]

Hi Jean,

As with your other patch, commit title needs fixing and also missing 
Signed-off-by line.

On 22/09/2016 22:17, Jean Tourrilhes wrote:
>   lib/librte_eal/common/eal_common_tailqs.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_tailqs.c b/lib/librte_eal/common/eal_common_tailqs.c
> index bb08ec8..6960d06 100644
> --- a/lib/librte_eal/common/eal_common_tailqs.c
> +++ b/lib/librte_eal/common/eal_common_tailqs.c
> @@ -143,6 +143,8 @@ rte_eal_tailq_update(struct rte_tailq_elem *t)
>   		t->head = rte_eal_tailq_create(t->name);
>   	} else {
>   		t->head = rte_eal_tailq_lookup(t->name);
> +		if (t->head != NULL)
> +			rte_tailqs_count++;
>   	}
>   }
>   
> @@ -188,9 +190,16 @@ rte_eal_tailqs_init(void)
>   		if (t->head == NULL) {
>   			RTE_LOG(ERR, EAL,
>   				"Cannot initialize tailq: %s\n", t->name);
> -			/* no need to TAILQ_REMOVE, we are going to panic in
> -			 * rte_eal_init() */
> -			goto fail;
> +			if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +				/* no need to TAILQ_REMOVE, we are going
> +				 * to panic in rte_eal_init() */
> +				goto fail;
> +			} else {
> +				/* This means our list of constructor is
> +				 * no the same as primary. Just remove
> +				 * that missing tailq and continue */
> +				TAILQ_REMOVE(&rte_tailq_elem_head, t, next);
> +			}
>   		}
>   	}
>   
I might be missing something here so bear with me.
The case you are trying to fix is, as an example, when your secondary 
app is using LPM but your primary is not.
So basically with this patch, you are removing the tailq for LPM on 
secondary and continuing as normal, is that the case?

I am not convinced about this approach.
I guess the assumption here is that all the TAILQ infrastructure works 
even when the tailq list itself is NULL?
I say assumption because I don't have an easy way to trigger this use 
case with default DPDK sample/test apps.

What about letting the secondary process create a tailq if it doesn't 
exists?
We would need to lock protect at least the register function to avoid 
race conditions when having multiple secondary processes.

David, what do you think?

Sergio