Re: [dpdk-dev] [PATCH 2/4] net/ice: rework for generic flow enabling

[Ye Xiaolong <xiaolong.ye@intel.com>]

On 09/09, Wang, Ying A wrote:

[snip]

>> >+	if (ad->devargs.pipeline_mode_support) {
>> >+		if (0 == attr->priority)
>> >+			ice_pipeline_stage =
>> >+				ICE_FLOW_CLASSIFY_STAGE_PERMISSION;
>> >+		else
>> >+			ice_pipeline_stage =
>> >+				ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR;
>> >+	} else {
>> >+		ice_pipeline_stage =
>> >+			ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR_ONLY;
>> 
>> Do we really this assignment?
>
>Yes. We use devargs.pipeline_mode_support as a hint to decide which mode to use, 1 for pipeline mode, 0 for non-pipeline mode.
>By default, non-pipeline mode is used and both switch/fdir used as distributor, switch is fdir's backup. 
>In pipeline mode, attr->priority is enabled,  0 for permission stage and 1 for distributor stage.
>

I saw ice_pipeline_stage has been set to ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR_ONLY 
in its initialization, do we need to reassign it every time here. The pipeline
mode won't change at runtime, right?

>> 
>> >+		/* Not supported */
>> >+		if (attr->priority) {

[snip]

>> > free_flow:
>> >-	rte_flow_error_set(error, -ret,
>> >-			   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>> >-			   "Failed to create flow.");
>> >+	PMD_DRV_LOG(ERR, "Failed to create flow");
>> 
>> Why is this change?
>
>For framework has passed the "error" to each filter, rte_flow_error_set() will be used within each filter (switch/fdir/rss).
>If used rte_flow_error_set() here, it will cover the error set value by each filter, so PMD_DRV_LOG is used here.
>

I think it makes sense, thanks for the explanation.

Thanks,
Xiaolong