Hi Olivier, > -----Original Message----- > From: Olivier Matz <olivier.m...@6wind.com> > Sent: Monday, October 26, 2020 18:22 > To: Wang, Haiyue <haiyue.w...@intel.com> > Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yi...@intel.com>; Guo, Jia > <jia....@intel.com>; Zhang, Qi Z > <qi.z.zh...@intel.com>; Chen, Zhaoyan <zhaoyan.c...@intel.com>; Yang, Qiming > <qiming.y...@intel.com>; > Ray Kinsella <m...@ashroe.eu>; Neil Horman <nhor...@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.w...@intel.com> > > --- > > 2.29.0 > >
