DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v1 1/5] eal: add accessor functions for lcore_config
Date: Tue, 9 Apr 2019 09:43:00 +0200	[thread overview]
Message-ID: <CAJFAV8z3Se7z6u6zFAzGxJPF2zt3LetUm_pdywjWPJgL04dECA@mail.gmail.com> (raw)
In-Reply-To: <20190408182510.16078-2-stephen@networkplumber.org>

On Mon, Apr 8, 2019 at 8:25 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> 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. Mark the state and return code functions as experimental
> since these values might change in the future and probably shouldn't
> have been used by non EAL code anyway.
>

You removed the state function, no need to mention it.

Since we already expose the state, exposing the ret code seems fair.
Applications can wait themselves and check the return code the way they
want.

Just want to point that the current test app could do without it, since it
is quite simple, it waits for all, then checks all return codes.



> Most of these functions are not marked as experimental since
> we want applications to convert over asap. The one exception is
> the return_code, which is only used by some tests now and not
> clear that it is even that useful.
>

+1


diff --git a/lib/librte_eal/common/eal_common_lcore.c
> b/lib/librte_eal/common/eal_common_lcore.c
> index 1cbac42286ba..c3cf5a06269d 100644
> --- a/lib/librte_eal/common/eal_common_lcore.c
> +++ b/lib/librte_eal/common/eal_common_lcore.c
> @@ -16,6 +16,45 @@
>  #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
>

Ah, checkpatch does not complain for me, I would have expected a warning on
"unsigned int".
Can you fix it so that it matches the declaration from the header ?

+rte_lcore_to_socket_id(unsigned int lcore_id)
> +{
> +       return lcore_config[lcore_id].socket_id;
> +}
> +
> +int
> +rte_lcore_return_code(unsigned int lcore_id)
> +{
> +       return lcore_config[lcore_id].ret;
> +}
> +
>  static int
>  socket_id_cmp(const void *a, const void *b)
>  {



The rest looks fine.
Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

  parent reply	other threads:[~2019-04-09  7:43 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-08 18:25 [dpdk-dev] [PATCH v1 0/5] make lcore_config internal Stephen Hemminger
2019-04-08 18:25 ` Stephen Hemminger
2019-04-08 18:25 ` [dpdk-dev] [PATCH v1 1/5] eal: add accessor functions for lcore_config Stephen Hemminger
2019-04-08 18:25   ` Stephen Hemminger
2019-04-09  7:43   ` David Marchand [this message]
2019-04-09  7:43     ` David Marchand
2019-04-08 18:25 ` [dpdk-dev] [PATCH v1 2/5] bus: use lcore accessor functions Stephen Hemminger
2019-04-08 18:25   ` Stephen Hemminger
2019-04-09  7:43   ` David Marchand
2019-04-09  7:43     ` David Marchand
2019-04-08 18:25 ` [dpdk-dev] [PATCH v1 3/5] examples/bond: use lcore accessor Stephen Hemminger
2019-04-08 18:25   ` Stephen Hemminger
2019-04-09  7:43   ` David Marchand
2019-04-09  7:43     ` David Marchand
2019-04-08 18:25 ` [dpdk-dev] [PATCH v1 4/5] app/test: use lcore accessor functions Stephen Hemminger
2019-04-08 18:25   ` Stephen Hemminger
2019-04-09  7:43   ` David Marchand
2019-04-09  7:43     ` David Marchand
2019-04-08 18:25 ` [dpdk-dev] [PATCH v1 5/5] eal: make lcore_config private Stephen Hemminger
2019-04-08 18:25   ` Stephen Hemminger
2019-04-10 17:15 ` [dpdk-dev] [PATCH v2 0/5] make lcore_config internal Stephen Hemminger
2019-04-10 17:15   ` Stephen Hemminger
2019-04-10 17:15   ` [dpdk-dev] [PATCH v2 1/5] eal: use unsigned int in rte_lcore.h functions Stephen Hemminger
2019-04-10 17:15     ` Stephen Hemminger
2019-05-03  7:24     ` David Marchand
2019-05-03  7:24       ` David Marchand
2019-04-10 17:16   ` [dpdk-dev] [PATCH v2 2/5] eal: add accessor functions for lcore_config Stephen Hemminger
2019-04-10 17:16     ` Stephen Hemminger
2019-04-16 17:03     ` Jerin Jacob Kollanukkaran
2019-04-16 17:03       ` Jerin Jacob Kollanukkaran
2019-04-30 20:53       ` Stephen Hemminger
2019-04-30 20:53         ` Stephen Hemminger
2019-05-01  2:12         ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2019-05-01  2:12           ` Jerin Jacob Kollanukkaran
2019-05-03  7:22     ` [dpdk-dev] " David Marchand
2019-05-03  7:22       ` David Marchand
2019-04-10 17:16   ` [dpdk-dev] [PATCH v2 3/5] bus: use lcore accessor functions Stephen Hemminger
2019-04-10 17:16     ` Stephen Hemminger
2019-04-10 17:16   ` [dpdk-dev] [PATCH v2 4/5] examples/bond: use lcore accessor Stephen Hemminger
2019-04-10 17:16     ` Stephen Hemminger
2019-05-03  7:29     ` David Marchand
2019-05-03  7:29       ` David Marchand
2019-04-10 17:16   ` [dpdk-dev] [PATCH v2 5/5] app/test: use lcore accessor functions Stephen Hemminger
2019-04-10 17:16     ` Stephen Hemminger
2019-05-02 23:15   ` [dpdk-dev] [PATCH v2 0/5] make lcore_config internal Stephen Hemminger
2019-05-02 23:15     ` Stephen Hemminger
2019-05-03 17:25   ` [dpdk-dev] [PATCH v3 0/5] prepare to make lcore_config not visible in ABI Stephen Hemminger
2019-05-03 17:25     ` Stephen Hemminger
2019-05-03 17:25     ` [dpdk-dev] [PATCH v3 1/5] eal: use unsigned int in rte_lcore.h functions Stephen Hemminger
2019-05-03 17:25       ` Stephen Hemminger
2019-05-03 17:25     ` [dpdk-dev] [PATCH v3 2/5] eal: add accessor functions for lcore_config Stephen Hemminger
2019-05-03 17:25       ` Stephen Hemminger
2019-05-03 17:25     ` [dpdk-dev] [PATCH v3 3/5] bus: use lcore accessor functions Stephen Hemminger
2019-05-03 17:25       ` Stephen Hemminger
2019-05-03 17:25     ` [dpdk-dev] [PATCH v3 4/5] examples/bond: use lcore accessor Stephen Hemminger
2019-05-03 17:25       ` Stephen Hemminger
2019-05-03 17:25     ` [dpdk-dev] [PATCH v3 5/5] app/test: use lcore accessor functions Stephen Hemminger
2019-05-03 17:25       ` Stephen Hemminger
2019-05-06  7:20     ` [dpdk-dev] [PATCH v3 0/5] prepare to make lcore_config not visible in ABI David Marchand
2019-05-06  7:20       ` David Marchand
2019-05-23 13:58 ` [dpdk-dev] [PATCH v4 0/5] make lcore_config internal David Marchand
2019-05-23 13:58   ` [dpdk-dev] [PATCH v4 1/5] eal: use unsigned int in lcore API prototypes David Marchand
2019-05-23 13:58   ` [dpdk-dev] [PATCH v4 2/5] eal: add lcore accessors David Marchand
2019-05-29 22:46     ` Thomas Monjalon
2019-05-29 22:51       ` Stephen Hemminger
2019-05-30  7:31         ` David Marchand
2019-05-30  7:40           ` Thomas Monjalon
2019-05-30 10:11             ` Bruce Richardson
2019-05-30 13:39               ` Thomas Monjalon
2019-05-30 17:00                 ` David Marchand
2019-05-30 20:08                   ` Bruce Richardson
2019-05-23 13:58   ` [dpdk-dev] [PATCH v4 3/5] drivers/bus: use " David Marchand
2019-05-23 13:59   ` [dpdk-dev] [PATCH v4 4/5] examples/bond: " David Marchand
2019-05-23 13:59   ` [dpdk-dev] [PATCH v4 5/5] test: " David Marchand
2019-05-23 15:14   ` [dpdk-dev] [PATCH v4 0/5] make lcore_config internal Stephen Hemminger
2019-05-31 15:36 ` [dpdk-dev] [PATCH v5 " David Marchand
2019-05-31 15:36   ` [dpdk-dev] [PATCH v5 1/5] eal: use unsigned int in lcore API prototypes David Marchand
2019-05-31 15:36   ` [dpdk-dev] [PATCH v5 2/5] eal: add lcore accessors David Marchand
2019-05-31 15:37   ` [dpdk-dev] [PATCH v5 3/5] drivers/bus: use " David Marchand
2019-05-31 15:37   ` [dpdk-dev] [PATCH v5 4/5] examples/bond: " David Marchand
2019-05-31 15:37   ` [dpdk-dev] [PATCH v5 5/5] test: " David Marchand
2019-06-04 15:11     ` Eads, Gage
2019-06-03 10:32   ` [dpdk-dev] [PATCH v5 0/5] make lcore_config internal Thomas Monjalon
2019-06-03 20:15     ` Stephen Hemminger

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=CAJFAV8z3Se7z6u6zFAzGxJPF2zt3LetUm_pdywjWPJgL04dECA@mail.gmail.com \
    --to=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /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).