On 1/25/2023 1:17 PM, Ferruh Yigit wrote: > On 1/25/2023 9:30 AM, Hanumanth Reddy Pothula wrote: >> ++ Ivan Malov and Andrew Rybchenko >> >>> -----Original Message----- >>> From: Ferruh Yigit <ferruh.yi...@amd.com> >>> Sent: Tuesday, January 24, 2023 11:34 PM >>> To: Hanumanth Reddy Pothula <hpoth...@marvell.com>; Aman Singh >>> <aman.deep.si...@intel.com>; Yuying Zhang <yuying.zh...@intel.com> >>> Cc: dev@dpdk.org; andrew.rybche...@oktetlabs.ru; >>> viachesl...@nvidia.com; Jerin Jacob Kollanukkaran <jer...@marvell.com>; >>> Nithin Kumar Dabilpuram <ndabilpu...@marvell.com> >>> Subject: [EXT] Re: [PATCH v5 2/2] app/testpmd: add command to process >>> Rx metadata negotiation >>> >>> External Email >>> >>> ---------------------------------------------------------------------- >>> On 12/21/2022 2:07 AM, Hanumanth Pothula wrote: >>>> Presently, Rx metadata is sent to PMD by default, leading to a >>>> performance drop as processing for the same in Rx path takes extra >>>> cycles. >>>> >>>> Hence, add new testpmd command, >>>> 'enable port <port_id> nic_to_pmd_rx_metadata' >>>> >>>> This command helps in sending Rx metadata to PMD and thereby Rx >>>> metadata flow command requests are processed. >>>> >>>> Signed-off-by: Hanumanth Pothula <hpoth...@marvell.com> >>> >>> Hi Hanumanth, >>> >>> I agree with Thomas for the patch. >>> >>> 'eth_rx_metadata_negotiate_mp()' requests all Rx metadata offloads to be >>> enabled, but at this stage if there is no flow rule for Rx metadata why it >>> is >>> consuming extra cycles? >>> >>> Can you update driver code to process Rx metadata when it is enabled by >>> application (via 'rte_eth_rx_metadata_negotiate()') AND there is at least >>> one flow rule for it? >> >> #1 What is the purpose of rte_eth_rx_metadata_negotiate() API if it is >> always called by testpmd. >> We thought it was added so that when that metadata is not needed, >> application need not call this >> thereby saving cycles/bandwidth. >> > > Purpose looks like to 'negotiate', so it is both ways: > a) To learn Rx Meta capability of HW > b) To configure Rx Meta feature for HW > > So it is both to help application to learn what flow rule is supported > and to help driver to improve performance. > > Before this API application assumed all Rx metadata flow rules are > supported, so why enabling all causing performance drop, that was the > default without this API. > > But other-way around can be true, to disable some offloads can improve > driver performance. > > Looking again, >
please ignore this email, it has unfinished draft reply by mistake, I will send a new one. > >> #2 We use this API similar to Rx/Tx offload flags so that we can set things >> up before device is >> configured. We thought that is the purpose of having this negotiate API and >> avoid depleting offload flags. >> >> #3 Generally any new offloads added to DPDK would be in disabled state in >> testpmd and we would have >> an option to enable it. In this case, testpmd is by default calling this >> negotiation. >> >> We can update the driver if the purpose of this API is clear. > > > Hi Hanumanth, > > After looking the history of the API again, you may be right. > > One of the previous version commit log describes the intention better > [1], because of negative performance impact of enabling Rx metadata > offload by default and difficulty to switch configuration dynamically, > there is a desire to learn application intention before configuration. > > Although I have some concerns with this API [2] it is already there as > stable API. > > So it sounds reasonable to make this configurable for a test > application, indeed intention of the API is to get this configuration > from application and operate based on it. > > Next question is what should be the default value, I am not sure about > it, there are only a few drivers impacted from this overall. > For the Thomas' point, it helps to test the feature of its impact if it > is enabled by default. > > Will it work to enable them all by default and add capability to disable > it in testpmd, which helps to run performance tests also to verify the > impact of the API? > > Thanks, > ferruh > > > > [1] > https://inbox.dpdk.org/dev/20210902142359.28138-2-ivan.ma...@oktetlabs.ru/ > > > [2] > API does two things: > a) Learn Rx Meta capability of HW > b) Configure Rx Meta feature for HW > > Functionality (a) conflicts with rest of the flow rules that capability > checked via `rte_flow_validate()` API. > > Functionality (b) conflicts with configuring flow actions, this > configuration should be controlled by flow rule not with a specific API, > although I understand the reasoning behind the API. > > RSS_HASH offload seems given example in the discussions but it is still > controlled via offload flag, there is no specific API to configure RSS > hash functionality, same could be done here. > [/2]
