[v4,2/5] eal: add lcore accessors
Checks
Commit Message
From: Stephen Hemminger <stephen@networkplumber.org>
The fields of the internal EAL core configuration are currently
laid bare as part of the API. This is not good practice and limits
fixing issues with layout and sizes.
Make new accessor functions for the fields used by current drivers
and examples.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
lib/librte_eal/common/eal_common_lcore.c | 33 +++++++++++++++++++++++++++
lib/librte_eal/common/include/rte_lcore.h | 38 +++++++++++++++++++------------
lib/librte_eal/rte_eal_version.map | 10 ++++++++
3 files changed, 67 insertions(+), 14 deletions(-)
---
Changelog since v3:
- updated title
- rebased on master
- removed doc update
- removed unneeded rte_lcore_return_code
Comments
23/05/2019 15:58, David Marchand:
> From: Stephen Hemminger <stephen@networkplumber.org>
>
> The fields of the internal EAL core configuration are currently
> laid bare as part of the API. This is not good practice and limits
> fixing issues with layout and sizes.
>
> Make new accessor functions for the fields used by current drivers
> and examples.
[...]
> +DPDK_19.08 {
> + global:
> +
> + rte_lcore_cpuset;
> + rte_lcore_index;
> + rte_lcore_to_cpu_id;
> + rte_lcore_to_socket_id;
> +
> +} DPDK_19.05;
> +
> EXPERIMENTAL {
> global:
Just to make sure, are we OK to introduce these functions
as non-experimental?
On Thu, 30 May 2019 00:46:30 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:
> 23/05/2019 15:58, David Marchand:
> > From: Stephen Hemminger <stephen@networkplumber.org>
> >
> > The fields of the internal EAL core configuration are currently
> > laid bare as part of the API. This is not good practice and limits
> > fixing issues with layout and sizes.
> >
> > Make new accessor functions for the fields used by current drivers
> > and examples.
> [...]
> > +DPDK_19.08 {
> > + global:
> > +
> > + rte_lcore_cpuset;
> > + rte_lcore_index;
> > + rte_lcore_to_cpu_id;
> > + rte_lcore_to_socket_id;
> > +
> > +} DPDK_19.05;
> > +
> > EXPERIMENTAL {
> > global:
>
> Just to make sure, are we OK to introduce these functions
> as non-experimental?
They were in previous releases as inlines this patch converts them
to real functions.
On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
stephen@networkplumber.org> wrote:
> On Thu, 30 May 2019 00:46:30 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > 23/05/2019 15:58, David Marchand:
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > >
> > > The fields of the internal EAL core configuration are currently
> > > laid bare as part of the API. This is not good practice and limits
> > > fixing issues with layout and sizes.
> > >
> > > Make new accessor functions for the fields used by current drivers
> > > and examples.
> > [...]
> > > +DPDK_19.08 {
> > > + global:
> > > +
> > > + rte_lcore_cpuset;
> > > + rte_lcore_index;
> > > + rte_lcore_to_cpu_id;
> > > + rte_lcore_to_socket_id;
> > > +
> > > +} DPDK_19.05;
> > > +
> > > EXPERIMENTAL {
> > > global:
> >
> > Just to make sure, are we OK to introduce these functions
> > as non-experimental?
>
> They were in previous releases as inlines this patch converts them
> to real functions.
>
>
Well, yes and no.
rte_lcore_index and rte_lcore_to_socket_id already existed, so making them
part of the ABI is fine for me.
rte_lcore_to_cpu_id is new but seems quite safe in how it can be used,
adding it to the ABI is ok for me.
rte_lcore_cpuset is new too, and still a bit obscure to me. I am not really
convinced we need it until I understand why dpaa2 and fslmc bus need to
know about this.
I might need more time to look at it, so flag this as experimental sounds
fair to me.
30/05/2019 09:31, David Marchand:
> On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> stephen@networkplumber.org> wrote:
>
> > On Thu, 30 May 2019 00:46:30 +0200
> > Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > > 23/05/2019 15:58, David Marchand:
> > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > >
> > > > The fields of the internal EAL core configuration are currently
> > > > laid bare as part of the API. This is not good practice and limits
> > > > fixing issues with layout and sizes.
> > > >
> > > > Make new accessor functions for the fields used by current drivers
> > > > and examples.
> > > [...]
> > > > +DPDK_19.08 {
> > > > + global:
> > > > +
> > > > + rte_lcore_cpuset;
> > > > + rte_lcore_index;
> > > > + rte_lcore_to_cpu_id;
> > > > + rte_lcore_to_socket_id;
> > > > +
> > > > +} DPDK_19.05;
> > > > +
> > > > EXPERIMENTAL {
> > > > global:
> > >
> > > Just to make sure, are we OK to introduce these functions
> > > as non-experimental?
> >
> > They were in previous releases as inlines this patch converts them
> > to real functions.
> >
> >
> Well, yes and no.
>
> rte_lcore_index and rte_lcore_to_socket_id already existed, so making them
> part of the ABI is fine for me.
>
> rte_lcore_to_cpu_id is new but seems quite safe in how it can be used,
> adding it to the ABI is ok for me.
It is used by DPAA and some test.
I guess adding as experimental is fine too?
I'm fine with both options, I'm just trying to apply the policy
we agreed on. Does this case deserve an exception?
> rte_lcore_cpuset is new too, and still a bit obscure to me. I am not really
> convinced we need it until I understand why dpaa2 and fslmc bus need to
> know about this.
> I might need more time to look at it, so flag this as experimental sounds
> fair to me.
On Thu, May 30, 2019 at 09:40:08AM +0200, Thomas Monjalon wrote:
> 30/05/2019 09:31, David Marchand:
> > On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> > stephen@networkplumber.org> wrote:
> >
> > > On Thu, 30 May 2019 00:46:30 +0200
> > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > > 23/05/2019 15:58, David Marchand:
> > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > >
> > > > > The fields of the internal EAL core configuration are currently
> > > > > laid bare as part of the API. This is not good practice and limits
> > > > > fixing issues with layout and sizes.
> > > > >
> > > > > Make new accessor functions for the fields used by current drivers
> > > > > and examples.
> > > > [...]
> > > > > +DPDK_19.08 {
> > > > > + global:
> > > > > +
> > > > > + rte_lcore_cpuset;
> > > > > + rte_lcore_index;
> > > > > + rte_lcore_to_cpu_id;
> > > > > + rte_lcore_to_socket_id;
> > > > > +
> > > > > +} DPDK_19.05;
> > > > > +
> > > > > EXPERIMENTAL {
> > > > > global:
> > > >
> > > > Just to make sure, are we OK to introduce these functions
> > > > as non-experimental?
> > >
> > > They were in previous releases as inlines this patch converts them
> > > to real functions.
> > >
> > >
> > Well, yes and no.
> >
> > rte_lcore_index and rte_lcore_to_socket_id already existed, so making them
> > part of the ABI is fine for me.
> >
> > rte_lcore_to_cpu_id is new but seems quite safe in how it can be used,
> > adding it to the ABI is ok for me.
>
> It is used by DPAA and some test.
> I guess adding as experimental is fine too?
> I'm fine with both options, I'm just trying to apply the policy
> we agreed on. Does this case deserve an exception?
>
While it may be a good candidate, I'm not sure how much making an exception
for it really matters. I'd be tempted to just mark it experimental and then
have it stable for the 19.11 release. What do we really lose by waiting a
release to stabilize it?
30/05/2019 12:11, Bruce Richardson:
> On Thu, May 30, 2019 at 09:40:08AM +0200, Thomas Monjalon wrote:
> > 30/05/2019 09:31, David Marchand:
> > > On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> > > stephen@networkplumber.org> wrote:
> > >
> > > > On Thu, 30 May 2019 00:46:30 +0200
> > > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > >
> > > > > 23/05/2019 15:58, David Marchand:
> > > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > > >
> > > > > > The fields of the internal EAL core configuration are currently
> > > > > > laid bare as part of the API. This is not good practice and limits
> > > > > > fixing issues with layout and sizes.
> > > > > >
> > > > > > Make new accessor functions for the fields used by current drivers
> > > > > > and examples.
> > > > > [...]
> > > > > > +DPDK_19.08 {
> > > > > > + global:
> > > > > > +
> > > > > > + rte_lcore_cpuset;
> > > > > > + rte_lcore_index;
> > > > > > + rte_lcore_to_cpu_id;
> > > > > > + rte_lcore_to_socket_id;
> > > > > > +
> > > > > > +} DPDK_19.05;
> > > > > > +
> > > > > > EXPERIMENTAL {
> > > > > > global:
> > > > >
> > > > > Just to make sure, are we OK to introduce these functions
> > > > > as non-experimental?
> > > >
> > > > They were in previous releases as inlines this patch converts them
> > > > to real functions.
> > > >
> > > >
> > > Well, yes and no.
> > >
> > > rte_lcore_index and rte_lcore_to_socket_id already existed, so making them
> > > part of the ABI is fine for me.
> > >
> > > rte_lcore_to_cpu_id is new but seems quite safe in how it can be used,
> > > adding it to the ABI is ok for me.
> >
> > It is used by DPAA and some test.
> > I guess adding as experimental is fine too?
> > I'm fine with both options, I'm just trying to apply the policy
> > we agreed on. Does this case deserve an exception?
> >
>
> While it may be a good candidate, I'm not sure how much making an exception
> for it really matters. I'd be tempted to just mark it experimental and then
> have it stable for the 19.11 release. What do we really lose by waiting a
> release to stabilize it?
I would agree Bruce.
If no more comment, I will wait for a v5 of this series.
On Thu, May 30, 2019 at 3:39 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> 30/05/2019 12:11, Bruce Richardson:
> > On Thu, May 30, 2019 at 09:40:08AM +0200, Thomas Monjalon wrote:
> > > 30/05/2019 09:31, David Marchand:
> > > > On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> > > > stephen@networkplumber.org> wrote:
> > > >
> > > > > On Thu, 30 May 2019 00:46:30 +0200
> > > > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > >
> > > > > > 23/05/2019 15:58, David Marchand:
> > > > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > > > >
> > > > > > > The fields of the internal EAL core configuration are currently
> > > > > > > laid bare as part of the API. This is not good practice and
> limits
> > > > > > > fixing issues with layout and sizes.
> > > > > > >
> > > > > > > Make new accessor functions for the fields used by current
> drivers
> > > > > > > and examples.
> > > > > > [...]
> > > > > > > +DPDK_19.08 {
> > > > > > > + global:
> > > > > > > +
> > > > > > > + rte_lcore_cpuset;
> > > > > > > + rte_lcore_index;
> > > > > > > + rte_lcore_to_cpu_id;
> > > > > > > + rte_lcore_to_socket_id;
> > > > > > > +
> > > > > > > +} DPDK_19.05;
> > > > > > > +
> > > > > > > EXPERIMENTAL {
> > > > > > > global:
> > > > > >
> > > > > > Just to make sure, are we OK to introduce these functions
> > > > > > as non-experimental?
> > > > >
> > > > > They were in previous releases as inlines this patch converts them
> > > > > to real functions.
> > > > >
> > > > >
> > > > Well, yes and no.
> > > >
> > > > rte_lcore_index and rte_lcore_to_socket_id already existed, so
> making them
> > > > part of the ABI is fine for me.
> > > >
> > > > rte_lcore_to_cpu_id is new but seems quite safe in how it can be
> used,
> > > > adding it to the ABI is ok for me.
> > >
> > > It is used by DPAA and some test.
> > > I guess adding as experimental is fine too?
> > > I'm fine with both options, I'm just trying to apply the policy
> > > we agreed on. Does this case deserve an exception?
> > >
> >
> > While it may be a good candidate, I'm not sure how much making an
> exception
> > for it really matters. I'd be tempted to just mark it experimental and
> then
> > have it stable for the 19.11 release. What do we really lose by waiting a
> > release to stabilize it?
>
> I would agree Bruce.
> If no more comment, I will wait for a v5 of this series.
>
I agree that there is no reason we make an exception for those 2 new ones.
But to me the existing rte_lcore_index and rte_lcore_to_socket_id must be
marked as stable.
This is to avoid breaking existing users that did not set
ALLOW_EXPERIMENTAL_API.
I will prepare a v5 later.
On Thu, May 30, 2019 at 07:00:36PM +0200, David Marchand wrote:
> On Thu, May 30, 2019 at 3:39 PM Thomas Monjalon
> <[1]thomas@monjalon.net> wrote:
>
> 30/05/2019 12:11, Bruce Richardson:
> > On Thu, May 30, 2019 at 09:40:08AM +0200, Thomas Monjalon wrote:
> > > 30/05/2019 09:31, David Marchand:
> > > > On Thu, May 30, 2019 at 12:51 AM Stephen Hemminger <
> > > > [2]stephen@networkplumber.org> wrote:
> > > >
> > > > > On Thu, 30 May 2019 00:46:30 +0200
> > > > > Thomas Monjalon <[3]thomas@monjalon.net> wrote:
> > > > >
> > > > > > 23/05/2019 15:58, David Marchand:
> > > > > > > From: Stephen Hemminger <[4]stephen@networkplumber.org>
> > > > > > >
> > > > > > > The fields of the internal EAL core configuration are
> currently
> > > > > > > laid bare as part of the API. This is not good practice
> and limits
> > > > > > > fixing issues with layout and sizes.
> > > > > > >
> > > > > > > Make new accessor functions for the fields used by
> current drivers
> > > > > > > and examples.
> > > > > > [...]
> > > > > > > +DPDK_19.08 {
> > > > > > > + global:
> > > > > > > +
> > > > > > > + rte_lcore_cpuset;
> > > > > > > + rte_lcore_index;
> > > > > > > + rte_lcore_to_cpu_id;
> > > > > > > + rte_lcore_to_socket_id;
> > > > > > > +
> > > > > > > +} DPDK_19.05;
> > > > > > > +
> > > > > > > EXPERIMENTAL {
> > > > > > > global:
> > > > > >
> > > > > > Just to make sure, are we OK to introduce these functions
> > > > > > as non-experimental?
> > > > >
> > > > > They were in previous releases as inlines this patch
> converts them
> > > > > to real functions.
> > > > >
> > > > >
> > > > Well, yes and no.
> > > >
> > > > rte_lcore_index and rte_lcore_to_socket_id already existed, so
> making them
> > > > part of the ABI is fine for me.
> > > >
> > > > rte_lcore_to_cpu_id is new but seems quite safe in how it can
> be used,
> > > > adding it to the ABI is ok for me.
> > >
> > > It is used by DPAA and some test.
> > > I guess adding as experimental is fine too?
> > > I'm fine with both options, I'm just trying to apply the policy
> > > we agreed on. Does this case deserve an exception?
> > >
> >
> > While it may be a good candidate, I'm not sure how much making an
> exception
> > for it really matters. I'd be tempted to just mark it experimental
> and then
> > have it stable for the 19.11 release. What do we really lose by
> waiting a
> > release to stabilize it?
> I would agree Bruce.
> If no more comment, I will wait for a v5 of this series.
>
> I agree that there is no reason we make an exception for those 2 new
> ones.
> But to me the existing rte_lcore_index and rte_lcore_to_socket_id must
> be marked as stable.
> This is to avoid breaking existing users that did not set
> ALLOW_EXPERIMENTAL_API.
> I will prepare a v5 later.
> --
Yes, agreed. Any existing APIs that were already present as static inlines
can go straight to stable when added to the .map file.
/Bruce
@@ -16,6 +16,39 @@
#include "eal_private.h"
#include "eal_thread.h"
+int rte_lcore_index(int lcore_id)
+{
+ if (unlikely(lcore_id >= RTE_MAX_LCORE))
+ return -1;
+
+ if (lcore_id < 0)
+ lcore_id = (int)rte_lcore_id();
+
+ return lcore_config[lcore_id].core_index;
+}
+
+int rte_lcore_to_cpu_id(int lcore_id)
+{
+ if (unlikely(lcore_id >= RTE_MAX_LCORE))
+ return -1;
+
+ if (lcore_id < 0)
+ lcore_id = (int)rte_lcore_id();
+
+ return lcore_config[lcore_id].core_id;
+}
+
+rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id)
+{
+ return lcore_config[lcore_id].cpuset;
+}
+
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id)
+{
+ return lcore_config[lcore_id].socket_id;
+}
+
static int
socket_id_cmp(const void *a, const void *b)
{
@@ -121,15 +121,7 @@ struct lcore_config {
* @return
* The relative index, or -1 if not enabled.
*/
-static inline int
-rte_lcore_index(int lcore_id)
-{
- if (lcore_id >= RTE_MAX_LCORE)
- return -1;
- if (lcore_id < 0)
- lcore_id = (int)rte_lcore_id();
- return lcore_config[lcore_id].core_index;
-}
+int rte_lcore_index(int lcore_id);
/**
* Return the ID of the physical socket of the logical core we are
@@ -177,11 +169,29 @@ struct lcore_config {
* @return
* the ID of lcoreid's physical socket
*/
-static inline unsigned int
-rte_lcore_to_socket_id(unsigned int lcore_id)
-{
- return lcore_config[lcore_id].socket_id;
-}
+unsigned int
+rte_lcore_to_socket_id(unsigned int lcore_id);
+
+/**
+ * Return the id of the lcore on a socket starting from zero.
+ *
+ * @param lcore_id
+ * The targeted lcore, or -1 for the current one.
+ * @return
+ * The relative index, or -1 if not enabled.
+ */
+int
+rte_lcore_to_cpu_id(int lcore_id);
+
+/**
+ * Return the cpuset for a given lcore.
+ * @param lcore_id
+ * the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
+ * @return
+ * The cpuset of that lcore
+ */
+rte_cpuset_t
+rte_lcore_cpuset(unsigned int lcore_id);
/**
* Test if an lcore is enabled.
@@ -287,6 +287,16 @@ DPDK_19.05 {
} DPDK_18.11;
+DPDK_19.08 {
+ global:
+
+ rte_lcore_cpuset;
+ rte_lcore_index;
+ rte_lcore_to_cpu_id;
+ rte_lcore_to_socket_id;
+
+} DPDK_19.05;
+
EXPERIMENTAL {
global: