From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 6944345B37;
Mon, 14 Oct 2024 15:04:45 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id D31D340280;
Mon, 14 Oct 2024 15:04:44 +0200 (CEST)
Received: from us-smtp-delivery-124.mimecast.com
(us-smtp-delivery-124.mimecast.com [170.10.129.124])
by mails.dpdk.org (Postfix) with ESMTP id 843594027F
for <dev@dpdk.org>; Mon, 14 Oct 2024 15:04:43 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
s=mimecast20190719; t=1728911083;
h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
to:to:cc:cc:mime-version:mime-version:content-type:content-type:
content-transfer-encoding:content-transfer-encoding:
in-reply-to:in-reply-to:references:references;
bh=6KAH6dOCVHqubzxBHXcZfjvaIsh6NVpSwEysHisCId8=;
b=Kidnr1FTpQAAt52Sdg2IJZUVU2kOmKbSuKkGqyeGUllg1reQKYTSxnuO62GFnTjJDufwxb
UHuUJFUU0jfr61ooMkEhdGcBRX1mSJK7Ytvqf0mLG14vPPsVWtDLb7wkBhRu4SVTnMcqG1
0EG5gLaE6Xin3DgdYkfWreMBxqKEtQo=
Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com
[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS
(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id
us-mta-553-grGLHGmiM-mJN_ZNygDELw-1; Mon, 14 Oct 2024 09:04:42 -0400
X-MC-Unique: grGLHGmiM-mJN_ZNygDELw-1
Received: by mail-wm1-f70.google.com with SMTP id
5b1f17b1804b1-4312731c7bfso10748075e9.1
for <dev@dpdk.org>; Mon, 14 Oct 2024 06:04:41 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1728911081; x=1729515881;
h=in-reply-to:references:user-agent:to:from:cc:subject:message-id
:date:content-transfer-encoding:mime-version:x-gm-message-state:from
:to:cc:subject:date:message-id:reply-to;
bh=MbERoZF+vbMB8INIeUo0tIfcuHSC57uUVRz6mU665nA=;
b=eH/X1vPQbguPKbrR1GbZZXOEm2CQOfFUBnuGC6m9YVBDYWRprSOOcideljKGJFg8Tl
zCtD304TAbMpsh1lXwEJZnPnDJR5/R02BOcWxKhgLxV/NMIW/1pVLSULX0l6yrfwF6nM
vrXB4QSvatL8WJq8LxbwPAiRhjYnqYrHU2SmwjjA09TUZ8GPjjo2N6UWHGmaJO0SmD9S
3XzdfnNXe2DdapyK4oSWiwIS3f2ju6hWrlo605RGuPmMxqoMx5tXZ3wqh2O371qM9p99
s00q/JefgooXraPQ8WAlCbnsQKTRm96HAKWH9gkXfEbP0WltIt3bOe4fJMLxQdDgmGGU
ozHA==
X-Gm-Message-State: AOJu0YzMFjd5nwanez1KGhL5fAXeDkNhIIaoZNiXRZFgCkH4IljcKRsF
jij09mS+7DSMIPkpAnMKZCq2n4CxJT4Wm2Z2UiVA+9FKxu/LkVr1ihN/4guCr840RcbYnXYcnYm
ZGLEmnJLeOSj6we6gMh3kXiYYe30eN2kBsZsXPD3C
X-Received: by 2002:a05:600c:4f81:b0:428:150e:4f13 with SMTP id
5b1f17b1804b1-4311df58860mr92409795e9.33.1728911080762;
Mon, 14 Oct 2024 06:04:40 -0700 (PDT)
X-Google-Smtp-Source: AGHT+IF4SM5aYl0j6JiMtPH3h9YBDPz0HN2a05gEHDs7havk2YmlY67/182f0MqWMYz2ZZczx0tBjw==
X-Received: by 2002:a05:600c:4f81:b0:428:150e:4f13 with SMTP id
5b1f17b1804b1-4311df58860mr92409575e9.33.1728911080369;
Mon, 14 Oct 2024 06:04:40 -0700 (PDT)
Received: from localhost (2a01cb00025433006239e1f47a0b2371.ipv6.abo.wanadoo.fr.
[2a01:cb00:254:3300:6239:e1f4:7a0b:2371])
by smtp.gmail.com with ESMTPSA id
5b1f17b1804b1-430d748d7ddsm152796065e9.45.2024.10.14.06.04.39
(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);
Mon, 14 Oct 2024 06:04:40 -0700 (PDT)
Mime-Version: 1.0
Date: Mon, 14 Oct 2024 15:04:39 +0200
Message-Id: <D4VJTT5V3Y2Z.32IFMLZKRHLY6@redhat.com>
Subject: Re: [PATCH v5 0/3] Introduce node-specific errors in graph library
Cc: <dev@dpdk.org>
From: "Robin Jarry" <rjarry@redhat.com>
To: <pbhagavatula@marvell.com>, <jerinj@marvell.com>,
<ndabilpuram@marvell.com>, <kirankumark@marvell.com>,
<zhirun.yan@intel.com>, <david.marchand@redhat.com>
User-Agent: aerc/0.18.2-81-g43ce621f36dd
References: <20240816150926.5789-1-pbhagavatula@marvell.com>
<20241014115821.4204-1-pbhagavatula@marvell.com>
In-Reply-To: <20241014115821.4204-1-pbhagavatula@marvell.com>
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8; format=Flowed
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Hi Pavan,
I am resending my review here. It seems you didn't get it.
, Oct 14, 2024 at 13:58:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
>
> Introduce the ability for nodes to advertise error counters during
> registration and increment them during the node process function in
> the graph library.
> This enhancement allows for better error tracking and debugging
> capabilities within the graph framework.
>
> The number of errors and the mapping of error IDs to error descriptions
> are defined during node registration.
> If an error is encountered during the node process function while walking
> the graph, the respective error counter is incremented.
This feature could be useful to store detailed statistics per node, not=20
only for errors. It would be better to rename "errors" to "xstats".
See below for a concrete suggestion.
>
> Example:
> =09static struct rte_node_errors ip4_reassembly_errors =3D {
> =09=09.nb_errors =3D 1,
> =09=09.err_desc =3D {
> =09=09=09[0] =3D "ip4_reassembly_error",
> =09=09},
> =09};
static const struct rte_node_xstats ip4_reassembly_xstats =3D {
.xstats_num =3D 1,
.xstats_desc =3D {
[0] =3D "ip4_reassembly_error",
},
}
>
> Here, "ip4_reassembly_error" is mapped to error ID 0, and the same ID is
> used in the `ip4_reassembly_node_process` function to increment reassembl=
y
> errors.
> Depending on the node, there can be multiple such errors that can be
> updated independently and retrieved using `rte_graph_cluster_stats_get`.
>
> Example:
> +-------------------------------+---------------+---------------+--------=
------+
> |Node |calls |objs |realloc_=
count |
> +-------------------------------+---------------+---------------+--------=
------+
> |ip4_lookup |1324083 |338965248 |2 =
|
> | ip4_lookup_error | |338965496 | =
|
> |pkt_drop |1324084 |338965504 |1 =
|
> |ethdev_rx-0-0 |1324086 |338966016 |2 =
|
> |pkt_cls |1324086 |338966016 |1 =
|
> +-------------------------------+---------------+---------------+--------=
------+
>
> v2 Changes:
> - Fix compilation.
> v3 Changes:
> - Resend as 1/5 didn't make it through.
> v4 Changes:
> - Address review comments.
> - Rebase on main branch.
> v5 Changes:
> - Shrink structure member names.(Robin)
> - add rte_node_error_increment utility function. (Robin)
> - Squash patches. (Robin)
> - Update RN, DN. (David)
>
> Pavan Nikhilesh (3):
> graph: add support for node specific errors
> graph: add node error counters
I think patch 1/3 and 2/3 should be split differently. My preference=20
would be to have documentation (especially large svg images) in=20
a separate commit. Other than that, the changes in lib/graph should be=20
squashed in the same patch.
> node: add error stats for ip4 nodes
>
> doc/guides/prog_guide/graph_lib.rst | 22 +-
> .../prog_guide/img/anatomy_of_a_node.svg | 329 +++++--
> .../prog_guide/img/graph_mem_layout.svg | 921 +++++++++++++-----
> doc/guides/rel_notes/deprecation.rst | 6 -
> doc/guides/rel_notes/release_24_11.rst | 8 +
> lib/graph/graph_populate.c | 20 +-
> lib/graph/graph_private.h | 3 +
> lib/graph/graph_stats.c | 78 +-
> lib/graph/node.c | 37 +-
> lib/graph/rte_graph.h | 11 +
> lib/graph/rte_graph_worker_common.h | 23 +
> lib/graph/version.map | 7 +
> lib/node/ip4_lookup.c | 9 +
> lib/node/ip4_lookup_neon.h | 5 +
> lib/node/ip4_lookup_sse.h | 6 +
> lib/node/ip4_reassembly.c | 9 +
> lib/node/node_private.h | 8 +
> 17 files changed, 1192 insertions(+), 310 deletions(-)
To summarize changes, here is my proposal:
struct rte_node_xstats {
uint8_t xstats_num; /**< Number of xstats. */
char (*xstats_desc)[RTE_NODE_XSTATS_DESC_SIZE]; /**< Names of xstats. */
};
struct rte_node_register {
...
const struct rte_node_xstats *xstats; /**< Node specific extra statistic=
s. */
...
};
static inline void
rte_node_xstat_increment(struct rte_node *node, uint16_t stat_id, uint64_t=
value)
{
#ifdef RTE_LIBRTE_GRAPH_STATS
uint64_t *errors =3D RTE_PTR_ADD(node, node->err_off);
errors[err_id] +=3D value;
#else
RTE_SET_USED(node);
RTE_SET_USED(err_id);
RTE_SET_USED(value);
#endif
}
struct __rte_cache_aligned rte_graph_cluster_node_stats {
...
uint8_t xstats_num;
char (*xstats_desc)[RTE_NODE_XSTATS_DESC_SIZE];
uint64_t *xstats_val;
...
};
Let me know what you think. Thanks!