Re: [spp] [PATCH 1/5] spp_vf: remove while loop in classifier_mac

[Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>]

On 2019/02/05 20:47, x-fn-spp@sl.ntt-tx.co.jp wrote:
> From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> 
> So far, spp_classifier_mac_do() has while loop and it does not return
> until the classifier component is requested to stop and thus the worker
> thread can not execute packet processing for forwarder/merger.
> To introduce core-sharing, this patch removes the while loop.
I am not sure why this change is required for core sharing. Could explain in shortly? I think it is explained in added code.

> 
> Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
> ---
>   src/vf/spp_vf.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/src/vf/spp_vf.c b/src/vf/spp_vf.c
> index b98444d..bdc4c10 100644
> --- a/src/vf/spp_vf.c
> +++ b/src/vf/spp_vf.c
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright(c) 2017-2018 Nippon Telegraph and Telephone Corporation
> + * Copyright(c) 2017-2019 Nippon Telegraph and Telephone Corporation
>    */
>   
>   #include <netinet/in.h>
> @@ -209,21 +209,18 @@ slave_main(void *arg __attribute__ ((unused)))
>   			core = get_core_info(lcore_id);
>   		}
>   
> +		/* It is for processing multiple components. */
>   		for (cnt = 0; cnt < core->num; cnt++) {
> -			if (spp_get_component_type(lcore_id) ==

It should have a comment for the condition of core sharing.
> +			if (spp_get_component_type(core->id[cnt]) ==
>   					SPP_COMPONENT_CLASSIFIER_MAC) {
> -				/* Classifier loops inside the function. */
>   				ret = spp_classifier_mac_do(core->id[cnt]);
> -				break;
> +				if (unlikely(ret != 0))
> +					break;

It should have here too.
> +			} else {
> +				ret = spp_forward(core->id[cnt]);
> +				if (unlikely(ret != 0))
> +					break;
>   			}
> -
> -			/*
> -			 * Forward / Merge returns at once.
> -			 * It is for processing multiple components.
> -			 */
> -			ret = spp_forward(core->id[cnt]);
> -			if (unlikely(ret != 0))
> -				break;
>   		}
>   		if (unlikely(ret != 0)) {
>   			RTE_LOG(ERR, APP, "Core[%d] Component Error. "
>