From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f53.google.com (mail-wg0-f53.google.com [74.125.82.53]) by dpdk.org (Postfix) with ESMTP id 752551DB1 for ; Wed, 10 Jun 2015 18:15:57 +0200 (CEST) Received: by wgez8 with SMTP id z8so39620041wge.0 for ; Wed, 10 Jun 2015 09:15:57 -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=uXFTHNBd8Hqf2wLX5Y9n3tXOwfS6aa520XcRbYuw6cA=; b=O5nO5QpiCb3VZ4OtRq9VD+JsfpiqrQ79FmPCIH0RwZv8XzipPe24tn0GX2rkY4/iXj 2mA+pTr8Tv2FdziITLJCcmybZ9WzwQR9KmOYXFNbE4u3buRRq2XdAjH060m20cPlw8Nv bghOshp8cARKhQO8KO9DR8lsTPEi/7KJKOrGIUU17+oIz633buQrSMpfwZSlYhz+Uuzp ZQ742ctE1UMAOyRTwy2PvBPzDARse06ARK3BRPlFONi83Ogi5/h7m/NWIMrbfRarcvB2 +bY7Rjz/GC++WKJmsoE/6OzEsqmLNyNsg4RTpJyhrxMUrnMhuipqwF1YeoAlEvChq+ZD MTjg== X-Gm-Message-State: ALoCoQmT4mZqF7n7MFX0WVsrFVZjwt6VkBGje6du4lsteRQ22J930H1yOq87LTEA4QcMWAv/xxBD X-Received: by 10.180.108.142 with SMTP id hk14mr9826605wib.5.1433952957306; Wed, 10 Jun 2015 09:15:57 -0700 (PDT) Received: from xps13.localnet (60.26.90.92.rev.sfr.net. [92.90.26.60]) by mx.google.com with ESMTPSA id lf4sm15260698wjb.42.2015.06.10.09.15.55 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 Jun 2015 09:15:56 -0700 (PDT) From: Thomas Monjalon To: Olivier MATZ , "O'Driscoll, Tim" , "Zhang, Helin" , nhorman@tuxdriver.com Date: Wed, 10 Jun 2015 18:14:59 +0200 Message-ID: <4747564.NbtnEmoytI@xps13> Organization: 6WIND User-Agent: KMail/4.14.8 (Linux/4.0.4-2-ARCH; KDE/4.14.8; x86_64; ; ) In-Reply-To: <55784A82.6020801@6wind.com> References: <1432284264-17376-1-git-send-email-helin.zhang@intel.com> <26FA93C7ED1EAA44AB77D62FBE1D27BA54D536B3@IRSMSX102.ger.corp.intel.com> <55784A82.6020801@6wind.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v6 01/18] mbuf: redefine packet_type in rte_mbuf 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: Wed, 10 Jun 2015 16:15:57 -0000 2015-06-10 16:32, Olivier MATZ: > On 06/02/2015 03:27 PM, O'Driscoll, Tim wrote: > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ > >> On 06/01/2015 09:33 AM, Helin Zhang wrote: > >>> In order to unify the packet type, the field of 'packet_type' in > >>> 'struct rte_mbuf' needs to be extended from 16 to 32 bits. > >>> Accordingly, some fields in 'struct rte_mbuf' are re-organized to > >>> support this change for Vector PMD. As 'struct rte_kni_mbuf' for > >>> KNI should be right mapped to 'struct rte_mbuf', it should be > >>> modified accordingly. In addition, Vector PMD of ixgbe is disabled > >>> by default, as 'struct rte_mbuf' changed. > >>> To avoid breaking ABI compatibility, all the changes would be > >>> enabled by RTE_UNIFIED_PKT_TYPE, which is disabled by default. > >> > >> What are the plans for this compile-time option in the future? > >> > >> I wonder what are the benefits of having this option in terms > >> of ABI compatibility: when it is disabled, it is ABI-compatible but > >> the packet-type feature is not present, and when it is enabled we > >> have the feature but it breaks the compatibility. > >> > >> In my opinion, the v5 is preferable: for this kind of features, I > >> don't see how the ABI can be preserved, and I think packet-type > >> won't be the only feature that will modify the mbuf structure. I think > >> the process described here should be applied: > >> http://dpdk.org/browse/dpdk/tree/doc/guides/rel_notes/abi.rst > >> > >> (starting from "Some ABI changes may be too significant to reasonably > >> maintain multiple versions of"). > > > > This is just like the change that Steve (Cunming) Liang submitted for > > Interrupt Mode. We have the same problem in both cases: we want to find > > a way to get the features included, but need to comply with our ABI > > policy. So, in both cases, the proposal is to add a config option to > > enable the change by default, so we maintain backward compatibility. > > Users that want these changes, and are willing to accept the > > associated ABI change, have to specifically enable them. > > > > We can note in the Deprecation Notices in the Release Notes for 2.1 > > that these config options will be removed in 2.2. The features will > > then be enabled by default. > > > > This seems like a good compromise which allows us to get these changes > > into 2.1 but avoids breaking the ABI policy. > > Sorry for the late answer. > > After some thoughts on this topic, I understand that having a > compile-time option is perhaps a good compromise between > keeping compatibility and having new features earlier. > > I'm just afraid about having one #ifdef in the code for > each new feature that cannot keep the ABI compatibility. > What do you think about having one option -- let's call > it "CONFIG_RTE_NEXT_ABI" --, that is disabled by default, > and that would surround any new feature that breaks the > ABI? > > This would have several advantages: > - only 2 cases (on or off), the combinatorial is smaller than > having one option per feature > - all next features breaking the abi can be identified by a grep > - the code inside the #ifdef can be enabled in a simple operation > by Thomas after each release. > > Thomas, any comment? As previously discussed (1to1) with Olivier, I think that's a good proposal to introduce changes breaking deeply the ABI. Let's sum up the current policy: 1/ For changes which have a limited impact on the ABI, the backward compatibility must be kept during 1 release including the notice in doc/guides/rel_notes/abi.rst. 2/ For important changes like mbuf rework, there was an agreement on skipping the backward compatibility after having 3 acknowledgements and an 1-release long notice. Then the ABI numbering must be incremented. This CONFIG_RTE_NEXT_ABI proposal would change the rules for the second case. In order to be adopted, a patch for the file doc/guides/rel_notes/abi.rst must be submitted and strongly acknowledged. The ABI numbering must be also clearly explained: 1/ Should we have different libraries version number depending of CONFIG_RTE_NEXT_ABI? It seems straightforward to use "ifeq" when LIBABIVER in the Makefiles 2/ Are we able to have some "if CONFIG_RTE_NEXT_ABI" statement in the .map files? Maybe we should remove these files and generate them with some preprocessing. Neil, as the ABI policy author, what is your opinion?