On Wed, Mar 23, 2022 at 12:04 PM Thomas Monjalon wrote: > > 23/03/2022 12:04, lihuisong (C): > > 在 2022/3/23 17:14, Thomas Monjalon 写道: > > > 23/03/2022 04:05, lihuisong (C): > > >> 在 2022/3/23 1:13, Thomas Monjalon 写道: > > >>> 21/03/2022 08:14, lihuisong (C): > > >>>> 2022/3/10 16:08, lihuisong (C): > > >>>>> 2022/3/9 17:55, Ori Kam: > > >>>>>> From: lihuisong (C) > > >>>>>>> 2022/3/3 10:47, lihuisong (C): > > >>>>>>>> 2022/3/2 22:07, Ori Kam: > > >>>>>>>>> From: lihuisong (C) > > >>>>>>>>>> 2022/3/1 0:42, Ferruh Yigit: > > >>>>>>>>>>> On 2/28/2022 3:21 AM, Min Hu (Connor) wrote: > > >>>>>>>>>>>> From: Huisong Li > > >>>>>>>>>>>> > > >>>>>>>>>>>> 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 > > >>>>>>>>>>> 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 want to make sure that the behavior that PMD configured RSS is > > >> consistent across different interfaces. The RTE_ETH_MQ_RX_RSS_FLAG > > >> flag is a switch to enable RSS hash. If the switch isn't open, some > > >> PMD do not configure RSS function. I think the consistency is necessary. > > >> If not set RSS muti-queue mode, it is unnecessary to configure RSS. > > >> > > >>> I can imagine we configure the hash and the table before enabling RSS > > >>> with the RTE_ETH_MQ_RX_RSS_FLAG flag. > > >> The flag is derived from dev_configure() which also configures > > >> hash and key. I don't think it makes sense to configure hash and > > >> reta before calling dev_configure. Because they'll be updated. > > >> This is similar to configuring mtu. > > > OK I see your point. > > > So you would like to return an error in RSS functions > > > if the flag RTE_ETH_MQ_RX_RSS_FLAG is not set? > > Yes > > > Should it be checked in ethdev library or PMDs? > > I think it's better to put it in the ethdev layer if we do it. > > Should we add this check in 'rss_hash_update' and 'reta_update' APIs? > > I'm OK with adding the check in ethdev. +1 Apologies for getting late to this thread.