From: "Wang, Haiyue" <haiyue.wang@intel.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags
Date: Thu, 11 Jul 2019 08:04:00 +0000 [thread overview]
Message-ID: <E3B9F2FDCB65864C82CD632F23D8AB8773399486@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20190711072619.fldr3dz7qblyvej5@glumotte.dev.6wind.com>
> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Thursday, July 11, 2019 15:26
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags
>
> Hi,
>
> On Wed, Jul 10, 2019 at 05:14:33PM +0000, Wang, Haiyue wrote:
> > Hi,
> >
> > Sounds cool, just have some questions inline.
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > > Sent: Wednesday, July 10, 2019 17:29
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [RFC] mbuf: support dynamic fields and flags
> > >
> > > Many features require to store data inside the mbuf. As the room in mbuf
> > > structure is limited, it is not possible to have a field for each
> > > feature. Also, changing fields in the mbuf structure can break the API
> > > or ABI.
> > >
> > > This commit addresses these issues, by enabling the dynamic registration
> > > of fields or flags:
> > >
> > > - a dynamic field is a named area in the rte_mbuf structure, with a
> > > given size (>= 1 byte) and alignment constraint.
> > > - a dynamic flag is a named bit in the rte_mbuf structure.
> > >
> > > The typical use case is a PMD that registers space for an offload
> > > feature, when the application requests to enable this feature. As
> > > the space in mbuf is limited, the space should only be reserved if it
> > > is going to be used (i.e when the application explicitly asks for it).
> > >
> > > The registration can be done at any moment, but it is not possible
> > > to unregister fields or flags for now.
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>
> (...)
>
> > > +/**
> > > + * @file
> > > + * RTE Mbuf dynamic fields and flags
> > > + *
> > > + * Many features require to store data inside the mbuf. As the room in
> > > + * mbuf structure is limited, it is not possible to have a field for
> > > + * each feature. Also, changing fields in the mbuf structure can break
> > > + * the API or ABI.
> > > + *
> > > + * This module addresses this issue, by enabling the dynamic
> > > + * registration of fields or flags:
> > > + *
> > > + * - a dynamic field is a named area in the rte_mbuf structure, with a
> > > + * given size (>= 1 byte) and alignment constraint.
> > > + * - a dynamic flag is a named bit in the rte_mbuf structure.
> > > + *
> > > + * The typical use case is a PMD that registers space for an offload
> > > + * feature, when the application requests to enable this feature. As
> > > + * the space in mbuf is limited, the space should only be reserved if it
> > > + * is going to be used (i.e when the application explicitly asks for it).
> > > + *
> > > + * The registration can be done at any moment, but it is not possible
> > > + * to unregister fields or flags for now.
> > > + *
> > > + * Example of use:
> > > + *
> > > + * - RTE_MBUF_DYN_<feature>_(ID|SIZE|ALIGN) are defined in this file
> >
> > Does it means that all PMDs define their own 'RTE_MBUF_DYN_<feature>_(ID|SIZE|ALIGN)'
> > here ? In other words, each PMD can expose its private DYN_<feature> here for public
> > using ?
>
> For generic fields, I think they should be declared in this file. For
> instance, if we decide to replace the current m->timestamp field by a
> dynamic field, we should add like this:
>
> #define RTE_MBUF_DYN_TIMESTAMP_ID "rte_timestamp"
> #define RTE_MBUF_DYN_TIMESTAMP_SIZE sizeof(uint64_t)
> #define RTE_MBUF_DYN_TIMESTAMP_ALIGN __alignof__(uint64_t)
>
> If the feature is PMD-specific, the defines could be exposed in a
> PMD header.
>
Now, understand the comments a little : ... must not define identifers prefixed with "rte_",
which are reserved for standard features. Seems have big plan ?
> > How about adding another eth_dev_ops API definitions to show the PMD's supporting feature
> > names, sizes, align in run time for testpmd ? And also another eth_dev_ops API for showing
> > the data saved in rte_mbuf by 'dump_pkt_burst' ? Adding a new command for testpmd to set
> > the dynamic feature may be good for PMD test.
> >
> > > + * - If the application asks for the feature, the PMD use
> >
> > How does the application ask for the feature ? By ' rte_mbuf_dynfield_register()' ?
>
> No change in this area. If we take again the timestamp example, the
> feature is asked by the application through the ethdev layer by passing
> DEV_RX_OFFLOAD_TIMESTAMP to port or queue configuration.
>
> >
> > > + * rte_mbuf_dynfield_register() to get the dynamic offset and stores
> > > + * in a global variable.
> >
> > In case, the PMD calls 'rte_mbuf_dynfield_register()' for 'dyn_feature' firstly, this
> > means that PMD requests the dynamic feature itself if I understand correctly. Should
> > PMD calls 'rte_mbuf_dynfield_lookup' for 'dyn_feature' to query the name exists, the
> > size and align are right as expected ? If exists, but size and align are not right, may
> > be for PMD change its definition, then PMD can give a warning or error message. If name
> > exists, both size and align are expected, then PMD think that the application request
> > the right dynamic features.
>
> The PMD should only call rte_mbuf_dynfield_register() if the application
> requests the feature (through ethdev, or through another mean if it's a
> PMD-specific feature). The goal is to only reserve the area in the mbuf
> for features that are actually needed.
>
> Hope this is clearer now. I think I need to enhance the documentation in
> next version ;)
>
Clearer now, more test code also will be better for fully understanding, thanks! :)
> Thanks for the feedback.
next prev parent reply other threads:[~2019-07-11 8:04 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-10 9:29 Olivier Matz
2019-07-10 17:14 ` Wang, Haiyue
2019-07-11 7:26 ` Olivier Matz
2019-07-11 8:04 ` Wang, Haiyue [this message]
2019-07-11 8:20 ` Olivier Matz
2019-07-11 8:34 ` Wang, Haiyue
2019-07-11 15:31 ` Stephen Hemminger
2019-07-12 9:18 ` Olivier Matz
2019-07-10 17:49 ` Stephen Hemminger
2019-07-10 18:12 ` Wiles, Keith
2019-07-11 7:53 ` Olivier Matz
2019-07-11 14:37 ` Wiles, Keith
2019-07-12 9:06 ` Olivier Matz
2019-07-11 7:36 ` Olivier Matz
2019-07-12 12:23 ` Jerin Jacob Kollanukkaran
2019-07-16 9:39 ` Olivier Matz
2019-07-16 14:43 ` Stephen Hemminger
2019-07-11 9:24 ` Thomas Monjalon
2019-07-12 14:54 ` Andrew Rybchenko
2019-07-16 9:49 ` Olivier Matz
2019-07-16 11:31 ` [dpdk-dev] ***Spam*** " Andrew Rybchenko
2019-09-18 16:54 ` [dpdk-dev] [PATCH] " Olivier Matz
2019-09-21 4:54 ` Wang, Haiyue
2019-09-23 8:31 ` Olivier Matz
2019-09-23 11:01 ` Wang, Haiyue
2019-09-21 8:28 ` Wiles, Keith
2019-09-23 8:56 ` Morten Brørup
2019-09-23 9:41 ` Olivier Matz
2019-09-23 9:13 ` Olivier Matz
2019-09-23 15:14 ` Wiles, Keith
2019-09-23 16:16 ` Olivier Matz
2019-09-23 17:14 ` Wiles, Keith
2019-09-23 16:09 ` Wiles, Keith
2019-10-01 10:49 ` Ananyev, Konstantin
2019-10-17 7:54 ` Olivier Matz
2019-10-17 11:58 ` Ananyev, Konstantin
2019-10-17 12:58 ` Olivier Matz
2019-10-17 14:42 ` [dpdk-dev] [PATCH v2] " Olivier Matz
2019-10-18 2:47 ` Wang, Haiyue
2019-10-18 7:53 ` Olivier Matz
2019-10-18 8:28 ` Wang, Haiyue
2019-10-18 9:47 ` Olivier Matz
2019-10-18 11:24 ` Wang, Haiyue
2019-10-22 22:51 ` Ananyev, Konstantin
2019-10-23 3:16 ` Wang, Haiyue
2019-10-23 10:21 ` Olivier Matz
2019-10-23 15:00 ` Stephen Hemminger
2019-10-23 15:12 ` Wang, Haiyue
2019-10-23 10:19 ` Olivier Matz
2019-10-23 11:45 ` Olivier Matz
2019-10-23 11:49 ` Ananyev, Konstantin
2019-10-23 12:00 ` Shahaf Shuler
2019-10-23 13:33 ` Olivier Matz
2019-10-24 4:54 ` Shahaf Shuler
2019-10-24 7:07 ` Olivier Matz
2019-10-24 7:38 ` Slava Ovsiienko
2019-10-24 7:56 ` Olivier Matz
2019-10-24 8:13 ` [dpdk-dev] [PATCH v3] " Olivier Matz
2019-10-24 15:30 ` Stephen Hemminger
2019-10-24 15:44 ` Thomas Monjalon
2019-10-24 17:07 ` Stephen Hemminger
2019-10-24 16:40 ` Thomas Monjalon
2019-10-26 12:39 ` [dpdk-dev] [PATCH v4] " Olivier Matz
2019-10-26 17:04 ` Thomas Monjalon
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=E3B9F2FDCB65864C82CD632F23D8AB8773399486@SHSMSX101.ccr.corp.intel.com \
--to=haiyue.wang@intel.com \
--cc=dev@dpdk.org \
--cc=olivier.matz@6wind.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).