On Tue, May 26, 2020 at 09:59:45PM +0530, Jerin Jacob wrote: > On Tue, May 26, 2020 at 9:36 PM Olivier Matz <olivier.m...@6wind.com> wrote: > > > > Hi, > > > > On Tue, May 26, 2020 at 01:09:37PM +0530, Jerin Jacob wrote: > > > On Tue, May 26, 2020 at 2:54 AM Thomas Monjalon <tho...@monjalon.net> > > > wrote: > > > > > > > > Since dynamic fields and flags were added in 19.11, > > > > the idea was to use them for new features, not only PMD-specific. > > > > > > > > The rule is made more explicit in doxygen, in the mbuf guide, > > > > and in the contribution design guidelines. > > > > > > > > For more information about the original design, see the presentation > > > > https://www.dpdk.org/wp-content/uploads/sites/35/2019/10/DynamicMbuf.pdf > > > > > > > > Signed-off-by: Thomas Monjalon <tho...@monjalon.net> > > > > --- > > > > doc/guides/contributing/design.rst | 13 +++++++++++++ > > > > doc/guides/prog_guide/mbuf_lib.rst | 23 +++++++++++++++++++++++ > > > > lib/librte_mbuf/rte_mbuf_core.h | 2 ++ > > > > 3 files changed, 38 insertions(+) > > > > > > > > diff --git a/doc/guides/contributing/design.rst > > > > b/doc/guides/contributing/design.rst > > > > index d3dd694b65..508115d5bd 100644 > > > > --- a/doc/guides/contributing/design.rst > > > > +++ b/doc/guides/contributing/design.rst > > > > @@ -57,6 +57,19 @@ The following config options can be used: > > > > * ``CONFIG_RTE_EXEC_ENV`` is a string that contains the name of the > > > > executive environment. > > > > * ``CONFIG_RTE_EXEC_ENV_FREEBSD`` or ``CONFIG_RTE_EXEC_ENV_LINUX`` are > > > > defined only if we are building for this execution environment. > > > > > > > > +Mbuf features > > > > +------------- > > > > + > > > > +The ``rte_mbuf`` structure must be kept small (128 bytes). > > > > + > > > > +In order to add new features without wasting buffer space for unused > > > > features, > > > > +some fields and flags can be registered dynamically in a shared area. > > > > > > I think, instead of "can", it should be "must" > > > > > > > +The "dynamic" mbuf area is the default choice for the new features. > > > > In my opinion, Thomas' proposal is correct, with the next sentence > > saying it is the default choice for new features. > > > > Giving guidelines is a good thing (thanks Thomas for documenting it), > > but I don't think we should be too strict: the door remains open for > > technical debate and exceptions. > > If you are open for the exception then it must be mention in what case > the exception is allowed and what are the criteria of the exception? > > For example, Why did n't we choose the following patch as expectation > http://patches.dpdk.org/patch/68733/ even if only one bit used. > > If we are not not defining the criteria, IMO, This patch serve no purpose than > the existing situation. > > Do you think, any case where the dynamic scheme can not be used as a > replacement > for static other than performance hit.
I don't think it is possible to anticipate all criteria in the documentation. With Thomas' proposal, it gives a direction is and a global view, but it must not completly replace reflection and discussion. Olivier