patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Li, Xiaoyun" <xiaoyun.li@intel.com>
To: "Yu, DapengX" <dapengx.yu@intel.com>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and Tx offload reconfig cmd
Date: Mon, 12 Apr 2021 02:21:43 +0000
Message-ID: <DM4PR11MB553442F61FCAA7230B234FEB99709@DM4PR11MB5534.namprd11.prod.outlook.com> (raw)
In-Reply-To: <DM5PR11MB140176B51C73A291FBEECF888C739@DM5PR11MB1401.namprd11.prod.outlook.com>



> -----Original Message-----
> From: Yu, DapengX <dapengx.yu@intel.com>
> Sent: Friday, April 9, 2021 18:29
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and Tx offload
> reconfig cmd
> 
> 
> 
> > -----Original Message-----
> > From: Li, Xiaoyun <xiaoyun.li@intel.com>
> > Sent: Friday, April 9, 2021 3:50 PM
> > To: Yu, DapengX <dapengx.yu@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org
> > Subject: RE: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and Tx
> > offload reconfig cmd
> >
> >
> >
> > > -----Original Message-----
> > > From: Yu, DapengX <dapengx.yu@intel.com>
> > > Sent: Friday, April 9, 2021 13:25
> > > To: Yigit, Ferruh <ferruh.yigit@intel.com>; Li, Xiaoyun
> > > <xiaoyun.li@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> > > Cc: dev@dpdk.org; stable@dpdk.org
> > > Subject: RE: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and Tx
> > > offload reconfig cmd
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > > > Sent: Thursday, April 8, 2021 11:42 PM
> > > > To: Yu, DapengX <dapengx.yu@intel.com>; Li, Xiaoyun
> > > > <xiaoyun.li@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> > > > Cc: dev@dpdk.org; stable@dpdk.org
> > > > Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix queue Rx and
> > > > Tx offload reconfig cmd
> > > >
> > > > On 4/1/2021 9:28 AM, dapengx.yu@intel.com wrote:
> > > > > From: Dapeng Yu <dapengx.yu@intel.com>
> > > > >
> > > > > Configure per queue rx offloading and per queue tx offloading
> > > > > command shouldn't trigger the rte_eth_dev_configure() to
> > > > > reconfigure
> > > device.
> > > > >
> > > > > The patch sets the queue reconfiguration flag only, and does not
> > > > > set the device reconfiguration flag. Therefore after port is
> > > > > restarted,
> > > > > rte_eth_dev_configure() will not be called again.
> > > > >
> > > >
> > > > Just to clarify the impact, was calling 'rte_eth_dev_configure()'
> > > > causing any problem, is this fixing any issue?
> > > > Or is this patch an optimization to eliminate an unnecessary call?
> > > >
> > > This patch does fix an issue, and it also eliminates an unnecessary call.
> > >
> > > The issue is:
> > > per-queue configuration, for example: port 0 rxq 0 rx_offload
> > > jumbo_frame off triggers the per-device configuration change: the
> > > RSS key is reconfigured and changes after rte_eth_dev_configure() is
> > > called on ICE PMD driver, that cause a test case failure.
> > >
> >
> > Hmmm. I agree on the following. It doesn't need dev_configure. That's
> > why I give you ack.
> >
> > But your issue. Shouldn't you fix the driver? The vsi->rss_key[] just
> > updated itself as a random value when dev_conf doesn't contain one.
> > But your case is dev_conf doesn't contain new rss key, but
> > vsi->rss_key is not null. In this case, it should keep the same not get a new
> random key.
> >
> In current implementation, dev->data->dev_conf.rx_adv_conf.rss_conf in PMD
> does not hold the rss_key value if user does not set it in the input parameter of
> rte_eth_dev_configure().
> 
> Even if PMD generate one automatically, it will not be saved in dev->data-
> >dev_conf.rx_adv_conf.rss_conf.
> 
> When user want to get rss_key, it will be retrieved on the fly from hardware, but
> not from any variable in PMD.
> 
> So PMD (ice and i40e) think only rss_key  which is set by user via
> rte_eth_dev_configure() can be reused when port is reconfigured.
> 
> If it is not present, PMD will generate one by itself anyway even if it is present in
> vsi->rss_key.
> 
> I don’t think this behavior is wrong, so did not fix ice PMD.

If you want to recover the rss key. The driver should store the key in vsi->rss_key also in ice_rss_hash_update(). Then when user not config rss_key in rss_conf and vs->rss_key is not 0, you should set the original value not a random value to hw.

Otherwise, you are assuming the behavior rss to different queues is right since user want random key. It's not an issue.

Any scenario, it's not testpmd issue. Please withdraw your patch.

> 
> > > There is an unnecessary call in original implementation because both
> > > cmd_config_per_queue_rx_offload_parsed() and
> > > cmd_config_per_queue_tx_offload_parsed()
> > > does not update the "port->dev_conf" which hold the port
> > > configuration, therefore there is no need to call rte_eth_dev_configure().


  reply	other threads:[~2021-04-12  2:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01  8:28 dapengx.yu
2021-04-07  2:30 ` Li, Xiaoyun
2021-04-08 15:41 ` Ferruh Yigit
2021-04-09  5:25   ` Yu, DapengX
2021-04-09  7:50     ` Li, Xiaoyun
2021-04-09 10:29       ` Yu, DapengX
2021-04-12  2:21         ` Li, Xiaoyun [this message]
2021-04-12 12:46           ` Ferruh Yigit
2021-04-15  6:04             ` Yu, DapengX
2021-04-19 15:46               ` Ferruh Yigit
2021-04-20  8:12                 ` Yu, DapengX
2021-04-20  8:22                   ` Ferruh Yigit
2021-04-20  8:40                     ` Yu, DapengX

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=DM4PR11MB553442F61FCAA7230B234FEB99709@DM4PR11MB5534.namprd11.prod.outlook.com \
    --to=xiaoyun.li@intel.com \
    --cc=dapengx.yu@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=qi.z.zhang@intel.com \
    --cc=stable@dpdk.org \
    /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

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/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 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


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