DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation
Date: Wed, 24 Jun 2015 23:09:29 +0200	[thread overview]
Message-ID: <4632196.zdulcdXT2t@xps13> (raw)
In-Reply-To: <1435170885-17643-2-git-send-email-nhorman@tuxdriver.com>

2015-06-24 14:34, Neil Horman:
> +Some ABI changes may be too significant to reasonably maintain multiple
> +versions. In those cases ABI's may be updated without backward compatibility
> +being provided. The requirements for doing so are:
> +
> +#. At least 3 acknowledgments of the need to do so must be made on the
> +   dpdk.org mailing list.
> +
> +#. A full deprecation cycle, as explained above, must be made to offer
> +   downstream consumers sufficient warning of the change.
> +
> +#. The ``LIBABIVER`` variable in the makefile(s) where the ABI changes are
> +   incorporated must be incremented in parallel with the ABI changes
> +   themselves.

The proposal was to provide the old and the new ABI in the same source code
during the deprecation cycle. The old ABI would be the default and people
can build the new one by enabling the NEXT_ABI config option.
So the migration to the new ABI is smoother.


[...]
> +The macros exported are:
> +
> +* ``VERSION_SYMBOL(b, e, n)``: Creates a symbol version table entry binding
> +  unversioned symbol ``b`` to the internal function ``b_e``.

The definition is the same as BASE_SYMBOL.

> +* ``BASE_SYMBOL(b, e)``: Creates a symbol version table entry binding
> +  unversioned symbol ``b`` to the internal function ``b_e``.


[...]
> +   DPDK_2.0 {
> +        global:
> +
> +        rte_acl_add_rules;
> +        rte_acl_build;
> +        rte_acl_classify;
> +        rte_acl_classify_alg;
> +        rte_acl_classify_scalar;
> +        rte_acl_create;

So it's declared twice, right?
I think it should be explicit.

> +        rte_acl_dump;
> +        rte_acl_find_existing;
> +        rte_acl_free;
> +        rte_acl_ipv4vlan_add_rules;
> +        rte_acl_ipv4vlan_build;
> +        rte_acl_list_dump;
> +        rte_acl_reset;
> +        rte_acl_reset_rules;
> +        rte_acl_set_ctx_classify;
> +
> +        local: *;
> +   };
> +
> +   DPDK_2.1 {
> +        global:
> +        rte_acl_create;
> +
> +   } DPDK_2.0;

[...]
> +Note that the base name of the symbol was kept in tact, as this is condusive to

s/in tact/intact/?

[...]
> +the macros used for versioning symbols.  That is our next step, mapping this new
> +symbol name to the initial symbol name at version node 2.0.  Immediately after
> +the function, we add this line of code
> +
> +.. code-block:: c
> +
> +   VERSION_SYMBOL(rte_acl_create, _v20, 2.0);

Can it be declared before the function?

[...]
> +Remembering to also add the rte_compat.h header to the requisite c file where
> +these changes are being made.  The above macro instructs the linker to create a
> +new symbol ``rte_acl_create@DPDK_2.0``, which matches the symbol created in older
> +builds, but now points to the above newly named function.  We have now mapped
> +the original rte_acl_create symbol to the original function (but with a new
> +name)

Could we use VERSION_SYMBOL(rte_acl_create, , 2.0);
when introducing the function in DPDK 2.0 (before any ABI breakage)?
It could help to generate the .map file.

When do we need to use BASE_SYMBOL?

[...]
> +This code serves as our new API call.  Its the same as our old call, but adds
> +the new parameter in place.  Next we need to map this function to the symbol
> +``rte_acl_create@DPDK_2.1``.  To do this, we modify the public prototype of the call
> +in the header file, adding the macro there to inform all including applications,
> +that on re-link, the default rte_acl_create symbol should point to this
> +function.  Note that we could do this by simply naming the function above
> +rte_acl_create, and the linker would chose the most recent version tag to apply
> +in the version script, but we can also do this in the header file
> +
> +.. code-block:: c
> +
> +   struct rte_acl_ctx *
> +   -rte_acl_create(const struct rte_acl_param *param);
> +   +rte_acl_create(const struct rte_acl_param *param, int debug);
> +   +BIND_DEFAULT_SYMBOL(rte_acl_create, _v21, 2.1);

Will it work with static library?

> +Next remove the corresponding versioned export
> +.. code-block:: c
> +
> + -VERSION_SYMBOL(rte_acl_create, _v20, 2.0);
> +
> +
> +Note that the internal function definition could also be removed, but its used
> +in our example by the newer version _v21, so we leave it in place.  This is a
> +coding style choice.
> +
> +Lastly, we need to bump the LIBABIVER number for this library in the Makefile to
> +indicate to applications doing dynamic linking that this is a later, and
> +possibly incompatible library version:
> +
> +.. code-block:: c
> +
> +   -LIBABIVER := 1
> +   +LIBABIVER := 2

Very well explained, thanks.

[...]
> +        rte_acl_add_rules;
> +        rte_acl_build;
> +        rte_acl_classify;
> +        rte_acl_classify_alg;
> +        rte_acl_classify_scalar;
> +        rte_acl_dump;
> +        rte_acl_create

Not in alphabetical order.


As you copy a part of abi.rst, it should be removed from the original doc.

Thanks Neil

  reply	other threads:[~2015-06-24 21:10 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-23 19:33 [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Neil Horman
2015-06-23 19:33 ` [dpdk-dev] [PATCH 2/2] ABI: Add some documentation Neil Horman
2015-06-24 11:21   ` Mcnamara, John
2015-06-24 11:23 ` [dpdk-dev] [PATCH 1/2] rte_compat.h : Clean up some typos Mcnamara, John
2015-06-24 18:06   ` Neil Horman
2015-06-24 18:34 ` [dpdk-dev] [PATCHv2 " Neil Horman
2015-06-24 18:34   ` [dpdk-dev] [PATCHv2 2/2] ABI: Add some documentation Neil Horman
2015-06-24 21:09     ` Thomas Monjalon [this message]
2015-06-25 11:35       ` Neil Horman
2015-06-25 13:22         ` Thomas Monjalon
2015-06-25  7:19     ` Zhang, Helin
2015-06-25  7:42       ` Gonzalez Monroy, Sergio
2015-06-25  8:00         ` Gonzalez Monroy, Sergio
2015-06-25 12:25       ` Neil Horman
2015-06-29 16:35         ` [dpdk-dev] [PATCH] lib: remove redundant definition of local symbols Thomas Monjalon
2015-06-30 15:50           ` Thomas Monjalon
2015-06-24 19:41   ` [dpdk-dev] [PATCHv2 1/2] rte_compat.h : Clean up some typos Thomas Monjalon
2015-06-24 20:15     ` Neil Horman
2015-06-24 20:49       ` Thomas Monjalon
2015-06-25  7:37 ` [dpdk-dev] [PATCH " Gajdzica, MaciejX T
2015-06-25 12:28   ` Neil Horman
2015-06-25 14:35 ` [dpdk-dev] [PATCHv3 1/3] " Neil Horman
2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 2/3] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
2015-06-26 10:13     ` Gajdzica, MaciejX T
2015-06-26 12:52     ` Thomas Monjalon
2015-06-26 14:30       ` Neil Horman
2015-06-28 20:13         ` Thomas Monjalon
2015-06-29 13:44           ` Neil Horman
2015-06-25 14:35   ` [dpdk-dev] [PATCHv3 3/3] ABI: Add some documentation Neil Horman
2015-06-26 13:00     ` Thomas Monjalon
2015-06-26 14:54       ` Neil Horman
2015-06-28 20:24         ` Thomas Monjalon
2015-06-29 13:53           ` Neil Horman
2015-06-26 12:45   ` [dpdk-dev] [PATCHv3 1/3] rte_compat.h : Clean up some typos Thomas Monjalon
2015-06-29 13:59 ` [dpdk-dev] [PATCHv4 1/4] " Neil Horman
2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 2/4] rte_compat: Add MAP_STATIC_SYMBOL macro Neil Horman
2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 3/4] rte_compat: remove BASE_SYMBOL Neil Horman
2015-06-29 13:59   ` [dpdk-dev] [PATCHv4 4/4] ABI: Add some documentation Neil Horman
2015-06-29 15:07     ` Thomas Monjalon
2015-07-08  9:52   ` [dpdk-dev] [PATCHv4 1/4] rte_compat.h : Clean up some typos Thomas Monjalon
2015-07-08 11:04     ` Neil Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4632196.zdulcdXT2t@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=nhorman@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).