DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wang, Haiyue" <haiyue.wang@intel.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"Guo, Jia" <jia.guo@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"Chen, Zhaoyan" <zhaoyan.chen@intel.com>,
	"Yang, Qiming" <qiming.yang@intel.com>,
	Ray Kinsella <mdr@ashroe.eu>, Neil Horman <nhorman@tuxdriver.com>
Subject: Re: [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction
Date: Mon, 26 Oct 2020 11:29:37 +0000
Message-ID: <BN8PR11MB379525B3106D488CBBD2C69CF7190@BN8PR11MB3795.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20201026102215.GK1898@platinum>

Hi Olivier,

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Monday, October 26, 2020 18:22
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Guo, Jia <jia.guo@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Chen, Zhaoyan <zhaoyan.chen@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
> Subject: Re: [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction
> 
> Hi Haiyue,
> 
> On Sun, Oct 25, 2020 at 03:13:52PM +0800, Haiyue Wang wrote:
> > Current dynamic mbuf design is that the driver will register the needed
> > field and flags at the device probing time, this will make iavf PMD use
> > different names to register the dynamic mbuf field and flags, but both
> > of them use the exactly same protocol extraction metadata.
> >
> > This will run out of the limited dynamic mbuf resource, meanwhile, the
> > application has to handle the dynamic mbuf separately.
> >
> > For making things simple and consistent, refactor dynamic mbuf in data
> > extraction handling: the PMD just lookups the same name at the queue
> > setup time after the application registers it.
> >
> > In other words, make the dynamic mbuf string name as API, not the data
> > object which is defined in each PMD.
> 
> In case the dynamic mbuf field is shared by several PMDs, it seems to
> be indeed a better solution.
> 
> Currently, the "union rte_pmd_proto_xtr_metadata" is still defined in
> rte_pmd_ice.h. Will it be the same for iavf, and will it be factorized
> somewhere? However I don't know where could be a good place.
> 
> There is already lib/librte_mbuf/rte_mbuf_dyn.h which is the place to
> centralize the name and description of dynamic fields/flags used in
> libraries. But I think neither structure definitions nor PMD-specific
> flags should go there too. I'd prefer to have them in
> drivers/net/<common-something>, but I'm not sure it is possible.

May be new 'lib/librte_mbuf/rte_mbuf_dyn_pmd.h' for all PMDs specific ?
So that the application knows exactly how many *dynamic* things. Also,
a new API to query the dynamic information + dev_ops may be introduced
in next release cycle, then 'rte_pmd_mlx5_get_dyn_flag_names' can be
removed. And the application will be clean.

Currently, we use " #define __INTEL_RX_FLEX_DESC_METADATA__ " to fix the
duplicated definition, but the application have to include the two header
files like "rte_pmd_ice.h" / "rte_pmd_iavf.h"

> 
> Also, it is difficult from the patch to see the impact it has on an
> application that was using these metadata. Should we have an example
> of use?
> 

Thanks your link in previous mail:
http://inbox.dpdk.org/dev/20191030165626.w3flq5wdpitpsv2v@platinum/

Original patch uses: Solution 1, provide static inline helpers to access to the
dyn fields/flags

Now now: Solution 2, without global variable export and helpers: the application
calls rte_mbuf_dynfield_register(&rte_pmd_ice_proto_xtr_metadata_param) to get
the offset, and store it privately.

https://patchwork.dpdk.org/patch/82165/
In v3 patch, I kept the metadata format, and rename it to be more generic:
'union rte_pmd_proto_xtr_metadata', but no dump function as the original design.

> Thanks,
> Olivier
> 
> 
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> > 2.29.0
> >

  reply	other threads:[~2020-10-26 11:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-25  7:13 Haiyue Wang
2020-10-26  6:50 ` [dpdk-dev] [PATCH v2] net/ice: refactor the protocol extraction design Haiyue Wang
2020-10-26  6:55 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
2020-10-27  1:35   ` Zhang, Qi Z
2020-10-26 10:22 ` [dpdk-dev] [PATCH v1] net/ice: refactor dynamic mbuf in data extraction Olivier Matz
2020-10-26 11:29   ` Wang, Haiyue [this message]
2020-10-27  1:23 ` [dpdk-dev] [PATCH v4] net/ice: rename the dynamic mbuf name Haiyue Wang
2020-10-27  1:42   ` Zhang, Qi Z

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=BN8PR11MB379525B3106D488CBBD2C69CF7190@BN8PR11MB3795.namprd11.prod.outlook.com \
    --to=haiyue.wang@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jia.guo@intel.com \
    --cc=mdr@ashroe.eu \
    --cc=nhorman@tuxdriver.com \
    --cc=olivier.matz@6wind.com \
    --cc=qi.z.zhang@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=zhaoyan.chen@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

This inbox may be cloned and mirrored by anyone:

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

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


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