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

[Ye Xiaolong]

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