From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id DBEA7A0C4A; Wed, 7 Jul 2021 15:11:00 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6582B413A8; Wed, 7 Jul 2021 15:11:00 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id C29BF41391 for ; Wed, 7 Jul 2021 15:10:58 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 3C2487F53D; Wed, 7 Jul 2021 16:10:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 3C2487F53D DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1625663458; bh=ZbkI/9exO63W543X4QTh65X4CfeGBvpCiFc6VBvT+4A=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=miyzFPMgXSe987ZZZ84VFPtjvUowbLCwOtR/ayG2T704lIABvvmBgKUOZLHtVJ/Pl L9KSrR36XrlsuKF1LZ+oGUng3ZZIRYKcFm484R++uxpwdskMzrRQfFMTkpWPTY9EQG To+37OTo7JUZaQQhoFRDeL12xAlArzhY3hV1jqlI= To: "Zhang, Qi Z" , "Zhang, AlvinX" , "ajit.khaparde@broadcom.com" Cc: "dev@dpdk.org" References: <20210603080352.10924-1-alvinx.zhang@intel.com> <20210615081956.23656-1-alvinx.zhang@intel.com> <7930ac91-7f55-6b42-f086-701d952fc151@oktetlabs.ru> <774225cd-b2f9-30e2-31c3-651329dfa25e@oktetlabs.ru> <238ebfd3-e869-433b-9249-311900ded588@oktetlabs.ru> <01b6e32291824df1beceed707eb343b2@intel.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <8d05b4a9-7bf0-8ffd-27f2-cb5ce106773d@oktetlabs.ru> Date: Wed, 7 Jul 2021 16:10:58 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <01b6e32291824df1beceed707eb343b2@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 7/7/21 4:00 PM, Zhang, Qi Z wrote: > > >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Wednesday, July 7, 2021 5:49 PM >> To: Zhang, Qi Z ; Zhang, AlvinX >> ; ajit.khaparde@broadcom.com >> Cc: dev@dpdk.org >> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types >> >> On 7/7/21 6:23 AM, Zhang, Qi Z wrote: >>> >>> >>>> -----Original Message----- >>>> From: Andrew Rybchenko >>>> Sent: Tuesday, July 6, 2021 4:05 PM >>>> To: Zhang, Qi Z ; Zhang, AlvinX >>>> ; ajit.khaparde@broadcom.com >>>> Cc: dev@dpdk.org >>>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload >>>> types >>>> >>>> On 7/6/21 10:18 AM, Zhang, Qi Z wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Zhang, AlvinX >>>>>> Sent: Tuesday, July 6, 2021 3:06 PM >>>>>> To: Andrew Rybchenko ; Zhang, Qi Z >>>>>> ; ajit.khaparde@broadcom.com >>>>>> Cc: dev@dpdk.org >>>>>> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS >>>>>> offload types >>>>>> >>>>>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf { >>>>>>>> #define ETH_RSS_PPPOE (1ULL << 31) >>>>>>>> #define ETH_RSS_ECPRI (1ULL << 32) >>>>>>>> #define ETH_RSS_MPLS (1ULL << 33) >>>>>>>> +#define ETH_RSS_IPV4_CHKSUM (1ULL << 34) >>>>>>>> +#define ETH_RSS_L4_CHKSUM (1ULL << 35) >>>>>>> >>>>>>> What does efine which L4 protocols are supported? How user will know? >>>>>> >>>>>> I think if we want to support L4 checksum RSS by using below >>>>>> command port config all rss (all|default|eth|vlan|...) >>>>>> >>>>>> We must define TCP/UDP/SCTP checksum RSS separately: >>>>>> #define ETH_RSS_TCP_CHKSUM (1ULL << 35) >>>>>> #define ETH_RSS_UDP_CHKSUM (1ULL << 36) >>>>>> #deifne ETH_RSS_SCTP_CHKSUM (1ULL << 37) >>>>>> >>>>>> Here 3 bits are occupied, this is not good for there are not many >>>>>> bits >>>> available. >>>>>> >>>>>> If we only want to using it in flows, we only need to define >>>>>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4 >>>>>> protocol type. >>>>>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss >>>>>> types l4-chksum end queues end / end >>>>> >>>>> +1, the pattern already give the hint to avoid the ambiguity and I >>>>> +think we >>>> already have ETH_RSS_LEVEL to figure out inner or outer. >>>> >>>> The problem that it may be used in generic RSS flags which has no the >> context. >>>> Also even in the case of flow API context could have no L4 protocol at all. >>> >>> For generic case, it can simply assume it cover all L4 checksum cases and I'm >> not sure if any user intend to use it as generic RSS, pmd can simply reject it if >> it's not necessary to support. >> >> Try to look at it from an application point of view which does not know any >> specifics of the driver. >> >> * Get dev_info and see ETH_RSS_L4_CHKSUM, good!, would like to >> use it. > > > The PMD should not expose it if it don't want to (or not able to) support all l4 checksum from generic RSS configure Document what is "all L4". > > And we should assume this is only apply for generic RSS configure but not for flow API. I don't think so. IMHO, it should report all RSS capabilities regardless generic vs flow API RSS action. It is just RSS capabilities reporting w/o any context. > > Because the rte_flow_validate is the recommended method to check if a RSS action is supported in flow API or not. It could restrict the subset. But superset should be reported in caps. > >> >> * If I try to use it in default RSS config, but the request >> fail, it could be very confusing. >> >> * Will it distribute TCP packets? UDP packets? SCTP packets? >> Or should I care about RSS for some of them based on other >> supported fields? E.g. if SCTP is not supported by the NIC, >> I need to install RSS flow rule for the IP protocol to do >> RSS based on IPv4/IPv6 addresses. But if SCTP is supported, >> I'm happy to use ETH_RSS_L4_CHKSUM for it as well. >> >>> In flow API, if no l4 protocol in pattern , the PMD should return >>> failure (or maybe some default behavior), and I think this is not a >>> new question as it happens all the cases >>> e.g.: >>> pattern eth / vlan / end action rss type ipv4 . >> >> IMHO, it would be pretty logical to apply RSS to IPv4 packets only and send >> everything else to default queue. > > Yes, this also make sense to me, but I think PMD's flow parser still can have more strict check, as it does not drop any feature that the NIC can support. > >> >>>> >>>> Is UDP checksum 0 treated as no checksum and go to default queue or >>>> treated as a regular checksum with value equal to 0? >>> >>> I think we can treat it as value 0, as least our hardware behavior like this, is >> this any issue? >> >> OK, no problem. Just document it. >> >>>> >>>> I tend to agree that 3 flags is too much for the feature, but one >>>> flag without properly defined meaning is not good as well. >>>> >>>> I just want rules to be defined and documented.' >>> >>> Agree, we need more document for this. if you agree above proposal. >>> >