Re: [dpdk-dev] [dpdk-announce] important design choices - statistics - ABI

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Morten Brørup
> Sent: Wednesday, June 17, 2015 10:54 AM
> To: Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-announce] important design choices -
> statistics - ABI
> 
> Dear Thomas,
> 
> I don't have time to follow the DPDK Developers mailing list, but since you call
> for feedback, I would like to share my thoughts regarding these design
> choices.
> 
> 
> Regarding the statistics discussion:
> 
> 1. The suggested solution assumes that, when statistics is disabled, the cost
> of allocating and maintaining zero-value statistics is negligible. If statistics
> counters are only available through accessor functions, this is probably true.
> 
> However, if statistics counters are directly accessible, e.g. as elements in the
> fast path data structures of a library, maintaining zero-value statistics may a
> have memory and/or performance impact.
> 

Counters are only accessible through API functions.

> Since the compile time flag
> CONFIG_RTE_<LIBRARY_NAME>_STATS_COLLECT already tells the
> application if the statistics are present or not, the application should simply
> use this flag to determine if statistics are accessible or not.
> 
> 2. The suggested solution with only one single flag per library prevents
> implementing statistics with varying granularity for different purposes. E.g. a
> library could have one set of statistics counters for ordinary SNMP purposes,
> and another set of statistics counters for debugging/optimization purposes.
> 
> Multiple flags per library should be possible. A hierarchy of flags per library is
> probably not required.
> 

Morten, thank you for your input. It would be good if you could add your contribution to the of the guidelines documentation patch by replying to the thread that Thomas indicated: http://dpdk.org/ml/archives/dev/2015-June/019461.html.

Our initial stats patch submission had a much finer granularity of stats configuration: per object type instead of per library, but a lot of people on this mailing list are against this, so we are now looking for one configuration flag per library.

> 
> Regarding the PHY speed ABI:
> 
> 1. The Ethernet PHY ABI for speed, duplex, etc. should be common
> throughout the entire DPDK. It might be confusing if some
> structures/functions use a bitmask to indicate PHY
> speed/duplex/personality/etc. and other structures/functions use a
> combination of an unsigned integer, duplex flag, personality enumeration
> etc. (By personality enumeration, I am referring to PHYs with multiple
> electrical interfaces. E.g. a dual personality PHY might have both an RJ45
> copper interface and an SFP module interface, whereof only one can be
> active at any time.)
> 
> 2. The auto-negotiation standard allows the PHY to announce (to its link
> partner) any subset of its capabilities to its link partner. E.g. a standard
> 10/100/100 Ethernet PHY (which can handle both 10 and 100 Mbit/s in both
> half and full duplex and 1 Gbit/s full duplex) can be configured to announce
> 10 Mbit/s half duplex and 100 Mbit/s full duplex capabilities to its link partner.
> (Of course, more useful combinations are normally announced, but the
> purpose of the example is to show that any combination is possible.)
> 
> The ABI for auto-negotiation should include options to select the list of
> capabilities to announce to the link partner. The Linux PHY ABI only allows
> forcing a selected speed and duplex (thereby disabling auto-negotiation) or
> enabling auto-negotiation (thereby announcing all possible speeds and
> duplex combinations the PHY is capable of). Don't make the same mistake in
> DPDK.
> 
> PS: While working for Vitesse Semiconductors (an Ethernet chip company) a
> long time ago, I actually wrote the API for their line of Ethernet PHYs. So I
> have hands on experience in this area.
> 
> 
> Regarding the discussion about backwards/forwards compatibility in the ABI:
> 
> 1. Sometimes, ABI breakage is required. That is the cost the users pay for
> getting the benefits from upgrading to the latest and greatest version of any
> library. The current solution of requiring acknowledgement from several
> qualified developers is fine - these developers will consider the cost/benefit
> on behalf of all the DPDK users and make a qualified decision.
> 
> 2. It is my general experience that documentation is not always updated to
> reflect the fine details of the source code, and this also applies to release
> notes. For open source software, the primary point of documentation is
> usually the source code itself.
> 
> 2a. It should be clearly visible directly in the DPDK source code (including
> makefiles etc.) which ABI (i.e. functions, macros, type definitions etc.) is the
> current, the deprecated, and the future.
> 
> 2b. When a developer migrates a project using DPDK from a previous version
> of the DPDK, it should be easy for the developer to identify all DPDK ABI
> modifications and variants, e.g. by using a common indicator in the DPDK
> source code, such as LIBAPIVER, that developer can simply search for.
> 
> 3. Adding special feature flags, e.g. CONFIG_RTE_EAL_RX_INTR, to indicate a
> breakage of the ABI, should only be done if it is the intention to keep both
> the current and the new variants of the feature in the DPDK in the future.
> Otherwise, such a flag should be combined with the standard ABI version
> indication, so it is clear that this feature belongs to certain versions (i.e.
> deprecated, current or future).
> 
> 
> Med venlig hilsen / kind regards
> 
> Morten Brørup
> CTO
> 
> 
> 
> SmartShare Systems A/S
> Tonsbakken 16-18
> DK-2740 Skovlunde
> Denmark
> 
> Office      +45 70 20 00 93
> Direct      +45 89 93 50 22
> Mobile      +45 25 40 82 12
> 
> mb@smartsharesystems.com
> www.smartsharesystems.com
> -----Original Message-----
> From: announce [mailto:announce-bounces@dpdk.org] On Behalf Of
> Thomas Monjalon
> Sent: 17. juni 2015 01:30
> To: announce@dpdk.org
> Subject: [dpdk-announce] important design choices - statistics - ABI
> 
> Hi all,
> 
> Sometimes there are some important discussions about architecture or
> design which require opinions from several developers. Unfortunately, we
> cannot read every threads. Maybe that using the announce mailing list will
> help to bring more audience to these discussions.
> Please note that
> 	- the announce@ ML is moderated to keep a low traffic,
> 	- every announce email is forwarded to dev@ ML.
> In case you want to reply to this email, please use dev@dpdk.org address.
> 
> There were some debates about software statistics disabling.
> Should they be always on or possibly disabled when compiled?
> We need to take a decision shortly and discuss (or agree) this proposal:
> 	http://dpdk.org/ml/archives/dev/2015-June/019461.html
> 
> During the development of the release 2.0, there was an agreement to keep
> ABI compatibility or to bring new ABI while keeping old one during one
> release.
> In case it's not possible to have this transition, the (exceptional) break should
> be acknowledged by several developers.
> 	http://dpdk.org/doc/guides-2.0/rel_notes/abi.html
> There were some interesting discussions but not a lot of participants:
> 	http://thread.gmane.org/gmane.comp.networking.dpdk.devel/8367
> /focus=8461
> 
> During the current development cycle for the release 2.1, the ABI question
> arises many times in different threads.
> To add the hash key size field, it is proposed to use a struct padding gap:
> 	http://dpdk.org/ml/archives/dev/2015-June/019386.html
> To support the flow director for VF, there is no proposal yet:
> 	http://dpdk.org/ml/archives/dev/2015-June/019343.html
> To add the speed capability, it is proposed to break ABI in the release 2.2:
> 	http://dpdk.org/ml/archives/dev/2015-June/019225.html
> To support vhost-user multiqueues, it is proposed to break ABI in 2.2:
> 	http://dpdk.org/ml/archives/dev/2015-June/019443.html
> To add the interrupt mode, it is proposed to add a build-time option
> CONFIG_RTE_EAL_RX_INTR to switch between compatible and ABI breaking
> binary:
> 	http://dpdk.org/ml/archives/dev/2015-June/018947.html
> To add the packet type, there is a proposal to add a build-time option
> CONFIG_RTE_NEXT_ABI common to every ABI breaking features:
> 	http://dpdk.org/ml/archives/dev/2015-June/019172.html
> We must also better document how to remove a deprecated ABI:
> 	http://dpdk.org/ml/archives/dev/2015-June/019465.html
> The ABI compatibility is a new constraint and we need to better understand
> what it means and how to proceed. Even the macros are not yet well
> documented:
> 	http://dpdk.org/ml/archives/dev/2015-June/019357.html
> 
> Thanks for your attention and your participation in these important choices.