From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 76F43A034E; Wed, 27 May 2020 14:07:31 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B662B1D739; Wed, 27 May 2020 14:07:30 +0200 (CEST) Received: from mail-io1-f66.google.com (mail-io1-f66.google.com [209.85.166.66]) by dpdk.org (Postfix) with ESMTP id BA81F1D733 for <dev@dpdk.org>; Wed, 27 May 2020 14:07:29 +0200 (CEST) Received: by mail-io1-f66.google.com with SMTP id d5so16099468ios.9 for <dev@dpdk.org>; Wed, 27 May 2020 05:07:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=B0WBx+TCXcAjEeWzDkav5aMf5tZp5RZBWGU6X6C+dOE=; b=eAxPw4DWt+pO5j3mrWD+CZUs5gp95yHejnpZJ6tcSmkkLxuTzYdoof8BW+AsTmBSZw hTOHLQF/yjt6E3WWJ/nsqkIRLdtd6T2qNuw3uNcfA3Ju7qtIH/nHE/Ce9MPPQYzCY+PX L/fcjnE0tBqvpp7YsLfVmRzRqr5nqrFX+fvpDfUWc3A7OaoycPw1EKXS5nhV3xz9+JwC cByuD+0wynaN8xqv9arrbY6yBTWXsxngtGs7ASwDRe0AHEqVLAvQ9luASyDW3J5nLrpT si9jREPhZcsltwKW/zTHS3J9TjMNHOqMXR/v+T0//mb/lAhMz7xnyPxEmRVbTSCltRoy wcjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=B0WBx+TCXcAjEeWzDkav5aMf5tZp5RZBWGU6X6C+dOE=; b=BIm56+WqE4wGUkh1P0jAoqzSvanC2cqPpXhGTo0sz7QeRaw9QZ3x4MZeGaTaOPkrMy CgCDo0hRLrw16EcDFMD6nl95Pk8VY+e8cBHQVrZTNQDeWg2++jM2LeTs0C04MdHfUsQi oJtqadC9hdIkU7aXKAu+ufcN9SOc0PXu6hMFpFBguNrOpmzpfAjX/DiEhoAQ/KTubnP+ x8DkksHD1vMI3OkGlBZKvk6Gvilffx74NiHR28WV1kWqM35h2aoJZZJ8a6UpPKIK33Y9 Vr150f4ctye0/I6g6VJv81FScf6pgV7wSrqJGy6gYgLHYgGU5I03epNNr8ziMkxAFPhV BYGA== X-Gm-Message-State: AOAM531BU3MAZIrPf5rQycXOcjPXXhrBmOTInwK+Et+v8fJ/2Wyw1pyc bf8mFp3nu2h6EyrHmkoSidFt5WNNWKLECh6StpI= X-Google-Smtp-Source: ABdhPJwip8fz5HKd7nPiXQUWKgRMiBUeV6jnJ4FCg1snUkwjg4oC3mD9qcabUIjtrX+tIPzPwZUSz6y2mAa+V/WCfSo= X-Received: by 2002:a6b:f40b:: with SMTP id i11mr20718872iog.59.1590581248729; Wed, 27 May 2020 05:07:28 -0700 (PDT) MIME-Version: 1.0 References: <20200525212415.3173817-1-thomas@monjalon.net> <2617275.OaBZfIjOFF@thomas> <CALBAE1OfxPBqikviDn_q_43J=H8MwoRB0Yyq6fNne8qLgc_91g@mail.gmail.com> <3071819.TdKep6GQZa@thomas> In-Reply-To: <3071819.TdKep6GQZa@thomas> From: Jerin Jacob <jerinjacobk@gmail.com> Date: Wed, 27 May 2020 17:37:13 +0530 Message-ID: <CALBAE1N4YvrTzqqNOzk2w0dJxMD3XEserjEP_ocb4_0fZoWPew@mail.gmail.com> To: Thomas Monjalon <thomas@monjalon.net> Cc: Jerin Jacob <jerinj@marvell.com>, Olivier Matz <olivier.matz@6wind.com>, dpdk-dev <dev@dpdk.org>, David Marchand <david.marchand@redhat.com>, Nithin Dabilpuram <ndabilpuram@marvell.com>, Krzysztof Kanas <kkanas@marvell.com>, Andrew Rybchenko <arybchenko@solarflare.com>, Ferruh Yigit <ferruh.yigit@intel.com>, "Richardson, Bruce" <bruce.richardson@intel.com>, John McNamara <john.mcnamara@intel.com>, Marko Kovacevic <marko.kovacevic@intel.com> Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] mbuf: document rule for new fields and flags X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> On Wed, May 27, 2020 at 5:26 PM Thomas Monjalon <thomas@monjalon.net> wrote: > > 27/05/2020 13:43, Jerin Jacob: > > On Wed, May 27, 2020 at 3:21 PM Thomas Monjalon <thomas@monjalon.net> wrote: > > > 27/05/2020 09:31, Jerin Jacob: > > > > On Wed, May 27, 2020 at 12:39 PM Olivier Matz <olivier.matz@6wind.com> wrote: > > > > > 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.matz@6wind.com> wrote: > > > > > > > On Tue, May 26, 2020 at 01:09:37PM +0530, Jerin Jacob wrote: > > > > > > > > On Tue, May 26, 2020 at 2:54 AM Thomas Monjalon <thomas@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 <thomas@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 > > > > > > > > > +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. > > > > > > > > I don't think, we need to anticipate all the criteria in the documentation. > > > > At least ONE should be given as an example of an exception. > > > > > > I think it is too early to be more specific in the guidelines. > > > Do we agree this patch is a first good step in the documentation? > > > > IMO, there is a gap. The subject says the rule, but no rule here. > > We are just giving some guideline and following info in the patch > > given by Olivier is not > > expressed if we read the patch. > > > > " > > I don't think we should be too strict: the door remains open for > > technical debate and exceptions. > > " > > Indeed, the headline should be > mbuf: add guideline for new fields and flags > > > > > We can extend the guidelines a bit later after having some > > > discussions on specific cases, and probably in the techboard too. > > > > > > > > > > I would say, > > > > a) If a feature takes only one bit and its part of normative API spec > > > > and it used in fastpath we should consider the static scheme. > > > > b) Adding an exception to the existing list needs approval at least > > > > from three maintainers > > > > > > > > For me, it is a very legitimate case to have support for > > > > http://patches.dpdk.org/patch/68733/ to the static scheme > > > > as it takes 1 bit for a feature and it is part of the normative spec. > > > > I don't get in explanation in the ml, why > > > > we can not make it as the static scheme for this case. > > > > > > We can continue discussion about this specific case in the right thread. > > > > Yes. The email thread[1] provided all the details. We have optimized > > to one bit for this feature. > > We are expecting Olivier to comment on the new proposal. > > [1] > > http://patches.dpdk.org/patch/68733/ > > > > > Note: I don't have a definitive opinion on it, I need to read it carefully. > > > > Please read it carefully and please provide any technical opinions if > > you have any. > > > > > > > > My worry is, if we are keeping as open-ended means, we are giving room > > > > for the disparity among the vendors/feature > > > > as I dont think, There is use case where dynamic scheme can not be > > > > used as a replacement > > > > for static other than performance hit.(Could think of any use case?) > > > > So open ended boils down to preference to specific feature/vendor. I > > > > think,that path should be avoided. > > > > > > Of course all rules and decisions have to be fair. > > > It's not even a question. > > > > Yes. But I dont think, this patch is not enforcing anything such, > > instead it makes it as an open-ended > > for more confusion. IMO, if it not black and white then better to not > > express the rule. > > I disagree about "more confusion". My confusion will clear up if 1) s/rule/guide line/ change across the patch 2) Explicit mention of the following or similar sentence. it is a guideline and exception is allowed on a case by case using technical debate. > I think the value of this patch is to improve awareness > about the need for using dynamic fields and flags. > > Let's ask other opinions about the added value of this patch. > >