From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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: Subject: Re: [PATCH v5 0/3] Introduce node-specific errors in graph library Cc: From: "Robin Jarry" To: , , , , , 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 > > > 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!