From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DEA14A318B for ; Fri, 18 Oct 2019 09:33:07 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9B4DD1C0CD; Fri, 18 Oct 2019 09:33:06 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 008891C0CC for ; Fri, 18 Oct 2019 09:33:04 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (webmail.solarflare.com [12.187.104.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 7C897980067; Fri, 18 Oct 2019 07:33:03 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ocex03.SolarFlarecom.com (10.20.40.36) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Fri, 18 Oct 2019 00:32:59 -0700 To: Ferruh Yigit , , CC: , Adrien Mazarguil , "Thomas Monjalon" , Xiaolong Ye References: <20191010105130.3382-1-pbhagavatula@marvell.com> <20191017120245.984-1-pbhagavatula@marvell.com> <868c4186-a899-3974-ce1a-9d0d395b2f81@intel.com> From: Andrew Rybchenko Message-ID: Date: Fri, 18 Oct 2019 10:32:50 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <868c4186-a899-3974-ce1a-9d0d395b2f81@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ocex03.SolarFlarecom.com (10.20.40.36) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24984.005 X-TM-AS-Result: No-20.115100-4.000000-10 X-TMASE-MatchedRID: 8HTFlOrbAtH4ECMHJTM/uS2416nc3bQlpvDLLlsAH/0rxUs8Nw/2fiGU b2JNxi1qWASbhXBhlp4v3RWjRyG6TtF/tAhYTSzX34b00P59Zxm3xwqfnvnKHny/Hx1AgJrrstf L6WfCYzEoFhurGV67Z11A7/0AFfe7GXC6wGf12RWqNnzrkU+2mvsJB65N+sbMSX8n1Gj4wAEBSr A72lIhPcKRiXfaKYmpV5ch7TAAY1TVqiuVd9LMeRlJRfzNw8afGUqOjzOC7Ip7U6ND9bLE/m2rD qsRwTo30P8y4NB7uOvvpVnoxBz4V/LETRWl+wGP/ccgt/EtX/35qGeseGYAlOG3yXbqLuSX9VlG BjCDncjvYIeV5cEfgxORzWaRXvoq3qSGFWY8YIURZz7NjbwI0mcuTq1AtxjpUVuDEJ37MoYdBq5 wHTF42njaU2yW0nfHyjNYHg1Z0TVhNdhmbyNY5UaMPBFKXyAUvHa+hRvAUH0LFqt+tcr8+CVefD VGpJ9w10g7wK6JDf6nn8iX90C8mV0W0181PjfblTsGW3DmpUui8D/o42y/Svn6214PlHOFsT+6X vVK5M+vxGOtmo34h+OWZlNALzwUrFVBH8/bXVHM0ihsfYPMYVRXlDwI7HfGalBYlHon8nX3muLh +GPYb30VgsgQseKutZB0R3gol4xqIRQGH08+HrsHVDDM5xAP+eBf9ovw8I0f7zGSt7fdn1B1btr ZqWv8eSxJ2Bou0OS3DVbxRAj7q7jfewp5zUsBB/XUnmGGOOoxXH/dlhvLv4u6fTXJM2Trl23JNS aQyjO51zElP0zIlPVMLFayrLGrruopIs03JaOeAiCmPx4NwLTrdaH1ZWqC1B0Hk1Q1KyLUZxEAl FPo846HM5rqDwqtlExlQIQeRG0= X-TM-AS-User-Approved-Sender: No X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--20.115100-4.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24984.005 X-MDID: 1571383984-1wP8_-ZsFNyM Subject: Re: [dpdk-dev] [PATCH v12 0/7] ethdev: add new Rx offload flags X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Hi Ferruh, since I've reviewed I'll reply as I understand it. On 10/17/19 8:43 PM, Ferruh Yigit wrote: > On 10/17/2019 1:02 PM, pbhagavatula@marvell.com wrote: >> From: Pavan Nikhilesh >> >> Add new Rx offload flags `DEV_RX_OFFLOAD_RSS_HASH` and >> `DEV_RX_OFFLOAD_FLOW_MARK`. These flags can be used to >> enable/disable PMD writes to rte_mbuf fields `hash.rss` and `hash.fdir.hi` >> and also `ol_flags:PKT_RX_RSS` and `ol_flags:PKT_RX_FDIR`. > Hi Pavan, > > Initially sorry for involving late, > > When we expose an interface to the applications, they will expect those will be > respected by underlying PMDs. > As far as I can see drivers are updated to report new added Rx offload flags as > supported capabilities but drivers are not using those flags at all, so > application providing that flag won't really enable/disable anything, I think > this is a problem and it is wrong to lie even for the PMDs J It is required to let applications know that the offload is supported. There are a number of cases when an offload cannot be disabled, but it does not mean that the offload must not be advertised. If driver see benefits from disabling the offload (e.g. avoid delivery of RSS hash from NIC to host), it can do it after the patchset. > Specific to `DEV_RX_OFFLOAD_RSS_HASH`, we have already some RSS config > structures and it is part of the 'rte_eth_dev_configure()' API, won't it create > multiple way to do same thing? No, a new offload is responsible for RSS hash delivery from NIC to host and fill in in mbuf returned to application on Rx. > And for the `ol_flags:PKT_RX_RSS` flag, it was already used to mark that > 'mbuf::hash::rss' is valid, right? Is there anything new related that in the set? As I understand you mean, ol_flags::PKT_RX_RSS_HASH. Yes, the new offload allows say if application needs it or now. Basically it decouples RSS distribution and hash delivery. > Specific to the `DEV_RX_OFFLOAD_FLOW_MARK` and `RTE_FLOW_ACTION_FLAG`, they are > rte_flow actions, application can verify and later request these actions via > rte_flow APIs. Why we are adding an additional RX_OFFLOAD flag for them? The reason is basically the same as above. HW needs to know in advance, if application is going to use flow marks and configure Rx queue to enable the information delivery. >> Add new packet type set function `rte_eth_dev_set_supported_ptypes`, >> allows application to inform PMDs about the packet types it is interested >> in. Based on ptypes requested by application PMDs can optimize the Rx path. > OK to the API, but why "Packet type parsing" feature updated to say it should > implement this API? > Is this API really required to say "Packet type parsing" supported by PMD? As I understand it is not strictly required, but related to the feature. >> For example, if a given PMD doesn't support any packet types that the >> application is interested in then the application can disable[1] writes to >> `mbuf.packet_type` done by the PMD and use a software ptype parser. >> [1] rte_eth_dev_set_supported_ptypes(*port_id*, RTE_PTYPE_UNKNOWN, >> NULL, 0); > And for the 7/7 patch, why we are updating all examples, is the API something do > we really need to call for any DPDK application? I am for leaving the default > behavior unless there is a very specific case for set or disable packet typing. > Instead implement a command in testpmd to test this feature. If an application does not use packet types provided in mbuf, it is better to inform PMD that the information is not required to allow PMD to do optimizations. Yes, may be it would be better to have it as the default behaviour, but it would be behaviour change in comparison to previous DPDK releases and it is better to avoid it. Thanks, Andrew. >> v12 Changes: >> ----------- >> - Rebase onto next-net. >> >> v11 Changes: >> ----------- >> - Use RTE_DIM to get array size. >> - Since we are using a list of MASKs to validate ptype_mask return -EINVAL >> if any unknown mask is set. >> - Rebase to TOT. >> >> v10 Changes: >> ----------- >> - Modify ptype_mask validation in set_supported_ptypes.(Andrew) >> >> v9 Changes: >> ---------- >> - Add ptype_mask validation in set_supported_ptypes.(Andrew) >> - Make description more verbose. >> >> v8 Changes: >> ---------- >> - Make description more verbose. >> - Set RTE_PTYPE_UNKNOWN in set_ptypes array when either get ot set ptypes >> is not supported by ethernet device. >> >> v7 Changes: >> ---------- >> - Fix unused variable in net/octeontx2 >> >> v6 Changes: >> ---------- >> - Add additional checks for set supported ptypes.(Andrew) >> - Clarify `rte_eth_dev_set_supported_ptypes` documentation. >> - Remove DEV_RX_OFFLOAD_FLOW_MARK emulation from net/octeontx2. >> >> v5 Changes: >> ---------- >> - Fix typos. >> >> v4 Changes: >> ---------- >> - Set the last element in set_ptype array as RTE_PTYPE_UNKNOWN to mark the end >> of array. >> - Fix invalid set ptype function call in examples. >> - Remove setting rte_eth_dev_set_supported_ptypes to UNKNOWN in l3fwd-power. >> >> v3 Changes: >> ---------- >> - Add missing release notes. (Andrew) >> - Re-word various descriptions. >> - Fix ptype set logic. >> >> v2 Changes: >> ---------- >> - Update release notes. (Andrew) >> - Redo commit logs. (Andrew) >> - Disable ptype parsing for unsupported examples. (Jerin) >> - Disable RSS write only in generic mode eventdev_pipeline. (Jerin) >> - Modify set_supported_ptypes function to return successfuly set mask >> instead of failure. >> - Dropped set_supported_ptypes to drivers by handling in library >> layer, interested PMD can add it in. >> >> Pavan Nikhilesh (7): >> ethdev: add set ptype function >> ethdev: add mbuf RSS update as an offload >> ethdev: add flow action type update as an offload >> drivers/net: update Rx RSS hash offload capabilities >> drivers/net: update Rx flow flag and mark capabilities >> examples/eventdev_pipeline: add new Rx RSS hash offload >> examples: disable Rx packet type parsing > <...>