DPDK patches and discussions
 help / color / Atom feed
From: Phil Yang <Phil.Yang@arm.com>
To: oulijun <oulijun@huawei.com>,
	"wenzhuo.lu@intel.com" <wenzhuo.lu@intel.com>,
	"beilei.xing@intel.com" <beilei.xing@intel.com>,
	"adrien.mazarguil@6wind.com" <adrien.mazarguil@6wind.com>,
	"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"linuxarm@huawei.com" <linuxarm@huawei.com>, nd <nd@arm.com>,
	nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH v3] app/testpmd: fix the default RSS key configuration
Date: Tue, 22 Sep 2020 15:44:29 +0000
Message-ID: <DB7PR08MB3865551BFBF69EEE1A690134E93B0@DB7PR08MB3865.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <b1546aaa-63a2-94f3-d749-08c887d5d7b1@huawei.com>

dev <dev-bounces@dpdk.org> On Behalf Of Lijun Ou writes:


> >> Subject: [dpdk-dev] [PATCH v3] app/testpmd: fix the default RSS key
> >> configuration
> >
> > Hi Lijun,
> >
> > Please fix the coding style issues.
> >
> > "Must be a reply to the first patch (--in-reply-to)."
> >
> >
> >>
> >> When a user runs a flow create cmd to configure an RSS rule
> >> with specifying the empty rss actions in testpmd, this mean
> >                                                                                                      ^^^ means
> >> that the flow gets the default RSS functions from the valid
> >> NIC default RSS hash key. However, the testpmd is not set
> >                                                                                            ^^^ is set xxx incorrectly
> >> the default RSS key incorrectly when RSS key is not specified
> >> in flow create cmd.
> >
> > Use the NIC valid default RSS key instead of the testpmd dummy RSS key in
> the flow configuration when the RSS key is not specified in the flow rule. If
> the NIC RSS key is invalid, it will use testpmd dummy RSS key as the default
> key.
> >
> > Is that good to put it in this way? Because I think it is not a bug, your patch
> offers an approach to update the default testpmd RSS key.
> >
> Do you have any better advice?  or don't use my approach? I think the


No, I think you misunderstood me. I agree with your proposal and your patch looks good to me.
My suggestion is to reword the commit message to highlight that you mare making testpmd use the valid NIC RSS key as the default flow RSS key in this patch. 
In my perspective, if you don't specify any RSS key in your flow rule, it should allow any available RSS key work as the default key. 
So use a dummy RSS key is correct as well.


> previous methods are easy to misunderstand.Can we use the NUL KEY
> solution and fix the problem that the length is 0 and the RSS key is not
> NULL ?
> 
> Thanks
> Lijun Ou
> > I would also suggest making the commit message shorter and move the
> test result into the cover letter.
> > Because the checkepatch tool is not happy with the hex dumped below.
> > http://mails.dpdk.org/archives/test-report/2020-September/151395.html
> >
> >
> > Thanks,
> > Phil
> >
> >> the cmdline as flows:
> >> 1. first, startup testpmd:
> >> testpmd> show port 0 rss-hash key
> >> RSS functions:
> >>   all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> >> RSS key:
> >>
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> >> 20C6A42B73BBEAC01FA
> >>
> >> 2. create a rss rule
> >> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> >> types ipv4-udp end queues end / end
> >>
> >> 3. show rss-hash key
> >> testpmd> show port 0 rss-hash key
> >> RSS functions:
> >>   all ipv4-udp udp
> >> RSS key:
> >>
> 74657374706D6427732064656661756C74205253532068617368206B65792C206F
> >> 76657272696465
> >>
> >> Now, it uses rte_eth_dev_rss_hash_conf_get to correctly the
> >> default rss key. the cmdline and result as flows:
> >> testpmd> show port 0 rss-hash key
> >> RSS functions:
> >>   all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> >> RSS key:
> >>
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F2
> >> 0C
> >> 6A42B73BBEAC01FA
> >> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> >> types ipv4-udp end queues end / end
> >> testpmd> show port 0 rss-hash key
> >> RSS functions:
> >>   all ipv4-udp udp
> >> RSS key:
> >>
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> >> 20C6A42B73BBEAC01FA
> >>
> >> Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> >> ---
> >> V2->V3:
> >> -fix checkpatch warning.
> >>
> >> V1->V2:
> >> -fix the commit.
> >> ---
> >>   app/test-pmd/cmdline_flow.c | 8 ++++++++
> >>   1 file changed, 8 insertions(+)
> >>
> >> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
> pmd/cmdline_flow.c
> >> index 6263d30..e6648da 100644
> >> --- a/app/test-pmd/cmdline_flow.c
> >> +++ b/app/test-pmd/cmdline_flow.c
> >> @@ -4312,6 +4312,7 @@ parse_vc_action_rss(struct context *ctx, const
> >> struct token *token,
> >>   		action_rss_data->queue[i] = i;
> >>   	if (!port_id_is_invalid(ctx->port, DISABLED_WARN) &&
> >>   	    ctx->port != (portid_t)RTE_PORT_ALL) {
> >> +		struct rte_eth_rss_conf rss_conf = {0};
> >>   		struct rte_eth_dev_info info;
> >>   		int ret2;
> >>
> >> @@ -4322,6 +4323,13 @@ parse_vc_action_rss(struct context *ctx, const
> >> struct token *token,
> >>   		action_rss_data->conf.key_len =
> >>   			RTE_MIN(sizeof(action_rss_data->key),
> >>   				info.hash_key_size);
> >> +
> >> +		rss_conf.rss_key_len = sizeof(action_rss_data->key);
> >> +		rss_conf.rss_key = action_rss_data->key;
> >> +		ret2 = rte_eth_dev_rss_hash_conf_get(ctx->port,
> >> &rss_conf);
> >> +		if (ret2 != 0)
> >> +			return ret2;
> >> +		action_rss_data->conf.key = rss_conf.rss_key;
> >>   	}
> >>   	action->conf = &action_rss_data->conf;
> >>   	return ret;
> >> --
> >> 2.7.4
> >
> > .
> >

  reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10  1:51 Lijun Ou
2020-09-22  9:51 ` Phil Yang
2020-09-22 13:50   ` oulijun
2020-09-22 15:44     ` Phil Yang [this message]
2020-09-24 13:45 ` [dpdk-dev] [PATCH v4] RSS key use with testpmd Lijun Ou
2020-09-24 13:45   ` [dpdk-dev] [PATCH v4] app/testpmd: fix the default RSS key configuration Lijun Ou
2020-09-29 15:35     ` Phil Yang
2020-09-30 12:57     ` Ferruh Yigit
2020-10-09 11:55       ` oulijun
2020-10-09 18:27         ` Ferruh Yigit
2020-10-09 18:54           ` Ferruh Yigit
2020-09-30 13:36     ` Ferruh Yigit
2020-10-09 11:59       ` oulijun
2020-10-09 18:36         ` Ferruh Yigit
2020-10-15 12:41     ` [dpdk-dev] [PATCH v5] " Lijun Ou
2020-10-15 13:52       ` Ferruh Yigit
2020-10-15 14:04         ` oulijun
2020-10-15 14:43           ` Ferruh Yigit
2020-10-15 16:05             ` Ferruh Yigit
2020-10-15 23:21               ` Ophir Munk
2020-10-15 23:53                 ` Ferruh Yigit
2020-10-16  0:14                   ` Ajit Khaparde
2020-10-16  6:46                   ` Ophir Munk
2020-10-16 10:05                     ` oulijun
2020-10-16 15:13                       ` Ophir Munk
2020-10-16 10:04                   ` oulijun
2020-10-16 10:57                     ` Ferruh Yigit
2020-10-16 14:59                       ` Ophir Munk
2020-10-20  9:00                       ` oulijun
2020-10-20 10:02                         ` Ferruh Yigit
2020-10-20 13:35                           ` oulijun
2020-10-20 14:34                             ` Ferruh Yigit
2020-10-21  8:19                               ` oulijun
2020-10-21  9:38                                 ` Ferruh Yigit
2020-10-21 10:07                                   ` oulijun
     [not found]       ` <20201015195637.26476-1-robot@bytheb.org>
2020-10-16  9:39         ` [dpdk-dev] |WARNING| pw80899 " oulijun
2020-10-16  9:41           ` David Marchand
2020-09-30 13:17   ` [dpdk-dev] [PATCH v4] RSS key use with testpmd Ferruh Yigit
2020-10-09 12:09     ` oulijun
2020-10-09 18:52       ` Ferruh Yigit
2020-10-10  3:07         ` Phil Yang
2020-10-14  6:15         ` oulijun
2020-10-14  8:41           ` Ferruh Yigit

Reply instructions:

You may reply publically 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=DB7PR08MB3865551BFBF69EEE1A690134E93B0@DB7PR08MB3865.eurprd08.prod.outlook.com \
    --to=phil.yang@arm.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=linuxarm@huawei.com \
    --cc=nd@arm.com \
    --cc=oulijun@huawei.com \
    --cc=wenzhuo.lu@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

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox