From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by dpdk.org (Postfix) with ESMTP id EF08069FC for ; Thu, 12 Jan 2017 14:22:14 +0100 (CET) Received: by mail-wm0-f50.google.com with SMTP id c85so17916082wmi.1 for ; Thu, 12 Jan 2017 05:22:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=ouA6D4hlyyTufmcEYtzm9lC/NIpeGTauFiq+MxXR2hg=; b=sUhY0V5iSrpb+7B38k7aZkGpWslXz7gMUMfAdizkdh1vvd6YRpGzr6qLk9Qp5wY4Dd PoqHAMgCy2Gc/RCaJcxI5l2wBY+KbF/QYABZoWuhMV1EtOK5BaGbQbST7s9fvaL0XIdS I+mGxAgqbEnaoHFbSjjFwEBEUHod4RfgTqfnsmlIBr5QPsZUxk3DdhHH4gpEZ0PT1bs4 OoKnPXYF1Hnr9jtdGksTlgl4xEkm0USot+uipuPmwaiendbeqfXiutHLr3LnD2OAWj7C humDhY94+5MTjNQN0Aw8FoZLafbqFZEKJikL44jga6OQnDORcJVdVGejWGTTiBvJkYVC tmQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=ouA6D4hlyyTufmcEYtzm9lC/NIpeGTauFiq+MxXR2hg=; b=XGAGbtPr9mD/o8LQ0tYaq2vw1pI64TN+tgTcCzN95rQNOr2TbSvNRqKG7G9BGqIiIk bX7fhstxTB5iK53oK2pXGMGiiZ4IVfNS0lU3qDPPImCqSj5xvuYngeGCQfUf27SegCZ6 6p0GwMpEdDDoyNCm5I2W9ZA6EWsRtuO14QpZ+biwpNdUD4YcbZoi0mF9OQqdvi95qViW 4DL+EoOo0QisTZb1qDlDxDeut95GmXjNEkc2QZCUHOwtA+gM9UE9aPU5pvROcQBfLNbR vXInamRYP2hVmPU7lTjcpte5A74H9q9lBDyH/fWRVa2sNlVnaQrHMMFXM5fBSoTiOSCB T+3Q== X-Gm-Message-State: AIkVDXJkxfhXVamjeKs/DpHuBGAjenGP2THLOl5G+lu9ZJcosZk4dEej4mP0iwTF/u/ZqBpI X-Received: by 10.28.16.211 with SMTP id 202mr2395962wmq.133.1484227334533; Thu, 12 Jan 2017 05:22:14 -0800 (PST) Received: from xps13.localnet (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id u78sm3178310wma.11.2017.01.12.05.22.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Jan 2017 05:22:13 -0800 (PST) From: Thomas Monjalon To: Remy Horton Cc: dev@dpdk.org Date: Thu, 12 Jan 2017 14:22:24 +0100 Message-ID: <1951093.Zqtj2Qm3TA@xps13> User-Agent: KMail/4.14.10 (Linux/4.5.4-1-ARCH; KDE/4.14.11; x86_64; ; ) In-Reply-To: <1484150594-3758-2-git-send-email-remy.horton@intel.com> References: <1484150594-3758-1-git-send-email-remy.horton@intel.com> <1484150594-3758-2-git-send-email-remy.horton@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib: add information metrics library 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: , X-List-Received-Date: Thu, 12 Jan 2017 13:22:15 -0000 2017-01-12 00:03, Remy Horton: > This patch adds a new information metric library that allows other > modules to register named metrics and update their values. It is > intended to be independent of ethdev, rather than mixing ethdev > and non-ethdev information in xstats. [...] > --- a/doc/api/doxy-api.conf > +++ b/doc/api/doxy-api.conf > @@ -58,6 +58,7 @@ INPUT = doc/api/doxy-api-index.md \ > lib/librte_reorder \ > lib/librte_ring \ > lib/librte_sched \ > + lib/librte_metrics \ > lib/librte_table \ > lib/librte_timer \ > lib/librte_vhost It is not in the right order. Tip: when you add an item to a list, you should ask yourself what is the order. There are 3 types of order for the lists in DPDK: - chronological (add at the end) - alphabetical - logical/semantic The game is to find the right one :) [...] > @@ -171,6 +177,7 @@ The libraries prepended with a plus sign were incremented in this version. > librte_mbuf.so.2 > librte_mempool.so.2 > librte_meter.so.1 > + + librte_metrics.so.1 > librte_net.so.1 > librte_pdump.so.1 > librte_pipeline.so.3 Right order here ;) [...] > --- /dev/null > +++ b/lib/librte_metrics/rte_metrics.h > +/** Used to indicate port-independent information */ > +#define RTE_METRICS_NONPORT -1 I do not understand this constant. Why using the word "port" to name any device? What means independent? > +/** > + * Metric name > + */ > +struct rte_metric_name { > + /** String describing metric */ > + char name[RTE_METRICS_MAX_NAME_LEN]; > +}; Why a struct for a simple string? > +/** > + * Metric name. Copy/paste typo? > + */ > +struct rte_metric_value { > + /** Numeric identifier of metric */ > + uint16_t key; How the key is bound to the name? Remember how the xstats comments were improved: http://dpdk.org/commit/6d52d1d > + /** Value for metric */ > + uint64_t value; > +}; > + > + > +/** > + * Initializes metric module. This only has to be explicitly called if you > + * intend to use rte_metrics_reg_metric() or rte_metrics_reg_metrics() from a > + * secondary process. This function must be called from a primary process. > + */ > +void rte_metrics_init(void); > + > + > +/** > + * Register a metric You need to explain what is implied in registering. I have the same comment for registering a set of metrics. [...] > +int rte_metrics_reg_metric(const char *name); > + > +/** > + * Register a set of metrics [...] > +int rte_metrics_reg_metrics(const char **names, uint16_t cnt_names); > + > +/** > + * Get metric name-key lookup table. > + * > + * @param names > + * Array of names to receive key names > + * > + * @param capacity > + * Space available in names What happens if there is not enough space? > + * @return > + * - Non-negative: Success (number of names) > + * - Negative: Failure > + */ > +int rte_metrics_get_names( > + struct rte_metric_name *names, > + uint16_t capacity);