Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

[Thomas Monjalon <thomas@monjalon.net>]

21/03/2022 08:14, lihuisong (C):
> 2022/3/10 16:08, lihuisong (C):
> > 2022/3/9 17:55, Ori Kam:
> >> From: lihuisong (C) <lihuisong@huawei.com>
> >>> 2022/3/3 10:47, lihuisong (C):
> >>>> 2022/3/2 22:07, Ori Kam:
> >>>>> From: lihuisong (C) <lihuisong@huawei.com>
> >>>>>> 2022/3/1 0:42, Ferruh Yigit:
> >>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> >>>>>>>> From: Huisong Li <lihuisong@huawei.com>
> >>>>>>>>
> >>>>>>>> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be 
> >>>>>>>> set in
> >>>>>>>> dev_configure phase. However, if this flag isn't set, RSS can be
> >>>>>>>> enabled
> >>>>>>>> through the ethdev ops and rte_flow API. This behavior is 
> >>>>>>>> contrary to
> >>>>>>>> each
> >>>>>>>> other.
> >>>>>>>>
> >>>>>>>> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> >>>>>>>> Cc: stable@dpdk.org
> >>>>>>>>
> >>>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >>>>>>> Hi Huisong, Connor,
> >>>>>>>
> >>>>>>> Let's get a little more feedback for this patch, cc'ed more people.
> >>>>>>>
> >>>>>>> To enable RSS, multi queue mode should be set to
> >>>>>>> 'RTE_ETH_MQ_RX_RSS_FLAG'.
> >>>>>>>
> >>>>>>> But I wonder if it is required to configure RSS via flow API,
> >>>>>> I do not know the original purpose of adding the RSS 
> >>>>>> configuration in
> >>>>>> flow API.
> >>>>>>
> >>>>> The purpose is simple, this allow to create RSS per rule and not a
> >>>>> global one.
> >>>>> For example create RSS that sends TCP to some queues while othe RSS
> >>>>> will send
> >>>>> UDP traffic to different queues.
> >>>> I'm a little confused now. The "per rule" also seems to be a global
> >>>> configuration.
> >>>> Example:
> >>>>   - start PMD with 0,1,2,3
> >>>>   - create TCP packets to 2,3 queues. At this moment, only 2,3 queues
> >>>> can be received for other types of packets.
> >>>> Because this rule is implemented by modifying the entry of the
> >>>> redirection table which is global for this device.
> >>> Hi, Ori and Stephen.
> >>> Can you help me clear up the confusion above? If some NICs behave like
> >>> this, what should we do about it?
> >> I'm not sure I understand the issue, maybe it is releated to some 
> >> HW/PMD limitation.
> >> In your example non TCP traffic will be routed to one of the 4 queues 
> >> (0,1,2,3),
> >> While TCP traffic will only be routed to queues 2,3.
> >>
> >> Now I can add new rule that matches on UDP packet and RSS to queue 0 
> >> and 3 in this case:
> >> TCP packets will be routed to queues 0,3.
> >> UDP packets will be routed to queues 2,3.
> >> All the rest of the traffic will be routed to queues 0,1,2,3
> >>
> >> And just to be clear if now I add a rule to match all packets in 
> >> higher priority,
> >> with RSS to queues 1,2. Then all traffic will be routed to queues 1,2.
> >>
> >> At least this is what is expected, from API point of view.
> >>
> >> Best,
> >> Ori
> > Thank you for your answer. I understand it.
> > hns3 PMD cannot implement the above functions due to hardware limitation.
> > we may need add a check that specified RSS queues cannot be supported 
> > when specified packets types.
> > And only the packet type is specified, which meets the requirements of 
> > rte_flow API.
> > The check for the RTE_ETH_MQ_RX_RSS_FLAG flag in rte_flow is not correct.
> > Thanks, Ori and Stephen😁
> >
> > But, I think, it is necessary for the '.rss_hash_update' and 
> > '.reta_update' APIs
> > in eth_dev_ops to verify this flag. What do you think? @Thomas, 
> > @Ferruh, @Ori and @Stephen.
> What's your take on it? I am looking forward to your reply. Thanks!

I am not sure why you want to check this flag.
I can imagine we configure the hash and the table before enabling RSS
with the RTE_ETH_MQ_RX_RSS_FLAG flag.