From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) by dpdk.org (Postfix) with ESMTP id 158A8DE6 for ; Wed, 10 Jun 2015 16:32:41 +0200 (CEST) Received: by wibut5 with SMTP id ut5so50501931wib.1 for ; Wed, 10 Jun 2015 07:32:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=9u4VRUdz6ChnyTgPy+/VWkJWRSl9dwYm5xzC9WrZbcA=; b=RnfQ56mQhFMKQF53KHQ4MB195Be7SDuXdSMN0AfIYyhG/mgbSm6BlagSwgB9UDsSk5 /47kw3h67DepzIXktXTvmR5TDsbNo1qLsLYpynEbukZExIGDYZSBrqQQkuDrSslWU6df Vrt8WU3wFfPmsR+5plUWfxBwUyaFlrAVWuZPzsHlndewaP0JaoND23zwOkjMzIZxkpfE 8zWt2bT+vVX7sqX6gx9pVBKMJh0ARsk8FPicD/q9EU5TfLEe0VQd+zxb9rGOXvtZ2r+y wMUto8/Mr4Z1mS3Rux+OSVRHw2NrbVEh0fJ3+TiQyyNPJIg9D9KVKNtDCvuAdajIivFG EYVg== X-Gm-Message-State: ALoCoQmbh4JqQh1NYevZhsUN+jHKBQIBgcrWMIrLHpPTdWfNXwH5V3rs3YUMznUyb2/P3P6JhyYK X-Received: by 10.194.103.2 with SMTP id fs2mr6812826wjb.151.1433946760884; Wed, 10 Jun 2015 07:32:40 -0700 (PDT) Received: from [10.16.0.195] (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id fb3sm8262109wib.21.2015.06.10.07.32.39 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 Jun 2015 07:32:39 -0700 (PDT) Message-ID: <55784A82.6020801@6wind.com> Date: Wed, 10 Jun 2015 16:32:34 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: "O'Driscoll, Tim" , "Zhang, Helin" , "dev@dpdk.org" References: <1432284264-17376-1-git-send-email-helin.zhang@intel.com> <1433144045-30847-1-git-send-email-helin.zhang@intel.com> <1433144045-30847-2-git-send-email-helin.zhang@intel.com> <556C1478.9040005@6wind.com> <26FA93C7ED1EAA44AB77D62FBE1D27BA54D536B3@IRSMSX102.ger.corp.intel.com> In-Reply-To: <26FA93C7ED1EAA44AB77D62FBE1D27BA54D536B3@IRSMSX102.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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 14:32:41 -0000 Hi Tim, Helin, On 06/02/2015 03:27 PM, O'Driscoll, Tim wrote: > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ >> Sent: Monday, June 1, 2015 9:15 AM >> To: Zhang, Helin; dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v6 01/18] mbuf: redefine packet_type in >> rte_mbuf >> >> Hi Helin, >> >> +CC Neil >> >> 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"). >> >> >> Regards, >> Olivier >> > > 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? Regards, Olivier