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:
>> >+ice_unregister_parser(struct ice_flow_parser *parser,
>> >+		struct ice_adapter *ad)
>> >+{
>> >+	struct ice_pf *pf = &ad->pf;
>> >+	struct ice_parser_list *list;
>> >+	struct ice_flow_parser *p_parser;
>> >+	void *temp;
>> >+
>> >+	switch (parser->stage) {
>> >+	case ICE_FLOW_STAGE_RSS:
>> >+		list = &pf->rss_parser_list;
>> >+		break;
>> >+	case ICE_FLOW_STAGE_PERMISSION:
>> >+		list = &pf->perm_parser_list;
>> >+		break;
>> >+	case ICE_FLOW_STAGE_DISTRIBUTOR:
>> >+		list = &pf->dist_parser_list;
>> >+		break;
>> >+	default:
>> >+		return;
>> >+	}
>> 
>> The switch blocks in above functions are the same, it's better to use a common
>> function to reduce the duplicated code.
>
>The switch blocks in the above two functions have little difference in the default behavior, one is return -EINVAL, the other is just return, for register/unregister funcs have different return value types. So, Can I just keep this format?
>

Duplication is bad and I think it should be easy to deal with the return type
difference, 

struct ice_prase_list *
ice_get_parser_list(struct ice_flow_parser *parser,
                struct ice_adapter *ad)
{
        struct ice_parser_list *list = NULL;
        struct ice_pf *pf = &ad->pf;

        switch (parser->stage) {
        case ICE_FLOW_STAGE_RSS:
                list = &pf->rss_parser_list;
                break;
        case ICE_FLOW_STAGE_PERMISSION:
                list = &pf->perm_parser_list;
                break;
        case ICE_FLOW_STAGE_DISTRIBUTOR:
                list = &pf->dist_parser_list;
                break;
        default:
		break;
        }

	return list;
}

Then you just need to check its return value, if it's NULL, simply return
-EINVAL on register and directly return on unregister.

Thanks,
Xiaolong