All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas F Herbert <therbert@redhat.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>,
	Neil Horman <nhorman@tuxdriver.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-announce] important design choices - statistics - ABI
Date: Fri, 19 Jun 2015 12:13:49 -0400	[thread overview]
Message-ID: <55843FBD.1050303@redhat.com> (raw)
In-Reply-To: <1784476.c2eg9hZKIA@xps13>



On 6/19/15 9:16 AM, Thomas Monjalon wrote:
> 2015-06-19 09:02, Neil Horman:
>> On Fri, Jun 19, 2015 at 02:32:33PM +0200, Thomas Monjalon wrote:
>>> 2015-06-19 06:26, Neil Horman:
>>>> On Thu, Jun 18, 2015 at 04:55:45PM +0000, O'Driscoll, Tim wrote:
>>>>> For the 2.1 release, I think we should agree to make patches that change
>>>>> the ABI controllable via a compile-time option. I like Olivier's proposal
>>>>> on using a single option (CONFIG_RTE_NEXT_ABI) to control all of these
>>>>> changes instead of a separate option per patch set (see
>>>>> http://dpdk.org/ml/archives/dev/2015-June/019147.html), so I think we
>>>>> should rework the affected patch sets to use that approach for 2.1.
>>>>
>>>> This is a bad idea.  Making ABI dependent on compile time options isn't a
>>>> maintainable solution.  It breaks the notion of how LIBABIVER is supposed to
>>>> work (that is to say you make it impossible to really tell what ABI version you
>>>> are building).
>>>
>>> The idea was to make LIBABIVER increment dependent of CONFIG_RTE_NEXT_ABI.
>>> So one ABI version number refers always to the same ABI.
>>>
>>>> If you have two compile time options that modify the ABI, you
>>>> have to burn through 4 possible LIBABIVER version values to accomodate all
>>>> possible combinations, and then you need to remember that when you make them
>>>> statically applicable.
>>>
>>> The idea is to have only 1 compile-time option: CONFIG_RTE_NEXT_ABI.
>>>
>>> Your intent when introducing ABI policy was to allow smooth porting of
>>> applications from a DPDK version to another. Right?
>>> The adopted solution was to provide backward compatibility during 1 release.
>>> But there are cases where it's not possible. So the policy was to notice
>>> the future change and wait one release cycle to break the ABI (failing
>>> compatibility goals).
>>> The compile-time option may provide an alternative DPDK packaging when the
>>> ABI backward compatibility cannot be provided (case of mbuf changes).
>>> In such case, it's still possible to upgrade DPDK by providing 2 versions of
>>> DPDK libs. So the existing apps continue to link with the previous ABI and
>>> have the possibility of migrating to the new one.
>>> Another advantage of this approach is that we don't have to wait 1 release
>>> to integrate the changes.
>>> The last advantage is to benefit early of these changes with static libraries.
>>
>> Hm, ok, thats a bit more reasonable, but it still seems shaky to me.
>> Implementing an ABI preview option like this implies the notion that, after a
>> release, you have to remove all the ifdefs that you inserted to create the new
>> ABI.  That seems like an easy task, but it becomes a pain when the ABI delta is
>> large, and is predicated on the centralization of work effort (that is to say
>> you need to identify someone to submit the 'remove the NEXT_ABI config ifdefs
>> from the build' patch every release.
>
> It won't be so huge if we reserve the NEXT_ABI solution to changes which cannot
> have easy backward compatibility with the compat macros you introduced.
> I feel I can do the job of removing the ifdefs NEXT_ABI after each release.
> At the same time, the deprecated API, using the compat macros, will be removed.
>
>> What might be better would be a dpdk-next branch (or even a dpdk-next tree, of
>> the sort that Thomas Herbert proposed a few weeks ago).
>
> This tree was created after Thomas' request:
> 	http://dpdk.org/browse/next/dpdk-next/

Thomas, I am sorry if I went quiet for awhile but I was on personal 
travel with inconsistent access so I almost missed most of this 
discussion about ABI changes.

My understanding of the purpose of the dpdk-next tree is to validate 
patches by applying and compiling against a "pull" from the main dpdk 
tree. I think a good way to handle ABI change while effectively using 
the dpdk-next might be to do as follows:

Create a specific branch for the new ABI such as 2.X in the main dpdk 
tree. Once that 2.X branch is created, dpdk-next would mirror the 2.X 
branch along with master.

Since, dpdk-next would also have the 2.X branch that is in the main dpdk 
tree, submitted patches could be applied to either the main branch or 
the new-ABI 2.X branch. Providing that patch submitters make it clear 
whether a submitted patch is for the new ABI or the old ABI, dpdk-next 
could continue to validate the patches for either the main branch or the 
new ABI 2.X branch.

>
>> Patches that aren't ABI stable can be put on the next-branch/tree in thier
>> final format.  You can delcare the branch unstable (thereby reserving your
>> right to rebase it).  People can use that to preview the next ABI version
>> (complete with the update LIBABIVER bump), and when you release dpdk-X,
>> the new ABI for dpdk-X+1 is achieved by simply merging.
>
> Having this tree living would be a nice improvement but it won't provide any
> stable (and enough validated) releases to rely on.
>

  parent reply	other threads:[~2015-06-19 16:13 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-16 23:29 [dpdk-announce] important design choices - statistics - ABI Thomas Monjalon
2015-06-17  4:36 ` Matthew Hall
2015-06-17  5:28   ` Stephen Hemminger
2015-06-17  8:23     ` Thomas Monjalon
2015-06-17  8:23     ` Marc Sune
2015-06-17 11:17   ` Bruce Richardson
2015-06-18 16:32     ` Dumitrescu, Cristian
2015-06-18 13:25   ` Dumitrescu, Cristian
2015-06-17  9:54 ` Morten Brørup
2015-06-18 13:00   ` Dumitrescu, Cristian
2015-06-17 10:35 ` Neil Horman
2015-06-17 11:06   ` Richardson, Bruce
2015-06-19 11:08     ` Mcnamara, John
2015-06-17 12:14   ` Panu Matilainen
2015-06-17 13:21     ` Vincent JARDIN
2015-06-18  8:36   ` Zhang, Helin
2015-06-18 16:55 ` O'Driscoll, Tim
2015-06-18 21:13   ` Vincent JARDIN
2015-06-19 10:26   ` Neil Horman
2015-06-19 12:32     ` Thomas Monjalon
2015-06-19 13:02       ` Neil Horman
2015-06-19 13:16         ` Thomas Monjalon
2015-06-19 15:27           ` Neil Horman
2015-06-19 15:51             ` Thomas Monjalon
2015-06-19 16:13           ` Thomas F Herbert [this message]
2015-06-19 17:02             ` Thomas Monjalon
2015-06-19 17:57               ` Thomas F Herbert

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=55843FBD.1050303@redhat.com \
    --to=therbert@redhat.com \
    --cc=dev@dpdk.org \
    --cc=nhorman@tuxdriver.com \
    --cc=thomas.monjalon@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.