From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f51.google.com (mail-wg0-f51.google.com [74.125.82.51]) by dpdk.org (Postfix) with ESMTP id 7FC5BC85C for ; Fri, 19 Jun 2015 17:52:10 +0200 (CEST) Received: by wgbhy7 with SMTP id hy7so93142557wgb.2 for ; Fri, 19 Jun 2015 08:52:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=IIjNnxQvt6VcbSpZIyiGDxqNyMQxexBCCbPD6gdMxxc=; b=DvylU71xNCqvX259MGiCiw2TgZ1+ykZ5gxcpaT+MmtxZGkmZ1+wcFP0m0N4CLvGXuq fpwbCl3sshUjOiBGT16cZuz0Q/WN9saNnuiZFRrOmOa/K6KIoY2GEzUGInK6orttbelQ EH/m1TO5tdxQ7k3k5FPMBOhwuii98HKgeuxkHYM/jUCY/ZphOEbUjg0qr/0QszAD0E6D QT9tFwIX60Rd6oCxGdavRjof1/oXKc/A6XKVTi0pHey0oN6MHEKYatH9LXJ7/8VeaazX DEL+18A2hguGRRKoKSV1XzWqaNvRIWF+4ZrNsgcFRn9ZOdcQ9pS0ipnXjF+sx1A3AWku 3+KQ== X-Gm-Message-State: ALoCoQkufyYib8eR6L6+0SyzA31BcR9PUtjUJW4KrDsqGtFVlIf2H8nnqULrtCGOVb44fpessLsD X-Received: by 10.194.71.105 with SMTP id t9mr26148043wju.128.1434729130342; Fri, 19 Jun 2015 08:52:10 -0700 (PDT) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id pg1sm17629067wjb.39.2015.06.19.08.52.08 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 19 Jun 2015 08:52:09 -0700 (PDT) From: Thomas Monjalon To: Neil Horman Date: Fri, 19 Jun 2015 17:51:07 +0200 Message-ID: <1699150.ygAkzfgmGl@xps13> Organization: 6WIND User-Agent: KMail/4.14.8 (Linux/4.0.4-2-ARCH; KDE/4.14.8; x86_64; ; ) In-Reply-To: <20150619152730.GB4619@hmsreliant.think-freely.org> References: <9092314.MoyqUJ5VU2@xps13> <1784476.c2eg9hZKIA@xps13> <20150619152730.GB4619@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [dpdk-announce] important design choices - statistics - ABI X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Jun 2015 15:52:10 -0000 2015-06-19 11:27, Neil Horman: > On Fri, Jun 19, 2015 at 03:16:53PM +0200, 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. > > > I think that is something you can't really predict, as its not an issue of how > stringent we are with its use, but rather a function of how much change > developers want in a given release. That is to say, if you only reserve it for > the most important/urgently needed changes, thats fine, but if you have a > release in which 50 developers want to make urgent and important changes that > breaks ABI, you still have quite a job on your hands to back out the config > changes. > > Not to mention the fact that backing those changes out is a manual process. > > > > 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/ > > > Awesome, Though I'm not sure thats entirely the right place either. IIRC that > location was intended to be an early integration site that took unreviewed > patches. I think this really calls for a branch from the mainline tree that > exclusively accepts reviewed ABI changing patches, that can then be merged after > the next release > > > > 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. > > > I'm not sure I follow you entirely here. If the goal is to find a place to > accept patches that are ABI altering ahead of the main release, why do you need > to provide stable/validated releases? Just base it off the HEAD of the git tree > during the DPDK release X merge window, any testing done in the base branch > should roughly apply, save for functional changes made by the ABI patches you > add in on the branch. OK, I didn't get you. So you are saying that the changes for the next release may be prepared in a branch. Yes, it's possible. But before stating on practical method to execute the policy, we need to agree on the policy. This is my proposal: - The ABI policy must be better explained: what happens to LIBABIVER and .map when adding a field, adding or removing a function. - The file doc/guides/rel_notes/abi.rst must contain the ABI notices but the policy must be moved to doc/guides/guidelines/compat.rst - The case "backward compatibility broken" must be replaced by the usage of CONFIG_RTE_NEXT_ABI while describing cases where it can apply. - The .map files must be generated in order to make it simpler and allows the use of CONFIG_RTE_NEXT_ABI.