Re: [dpdk-dev] [PATCH] net/i40e: fix hash enable issue in RSS flow

[Jeff Guo <jia.guo@intel.com>]

On 5/13/2020 5:54 PM, Wang, ShougangX wrote:
> Hi, Jeff
>
> Thanks for your review.
> This patch can be treated as a work around, the codes about p_info->action_flag will be reconstructed in the future.
>
> Thanks.
> Shougang
>
>> -----Original Message-----
>> From: Guo, Jia <jia.guo@intel.com>
>> Sent: Wednesday, May 13, 2020 4:41 PM
>> To: Xu, HailinX <hailinx.xu@intel.com>; Wang, ShougangX
>> <shougangx.wang@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
>> dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix hash enable issue in RSS flow
>>
>> hi, hailin
>>
>> Seems that this patch is fixing a issue about l2-playload but it is related with
>> the priors wrong usage of the pattern.
>>
>> The rework need further to do and sincerely it it not very easy to review
>> it.  But if consider it for the work around solution, commend as below.
>>
>>
>> On 5/8/2020 11:32 AM, Xu, HailinX wrote:
>>> Tested-by: Xu, Hailin <hailinx.xu@intel.com>
>>>
>>> Regards,
>>> Xu, Hailin
>>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wang, ShougangX
>>> Sent: Friday, May 8, 2020 9:52 AM
>>> To: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix hash enable issue in RSS
>>> flow
>>>
>>> Hi, Beilei
>>>
>>>> -----Original Message-----
>>>> From: Xing, Beilei <beilei.xing@intel.com>
>>>> Sent: Friday, May 8, 2020 9:04 AM
>>>> To: Wang, ShougangX <shougangx.wang@intel.com>; dev@dpdk.org
>>>> Subject: RE: [PATCH] net/i40e: fix hash enable issue in RSS flow
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Wang, ShougangX <shougangx.wang@intel.com>
>>>>> Sent: Thursday, May 7, 2020 5:44 PM
>>>>> To: dev@dpdk.org
>>>>> Cc: Xing, Beilei <beilei.xing@intel.com>; Wang, ShougangX
>>>>> <shougangx.wang@intel.com>
>>>>> Subject: [PATCH] net/i40e: fix hash enable issue in RSS flow
>>>>>
>>>>> This patch fixes the issue that failed to create an RSS rule with
>>>>> type
>>>>> L2- payload.
>>>>>
>>>>> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
>>>>> flow")
>>>>>
>>>>> Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
>>>>> ---
>>>>>    drivers/net/i40e/i40e_flow.c | 14 ++++++--------
>>>>>    1 file changed, 6 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/i40e/i40e_flow.c
>>>>> b/drivers/net/i40e/i40e_flow.c index 7e64ae53a..f5f2f0d5d 100644L2-
>>>>> --- a/drivers/net/i40e/i40e_flow.c
>>>>> +++ b/drivers/net/i40e/i40e_flow.c
>>>>> @@ -4511,6 +4511,7 @@ i40e_flow_parse_rss_pattern(__rte_unused
>>>> struct
>>>>> rte_eth_dev *dev,
>>>> <...>
>>>>
>>>>> @@ -4544,8 +4545,7 @@ i40e_flow_parse_rss_pattern(__rte_unused
>>>> struct
>>>>> rte_eth_dev *dev,  if
>>>>> (i40e_match_pattern(i40e_rss_pctype_patterns[i].item_array,
>>>>>    items)) {
>>>>>    p_info->types = i40e_rss_pctype_patterns[i].type;
>>>>> -rte_free(items); -return 0;
>>>>> +break;
>>>>>    }
>>>>>    }
>>>>>
>>>>> @@ -4580,11 +4580,9 @@ i40e_flow_parse_rss_pattern(__rte_unused
>>>>> struct rte_eth_dev *dev,
>>>>>    }
>>>>>    break;
>>>>>    default:
>>>>> -rte_flow_error_set(error, EINVAL,
>>>>> -RTE_FLOW_ERROR_TYPE_ITEM,
>>>>> -item,
>>>>> -"Not support range");
>>>>> -return -rte_errno;
>>>>> +p_info->action_flag = 0;
>>>>> +memset(info, 0, sizeof(struct i40e_queue_regions)); return 0;
>>
>> Could you check if all case have set
>>
>> p_info->action_flag such as RTE_FLOW_ITEM_TYPE_VLAN case.
>>
>>
>>>>>    }
>>>>>    }
>>>>>
>>>>> @@ -4640,7 +4638,7 @@ i40e_flow_parse_rss_action(struct
>> rte_eth_dev
>>>>> *dev,  return -rte_errno;  }
>>>>>
>>>>> -if (p_info.action_flag) {
>>>>> +if (p_info.action_flag && rss->queue_num) {
>>>>>    for (n = 0; n < 64; n++) {
>>>>>    if (rss->types & (hf_bit << n)) {
>>>>>    conf_info->region[0].hw_flowtype[0] = n;
>>>>> --
>>>>> 2.17.1
>>>> Are the above changes relating to L2-payload?
>>>>
>>> Yes, in order to resolve the conflict between hash enable and queue region
>> which caused by ether pattern, there are also a little bit changes for queue
>> region.
>>> Thanks.
>>> Shougang

Reviewed-by: Jeff Guo <jia.guo@intel.com <mailto:jia.guo@intel.com>>