DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Hanumanth Reddy Pothula <hpothula@marvell.com>,
	Aman Singh <aman.deep.singh@intel.com>,
	Yuying Zhang <yuying.zhang@intel.com>,
	Ivan Malov <ivan.malov@oktetlabs.ru>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Thomas Monjalon <thomas@monjalon.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"viacheslavo@nvidia.com" <viacheslavo@nvidia.com>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>
Subject: Re: [EXT] Re: [PATCH v5 2/2] app/testpmd: add command to process Rx metadata negotiation
Date: Wed, 25 Jan 2023 13:17:03 +0000	[thread overview]
Message-ID: <1cc09d94-cd90-e60a-27c1-4a0b7a6a3c02@amd.com> (raw)
In-Reply-To: <PH0PR18MB47507AAB867DF2197B7DC055CBCE9@PH0PR18MB4750.namprd18.prod.outlook.com>

On 1/25/2023 9:30 AM, Hanumanth Reddy Pothula wrote:
> ++ Ivan Malov and Andrew Rybchenko
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Tuesday, January 24, 2023 11:34 PM
>> To: Hanumanth Reddy Pothula <hpothula@marvell.com>; Aman Singh
>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
>> Cc: dev@dpdk.org; andrew.rybchenko@oktetlabs.ru;
>> viacheslavo@nvidia.com; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> Nithin Kumar Dabilpuram <ndabilpuram@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 <hpothula@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,


> #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.malov@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]

  parent reply	other threads:[~2023-01-25 13:17 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-01  4:41 [PATCH] app/testpmd: add command line argument 'rx-metadata' Hanumanth Pothula
2022-08-01 13:11 ` Hanumanth Pothula
2022-08-01 13:13 ` Hanumanth Pothula
2022-08-01 19:41   ` Ivan Malov
2022-08-02 16:45   ` [PATCH] app/testpmd: add command line argument 'nic-to-pmd-rx-metadata' Hanumanth Pothula
2022-08-02 16:45     ` [PATCH v2 1/2] version: 22.11-rc0 Hanumanth Pothula
2022-08-02 16:45     ` [PATCH v2 2/2] app/testpmd: add command line argument 'nic-to-pmd-rx-metadata' Hanumanth Pothula
2022-08-02 17:51   ` [PATCH v2 1/1] " Hanumanth Pothula
2022-08-30 12:36     ` Hanumanth Reddy Pothula
2022-09-01  8:03     ` Singh, Aman Deep
2022-10-04 14:48       ` Andrew Rybchenko
2022-10-06 18:35     ` [PATCH v3 1/1] app/testpmd: control passing Rx metadata to PMD Hanumanth Pothula
2022-10-17  8:32       ` Andrew Rybchenko
2022-10-27  7:34         ` Thomas Monjalon
2022-10-27 12:54           ` Thomas Monjalon
2022-12-02 16:14             ` [EXT] " Hanumanth Reddy Pothula
2022-12-02 19:41               ` Thomas Monjalon
2022-12-05  7:59                 ` Hanumanth Reddy Pothula
2022-12-05  8:28                   ` Thomas Monjalon
2022-12-05  9:43                     ` Slava Ovsiienko
2022-12-20 20:02       ` [PATCH v4 1/2] ethdev: control Rx metadata negotiation Hanumanth Pothula
2022-12-20 20:02         ` [PATCH v4 2/2] app/testpmd: add command to process " Hanumanth Pothula
2022-12-20 21:23           ` Stephen Hemminger
2022-12-21  2:07         ` [PATCH v5 1/2] ethdev: fix ethdev configuration state on reset Hanumanth Pothula
2022-12-21  2:07           ` [PATCH v5 2/2] app/testpmd: add command to process Rx metadata negotiation Hanumanth Pothula
2023-01-18 10:32             ` Thomas Monjalon
2023-01-19 10:33               ` [EXT] " Hanumanth Reddy Pothula
2023-01-25 12:51                 ` Thomas Monjalon
2023-01-24 18:04             ` Ferruh Yigit
2023-01-25  9:30               ` [EXT] " Hanumanth Reddy Pothula
2023-01-25 12:55                 ` Thomas Monjalon
2023-01-25 13:55                   ` Ferruh Yigit
2023-01-25 13:59                     ` Thomas Monjalon
2023-01-25 14:42                       ` Nithin Kumar Dabilpuram
2023-01-26 11:03                         ` Thomas Monjalon
2023-01-27  5:02                           ` Nithin Kumar Dabilpuram
2023-01-27  8:54                             ` Thomas Monjalon
2023-01-27 10:42                               ` Nithin Kumar Dabilpuram
2023-01-27 15:01                                 ` Thomas Monjalon
2023-01-31 16:17                                   ` Jerin Jacob
2023-01-31 23:03                                     ` Thomas Monjalon
2023-02-01  6:10                                       ` Ivan Malov
2023-02-01  7:16                                         ` Andrew Rybchenko
2023-02-01  8:53                                           ` Jerin Jacob
2023-02-01  9:00                                             ` Ori Kam
2023-02-01  9:05                                               ` Thomas Monjalon
2023-02-01  9:07                                                 ` Andrew Rybchenko
2023-02-01  9:14                                                   ` Jerin Jacob
2023-02-01  9:29                                                     ` Andrew Rybchenko
2023-02-01 10:48                                                       ` Jerin Jacob
2023-02-01 10:58                                                         ` Andrew Rybchenko
2023-02-01 11:04                                                           ` Thomas Monjalon
2023-02-01 11:15                                                             ` Jerin Jacob
2023-02-01 11:35                                                               ` Ferruh Yigit
2023-02-01 13:48                                                                 ` Jerin Jacob
2023-02-01 14:50                                                                   ` Ferruh Yigit
2023-02-01 15:22                                                                     ` Jerin Jacob
2023-02-02  8:43                                                                       ` Ferruh Yigit
2023-02-02  8:50                                                                         ` Ivan Malov
2023-02-02  9:17                                                                           ` Ferruh Yigit
2023-02-02 10:41                                                                             ` Ivan Malov
2023-02-02 10:48                                                                               ` Ivan Malov
2023-02-02 11:41                                                                                 ` Thomas Monjalon
2023-02-02 11:55                                                                                   ` Ivan Malov
2023-02-02 12:03                                                                                     ` Thomas Monjalon
2023-02-02 12:21                                                                                       ` Andrew Rybchenko
2023-02-01 11:20                                                             ` Ivan Malov
2023-01-25 13:17                 ` Ferruh Yigit [this message]
2023-01-25 13:21                   ` Ferruh Yigit
2023-01-25 13:21                 ` Ferruh Yigit
2023-01-16 10:43           ` [PATCH v5 1/2] ethdev: fix ethdev configuration state on reset Hanumanth Reddy Pothula
2023-01-18 10:29           ` Thomas Monjalon
2023-01-24 18:14             ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1cc09d94-cd90-e60a-27c1-4a0b7a6a3c02@amd.com \
    --to=ferruh.yigit@amd.com \
    --cc=aman.deep.singh@intel.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=hpothula@marvell.com \
    --cc=ivan.malov@oktetlabs.ru \
    --cc=jerinj@marvell.com \
    --cc=ndabilpuram@marvell.com \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    --cc=yuying.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).