From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9DCCBA00BE; Fri, 12 Jun 2020 15:02:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5F0441BEA6; Fri, 12 Jun 2020 14:58:18 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 763A01BE81 for ; Fri, 12 Jun 2020 14:58:14 +0200 (CEST) IronPort-SDR: 0M7JIc6afYIj9GYF/zkQsgPqYiWRcAJnWvmL5nAoDUIogTVfMm6rHONz2E4glQ4vVB3ns7qhz2 x/5Bqpwi2/2A== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Jun 2020 05:58:13 -0700 IronPort-SDR: H9fM5EO8BZlBxdKkyd0X3KzhPqyE7pO5XZxsPugxHBacaoC/a1nLUwVm8JPeflFlQlPdmK5IkG 7D6x5oP5yxng== X-IronPort-AV: E=Sophos;i="5.73,503,1583222400"; d="scan'208";a="474202595" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.13.196]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 12 Jun 2020 05:58:11 -0700 Date: Fri, 12 Jun 2020 13:58:08 +0100 From: Bruce Richardson To: Ciara Power Cc: kevin.laatz@intel.com, thomas@monjalon.net, ferruh.yigit@intel.com, arybchenko@solarflare.com, keith.wiles@intel.com, dev@dpdk.org Message-ID: <20200612125808.GA1607@bricha3-MOBL.ger.corp.intel.com> References: <20200612105344.15383-1-ciara.power@intel.com> <20200612105344.15383-2-ciara.power@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200612105344.15383-2-ciara.power@intel.com> Subject: Re: [dpdk-dev] [RFC 1/2] telemetry: support some recursive data objects X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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.