DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Ciara Power <ciara.power@intel.com>
Cc: kevin.laatz@intel.com, thomas@monjalon.net,
	ferruh.yigit@intel.com, arybchenko@solarflare.com,
	keith.wiles@intel.com, dev@dpdk.org
Subject: Re: [dpdk-dev] [RFC 1/2] telemetry: support some recursive data objects
Date: Fri, 12 Jun 2020 13:58:08 +0100	[thread overview]
Message-ID: <20200612125808.GA1607@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20200612105344.15383-2-ciara.power@intel.com>

On Fri, Jun 12, 2020 at 11:53:43AM +0100, Ciara Power wrote:
> Dict data objects now support uint64_t array data object values.
> Only one level of recursion supported.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> ---
>  lib/librte_telemetry/rte_telemetry.h          | 27 +++++++++++++++
>  .../rte_telemetry_version.map                 |  2 ++
>  lib/librte_telemetry/telemetry.c              | 34 +++++++++++++++++++
>  lib/librte_telemetry/telemetry_data.c         | 18 ++++++++++
>  lib/librte_telemetry/telemetry_data.h         |  3 ++
>  lib/librte_telemetry/telemetry_json.h         | 17 ++++++++++
>  6 files changed, 101 insertions(+)
> 
> diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
> index 2c3c96cf7..dc18c34d0 100644
> --- a/lib/librte_telemetry/rte_telemetry.h
> +++ b/lib/librte_telemetry/rte_telemetry.h
> @@ -44,6 +44,7 @@ enum rte_tel_value_type {
>  	RTE_TEL_STRING_VAL, /** a string value */
>  	RTE_TEL_INT_VAL,    /** a signed 32-bit int value */
>  	RTE_TEL_U64_VAL,    /** an unsigned 64-bit int value */
> +	RTE_TEL_DATA_VAL,   /** a rte_tel_data pointer value */
>  };
>  
>  /**
> @@ -188,6 +189,22 @@ int
>  rte_tel_data_add_dict_u64(struct rte_tel_data *d,
>  		const char *name, uint64_t val);
>  
> +/**
> + * Add a data object pointer to a dictionary.
> + * The dict must have been started by rte_tel_data_start_dict().
> + *
> + * @param d
> + *   The data structure passed to the callback
> + * @param x
> + *   The data pointer to be returned in the dictionary
> + * @return
> + *   0 on success, negative errno on error
> + */
> +__rte_experimental
> +int
> +rte_tel_data_add_dict_data(struct rte_tel_data *d, const char *name,
> +		struct rte_tel_data *val);
> +
>  /**
>   * This telemetry callback is used when registering a telemetry command.
>   * It handles getting and formatting information to be returned to telemetry
> @@ -253,4 +270,14 @@ int
>  rte_telemetry_init(const char *runtime_dir, rte_cpuset_t *cpuset,
>  		const char **err_str);
>  
> +/**
> + * Get the size of the rte_tel_data struct.
> + *
> + * @return
> + *  size_t of the struct
> + */
> +__rte_experimental
> +size_t
> +rte_tel_get_data_size(void);
> +
Thanks for this work. Thinking about this, the biggest issue I believe is
always going to be the memory management aspect of it. It seems here that
you are providing an API to allow the memory allocation of the additional
object to be done by the callback itself, but that leaves open a problem of
freeing the memory again - how does the telemetry library know what
allocation function was used, so it knows to free it appropriately? This
could be done by function pointer passed in by when adding the data
element, but I think that is clunky.

Perhaps a better approach here might be to instead add APIs for data alloc
and free, so that there is only one way to manage the memory. That would
remove the need for this size function.


  reply	other threads:[~2020-06-12 13:02 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-12 10:53 [dpdk-dev] [RFC 0/2] add basic ethdev stats with data object recursion Ciara Power
2020-06-12 10:53 ` [dpdk-dev] [RFC 1/2] telemetry: support some recursive data objects Ciara Power
2020-06-12 12:58   ` Bruce Richardson [this message]
2020-06-12 13:07   ` Bruce Richardson
2020-06-12 13:14     ` Bruce Richardson
2020-06-12 10:53 ` [dpdk-dev] [RFC 2/2] ethdev: add basic stats for telemetry Ciara Power
2020-06-12 13:10   ` Bruce Richardson
2020-06-24 13:48 ` [dpdk-dev] [PATCH v2 0/2] add basic ethdev stats with data object recursion Ciara Power
2020-06-24 13:48   ` [dpdk-dev] [PATCH v2 1/2] telemetry: support array values in data objects Ciara Power
2020-06-24 15:09     ` Bruce Richardson
2020-06-24 15:18     ` Bruce Richardson
2020-06-24 13:48   ` [dpdk-dev] [PATCH v2 2/2] ethdev: add basic stats for telemetry Ciara Power
2020-06-24 15:19     ` Bruce Richardson
2020-07-02 10:19 ` [dpdk-dev] [PATCH v3 0/2] add basic ethdev stats with data object recursion Ciara Power
2020-07-02 10:19   ` [dpdk-dev] [PATCH v3 1/2] telemetry: support array values in data objects Ciara Power
2020-07-07  7:15     ` Thomas Monjalon
2020-07-02 10:19   ` [dpdk-dev] [PATCH v3 2/2] ethdev: add common stats for telemetry Ciara Power
2020-07-13 14:23 ` [dpdk-dev] [PATCH v4 0/2] add basic ethdev stats with data object recursion Ciara Power
2020-07-13 14:23   ` [dpdk-dev] [PATCH v4 1/2] telemetry: support array values in data objects Ciara Power
2020-07-13 14:23   ` [dpdk-dev] [PATCH v4 2/2] ethdev: add common stats for telemetry Ciara Power
2020-07-15 12:29 ` [dpdk-dev] [PATCH v5 0/3] add basic ethdev stats with data object recursion Ciara Power
2020-07-15 12:29   ` [dpdk-dev] [PATCH v5 1/3] telemetry: support array values in data objects Ciara Power
2020-07-15 12:29   ` [dpdk-dev] [PATCH v5 2/3] test/test_telemetry_data: add unit tests for data to JSON Ciara Power
2020-07-17 11:53     ` Bruce Richardson
2020-07-15 12:29   ` [dpdk-dev] [PATCH v5 3/3] ethdev: add common stats for telemetry Ciara Power
2020-07-20 11:19 ` [dpdk-dev] [PATCH v6 0/3] add basic ethdev stats with data object recursion Ciara Power
2020-07-20 11:19   ` [dpdk-dev] [PATCH v6 1/3] telemetry: support array values in data objects Ciara Power
2020-07-20 11:19   ` [dpdk-dev] [PATCH v6 2/3] test/test_telemetry_data: add unit tests for data to JSON Ciara Power
2020-07-20 13:08     ` Bruce Richardson
2020-07-20 11:19   ` [dpdk-dev] [PATCH v6 3/3] ethdev: add common stats for telemetry Ciara Power
2020-07-20 14:04 ` [dpdk-dev] [PATCH v7 0/3] add basic ethdev stats with data object recursion Ciara Power
2020-07-20 14:04   ` [dpdk-dev] [PATCH v7 1/3] telemetry: support array values in data objects Ciara Power
2020-07-20 14:04   ` [dpdk-dev] [PATCH v7 2/3] test/test_telemetry_data: add unit tests for data to JSON Ciara Power
2020-07-20 14:32     ` Bruce Richardson
2020-07-20 14:04   ` [dpdk-dev] [PATCH v7 3/3] ethdev: add common stats for telemetry Ciara Power
2020-08-21 12:51 ` [dpdk-dev] [PATCH v8 0/3] add basic ethdev stats with data object recursion Ciara Power
2020-08-21 12:51   ` [dpdk-dev] [PATCH v8 1/3] telemetry: support array values in data objects Ciara Power
2020-08-21 12:51   ` [dpdk-dev] [PATCH v8 2/3] test/test_telemetry_data: add unit tests for data to JSON Ciara Power
2020-08-21 12:51   ` [dpdk-dev] [PATCH v8 3/3] ethdev: add common stats for telemetry Ciara Power
2020-09-23 11:12 ` [dpdk-dev] [PATCH v9 0/3] add basic ethdev stats with data object recursion Ciara Power
2020-09-23 11:12   ` [dpdk-dev] [PATCH v9 1/3] telemetry: support array values in data objects Ciara Power
2020-09-23 11:12   ` [dpdk-dev] [PATCH v9 2/3] test/test_telemetry_data: add unit tests for data to JSON Ciara Power
2020-09-23 11:12   ` [dpdk-dev] [PATCH v9 3/3] ethdev: add common stats for telemetry Ciara Power
2020-10-06 20:56   ` [dpdk-dev] [PATCH v9 0/3] add basic ethdev stats with data object recursion Thomas Monjalon

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=20200612125808.GA1607@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=ciara.power@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=keith.wiles@intel.com \
    --cc=kevin.laatz@intel.com \
    --cc=thomas@monjalon.net \
    /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).