Hi Tim, Helin, On 06/02/2015 03:27 PM, O'Driscoll, Tim wrote: > >> -----Original Message----- >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier MATZ >> Sent: Monday, June 1, 2015 9:15 AM >> To: Zhang, Helin; dev at 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
