DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Wiles, Keith" <keith.wiles@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3] mempool: reduce rte_mempool structure size
Date: Fri, 12 Feb 2016 16:38:56 +0100	[thread overview]
Message-ID: <10558998.3znIRhOpQL@xps13> (raw)
In-Reply-To: <09D5A01F-7205-49E8-9A27-95161235963E@intel.com>

2016-02-12 15:07, Wiles, Keith:
> >On 02/12/2016 03:57 PM, Thomas Monjalon wrote:
> >> 2016-02-12 13:23, Panu Matilainen:
> >>> On 02/10/2016 11:18 PM, Keith Wiles wrote:
> >>>>    static inline void *rte_mempool_get_priv(struct rte_mempool *mp)
> >>>>    {
> >>>> +#ifdef RTE_NEXT_ABI
> >>>> +	return (char *)mp +
> >>>> +		MEMPOOL_HEADER_SIZE(mp, mp->pg_num, mp->cache_size);
> >>>> +#else
> >>>>    	return (char *)mp + MEMPOOL_HEADER_SIZE(mp, mp->pg_num);
> >>>> +#endif /* RTE_NEXT_ABI */
> >>>>    }
> >>>
> >>> This is not RTE_NEXT_ABI material IMO, the added ifdef clutter is just
> >>> too much.
> >>
> >> The changes are restricted to the mempool files.
> >> I think it is not so much. However I wonder how much the feature is important
> >> to justify the use of NEXT_ABI.
> >
> >Well yes, to be precise: for the benefit of this patch, the ifdef 
> >clutter seems too much.
> >
> >Its not as if every change is expected to go through a NEXT_ABI phase, 
> >based on http://dpdk.org/ml/archives/dev/2016-February/032866.html there 
> >might be some confusion regarding that.
> 
> I think the NEXT_ABI is reasonable in this case as it does change a structure everyone uses and the ifdef clutter is caused by having to remove old ifdefs, which is a good thing for DPDK. The NEXT_ABI ifdefs only exist for one release and then they will disappear, which I think is more then reasonable.

OK, I'm going to sum it up with new words and let the conclusion comes
from Keith, Panu and Olivier.

We agreed to allow ABI breaking if a notification was done in the
previous release.
Keith has sent a notification for 16.04 so the "official" ABI will be
changed in 16.07.
It is also encouraged to show how the ABI will be broken when sending
a notification. It allows to give an informed opinion before ack'ing.
The code snippet will also be useful to app developpers when preparing
a future upgrade.
Keith has sent the whole code change.
This code change may be submitted in the current release without waiting
the deprecation time if gated in the NEXT_ABI ifdefs.
It allows to provide the feature to app developpers who don't care about
versioning. But the price is a more complicated code to read and manage.

To make it short, the rules to use NEXT_ABI are not strict and may change.
So now you have to decide if this change can be integrated in 16.04
as NEXT_ABI.

  reply	other threads:[~2016-02-12 15:40 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-02 23:02 [dpdk-dev] [PATCH] mempool: Reduce " Keith Wiles
2016-02-03 17:11 ` Ananyev, Konstantin
2016-02-08 11:02 ` Olivier MATZ
2016-02-08 15:57   ` Wiles, Keith
2016-02-09 17:30 ` [dpdk-dev] [PATCH v2] mempool: reduce " Keith Wiles
2016-02-10 16:59   ` Olivier MATZ
2016-02-10 17:22     ` Wiles, Keith
2016-02-10 18:35     ` Wiles, Keith
2016-02-10 20:06       ` Olivier MATZ
2016-02-10 21:18   ` [dpdk-dev] [PATCH v3] " Keith Wiles
2016-02-12 11:23     ` Panu Matilainen
2016-02-12 13:57       ` Thomas Monjalon
2016-02-12 14:19         ` Panu Matilainen
2016-02-12 15:07           ` Wiles, Keith
2016-02-12 15:38             ` Thomas Monjalon [this message]
2016-02-12 15:50               ` Olivier MATZ
2016-02-12 15:58                 ` Wiles, Keith
2016-02-15  9:58                 ` Hunt, David
2016-02-15 10:15                   ` Olivier MATZ
2016-02-15 10:21                     ` Hunt, David
2016-02-15 12:31                       ` Olivier MATZ
2016-02-12 15:54               ` Wiles, Keith
2016-02-12 18:36   ` [dpdk-dev] [PATCH v4] " Keith Wiles
2016-02-15  9:20     ` Olivier MATZ
2016-04-14  9:42     ` [dpdk-dev] [PATCH v5] " Olivier Matz
2016-04-14 13:28       ` Wiles, Keith
2016-04-14 13:43         ` Olivier MATZ
2016-04-14 13:53       ` Wiles, Keith
2016-05-17  5:31       ` 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=10558998.3znIRhOpQL@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=keith.wiles@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
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).