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

["Wang, ShougangX" <shougangx.wang@intel.com>]

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