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 4615643D78; Fri, 29 Mar 2024 08:45:05 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 10C0D4014F; Fri, 29 Mar 2024 08:45:05 +0100 (CET) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by mails.dpdk.org (Postfix) with ESMTP id 8C01F40042 for ; Fri, 29 Mar 2024 08:45:03 +0100 (CET) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.17.1.24/8.17.1.24) with ESMTP id 42T5sTpT031604; Fri, 29 Mar 2024 00:44:57 -0700 Received: from nam02-bn1-obe.outbound.protection.outlook.com (mail-bn1nam02lp2041.outbound.protection.outlook.com [104.47.51.41]) by mx0a-0016f401.pphosted.com (PPS) with ESMTPS id 3x5qw8r9yj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 29 Mar 2024 00:44:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=PjM9wEfNSuyzE4INnVRCo/ACNH9h7bdKn84waBRcWjIhGvB0u0u+7tz16sfoP1q5bv7n3aMwrwleruJnIAcS6I1xmoJIvrqixFBTqGjHCNwRvbgfWDILuKwMUbmlqGgUG2dNTR+juAnwViJWKD7dGXwCqy69J6aA3qyGePJRsT/IwqW4fr3h192W/iNnhwc3uyoz3viFBECGtxMUuN+i1aqY05lfMRFzsX5S2Gw/w3kl2p4XpK2ZKQXbklrFMZAV46joHxDxZ4w8VM1s31Ua1CMF8Cb9xCRMVUCCobxNkisbuDdP0rEQ6syFAyZZPbt0LMYdWfiujxGrYmxWDX0/Lw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=GnFviawdRqG81MCc9d7V/eI7CE7VFZACrgumA7CY06E=; b=f1x34NLKzcBakoH5KFEwaVA+3rYOEcFx6Rynsz26GRDJVoEIq07zi73s2k96ZQtJYtV/GIoqVQjK5QMP03QXzyQeiQRWlmofd6dJiJilCvmq57kGyX2uhj4DUchD0/lVpfm4X5DX+3tqjG1UbNspbstLRYUmXMVuBhiJnqOJLroDYtgj2OR8DoslcttVy9bsVXWBxQJLhjN5gPQWvDhqeUXZfbNGXtPTjPKBsZA1HDmRQF1WCtPzKwY5fJchECVM3svlP1zFQPqKcwdnU/m3SIiZTtcFGBxgVo1sFqntG97vhSSVl2DETjI0t3E9ETc5MqhFkSI/jZRE0TvZsjoLMA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GnFviawdRqG81MCc9d7V/eI7CE7VFZACrgumA7CY06E=; b=M2CdD4cQVnr9u7wyckSABpzBsl7M9DG97Xflt6kcxxpmwRIMG327UwYNEsUIAkT0GPOq8DYugmrCayHhc/U3OZnsCq9AaVd+dwCbwBvKVJKqjmjhca6KlQ1kD9KSk0FDHbyWvZE7AIK8wfUaIpSwR6TdrOQZfgtHZtyZBgXVFjc= Received: from PH0PR18MB5071.namprd18.prod.outlook.com (2603:10b6:510:16b::15) by SA0PR18MB3470.namprd18.prod.outlook.com (2603:10b6:806:95::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7409.40; Fri, 29 Mar 2024 07:44:53 +0000 Received: from PH0PR18MB5071.namprd18.prod.outlook.com ([fe80::b2bd:bab4:73f2:6c52]) by PH0PR18MB5071.namprd18.prod.outlook.com ([fe80::b2bd:bab4:73f2:6c52%7]) with mapi id 15.20.7409.023; Fri, 29 Mar 2024 07:44:52 +0000 From: Kiran Kumar Kokkilagadda To: Robin Jarry , "dev@dpdk.org" , Jerin Jacob , Nithin Kumar Dabilpuram , Zhirun Yan Subject: RE: [EXTERNAL] [PATCH] graph: avoid accessing graph list when getting stats Thread-Topic: [EXTERNAL] [PATCH] graph: avoid accessing graph list when getting stats Thread-Index: AQHafsymvtuMclbxTU2KUr2fiArQCrFOW5pQ Date: Fri, 29 Mar 2024 07:44:52 +0000 Message-ID: References: <20240325155303.770468-2-rjarry@redhat.com> In-Reply-To: <20240325155303.770468-2-rjarry@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: PH0PR18MB5071:EE_|SA0PR18MB3470:EE_ x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Ih7mB+HRCHv81Mp+t7xAx5/MU2wtCvLN0ZyDbZry74BsPh2tCyXaDAnyzRa0wofFLdesuLYhqJkulUKzD7hqAwjfKcOgRXhKsGjWNrtpd144mSOTkIYMKmlQvsOg19mwubP/m0EBDMUS8Ehm3nPJIBVYieW+/CpDjiE8+hKdV5/o5xyplw6F7kAhBCBYHcP7Z9gee7E0V0iIKn5P9UBQqynFqx6NLtMxPUKeKD0MZf5CJpT3X/yf2G7CWhF2bA9jKpBZdPPQmKlY3oWFB4BjBpnOsg0aCrk49HMYAtyUVqLW5U30QL3EY6ds8ciort3LBon7Aqg20BV5ZCMma/3Rt7/K9YFwG8+FgnJXnwnpFzSxz1GWT/aVz9Opc82w10QbJCSB7LBj1tEKEDWBD1VzVuEzG/Y9BNkQSfAUZDDgrBGzdIXnk5AXwjsysIemTG5tS3RZsBc1GmMbzpBnwk5CiEIZ+MRe5J0Lu7SyQ0zH2RCy6SA/ZavMZFpQFTcumVbgFt+HmDpntglmfWtCBHjtRA4HmJvwKmn2oJjhRx39+ULWuJr3JKH/aALiUOmdlS1KLFTFV3KHnDiu803CCR+9FLd7hhUW9PVIPQSv74UWoWisZUM8qtXmJJG53ZJ6pJ5SPKTEsNlMDnxgL3BBPVGdBNUGKGMagR1M0p5tBBW23fs= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR18MB5071.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230031)(376005)(1800799015)(366007); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?UgCHX7lsqKbMQASg+2O/7V7C4sIVrtgSpl95p/ONTbc+B1NFXcGO4abW4ieQ?= =?us-ascii?Q?YezcnKJ9EiyWgQNz6s47JFVOkQBWll3RTOai/R12UKDjb8RRDisTQxx1oIV/?= =?us-ascii?Q?sq1ybWVwwTtqJBkmJvHFYyO/EV0WxQsOhg7Hi4VeGEFHHVoXVjRd1kh/7psE?= =?us-ascii?Q?UtRF5Q8bS7R2KxzVSzPYedLLXU2DVbaP2iqQ5KxGYlz+2JA90FKDDeEtO2Ba?= =?us-ascii?Q?K7QiOEx+sxCbuzh13uMsu/6x1CoftupJa4TGRpfmR6uZGUhU+QZtsA68lhur?= =?us-ascii?Q?zIJjvy0lYaCw7omygsusZptdWAsKBNKeUd0rVLpDpnFM6zQ2+nIFwdWrAziR?= =?us-ascii?Q?s/njOpe3vx9T7wP5u+hz7psqXoSxdt4u/1fjLsjaKueEUyvOtLIIov22u+Xv?= =?us-ascii?Q?7EYjNI7Fci89gLNiVpcJDYTjwwKdXv2D/Fgy1/Y98q8Oh6nQZ0SnYuOkAEzM?= =?us-ascii?Q?K9X1oECkt5FjTxCYASh3GZvnACHcy54xVKb4lXnEhYQj67Q6l4RrLUzvJseT?= =?us-ascii?Q?1vII9JCxytgJI+CoUd/fHov+30aRPHxSiTe+EFFOD19Hvh3d335DRsrCan5g?= =?us-ascii?Q?9nFKNJ8ROqu7lVnDnyVPItLHRu5U7EYMeAWFkJMemjIq/K71ISEIJaZP58Sv?= =?us-ascii?Q?nY1JoboOepYjd5Sx6Bzq1t8gSR7ZohKZMwP0QCUO5E9xJVG+MLBWRBhe+x1v?= =?us-ascii?Q?nb9HQjvHCjl4A0/jSWifsBWX+NG2M6X/BNHL/DdSQNix3E0BazPf090XoRr7?= =?us-ascii?Q?kzbSBd2eQo0/yndDHqRgd+BXg6oM4xxoU8JB+bHftBaQLLjnc7L+4IB0geKS?= =?us-ascii?Q?XyH8bYgBl89mKMXsx49jJpl/FaD/q2HwynpQxAc5w+R9Uq1ltvuNRbWnlARa?= =?us-ascii?Q?BmBJkTAYoVnSnxa3lii9bxa75sM+y/CkKzAwyKlIuAAIOZGh8qyDmI+Vgf+p?= =?us-ascii?Q?WNR536guzRD6AAy2L2HHIGPb/U26kox1lDAP/giWgf/HFC1f6l9O4Yx5ky3P?= =?us-ascii?Q?2RKX5VLYXuTm9DUQEnF/17uKeWD6sUYNoUl7FFzVmJfKNQs2Xn5tHpxMYwfT?= =?us-ascii?Q?UAOZgKxk8DmppimMOVjofRM0jptoe2SLax24mqH+icptNw/fe1c0NXiIPvC1?= =?us-ascii?Q?lxENmWOIe4xxvW0setiDultnDZTNxzmHbzjfhHcboVezpji50UhPJMUpSfTS?= =?us-ascii?Q?hew9LNg/CdJf4FAH0pcADP7N67ipXQYIeOd0A6/nOfwe70J6qGHVYix3DBIw?= =?us-ascii?Q?gNpOBb61+9mMxW2XtOneebiiE2xMpMQ546E8+x2N5L4y3u2WuRnU0SsIdtY7?= =?us-ascii?Q?T5XG21fSJwAf8mE45Uo00p6rXEHj9x+XNkSCrHE6wTFCF4nmtrVmMtHjcB7M?= =?us-ascii?Q?LHST6CfIIa/d6SABIzpr70yCwUyU7X/DdwfcUyD1zAgM7f+Z0s462ljMd+kY?= =?us-ascii?Q?oIuAAPj3ZZDmfCA4JQ3jxwunRHNvAcNWjzFoi9NnEhg7cxagK3VUvgrPC512?= =?us-ascii?Q?HcLsum9sTOjqJd6JIUfDZ7BqdR3NM9lLD7cI7vfc1NflUa3ERx20EbouobmA?= =?us-ascii?Q?hWCza/7YS2bqbjgrIsKehwgIDbhMLTfssebCoHkm?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: marvell.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR18MB5071.namprd18.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 48aba0fe-735b-4393-943c-08dc4fc41ece X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Mar 2024 07:44:52.8779 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 5x1H1tZD6l+3cTAIcG9LyKnUX1wcOLFDKzjF+3tGGZEflJ9Z28KN8up9A27HQRtkh6zoLxMhf9K9HOinpoaViA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR18MB3470 X-Proofpoint-ORIG-GUID: L7rDZCGG0_Zsm9ZIwhCK5LvrTyS7KlKa X-Proofpoint-GUID: L7rDZCGG0_Zsm9ZIwhCK5LvrTyS7KlKa X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2024-03-29_06,2024-03-28_01,2023-05-22_02 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 > -----Original Message----- > From: Robin Jarry > Sent: Monday, March 25, 2024 9:23 PM > To: dev@dpdk.org; Jerin Jacob ; Kiran Kumar > Kokkilagadda ; Nithin Kumar Dabilpuram > ; Zhirun Yan > Subject: [EXTERNAL] [PATCH] graph: avoid accessing graph list when gettin= g > stats >=20 > Prioritize security for external emails: Confirm sender and content safet= y > before clicking links or opening attachments >=20 > ---------------------------------------------------------------------- > In rte_graph_cluster_stats_get, the walk model of the first graph is chec= ked > to determine if multi-core dispatch specific counters should be updated o= r > not. This global list is accessed without any locks. >=20 > If the global list is modified by another thread while > rte_graph_cluster_stats_get is called, it can result in undefined behavio= ur. >=20 > Adding a lock would make it impossible to call rte_graph_cluster_stats_ge= t in > packet processing code paths. Avoid accessing the global list instead by > storing a bool field in the private rte_graph_cluster_stats structure. >=20 > Also update the default callback to avoid accessing the global list and u= se a > different default callback depending on the graph model. >=20 > Signed-off-by: Robin Jarry > --- > lib/graph/graph_stats.c | 60 ++++++++++++++++++++++++++--------------- > 1 file changed, 39 insertions(+), 21 deletions(-) >=20 > diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c index > 2fb808b21ec5..f8048e08be8a 100644 > --- a/lib/graph/graph_stats.c > +++ b/lib/graph/graph_stats.c > @@ -34,6 +34,7 @@ struct __rte_cache_aligned rte_graph_cluster_stats { > uint32_t cluster_node_size; /* Size of struct cluster_node */ > rte_node_t max_nodes; > int socket_id; > + bool dispatch; > void *cookie; > size_t sz; >=20 > @@ -74,17 +75,16 @@ print_banner_dispatch(FILE *f) } >=20 > static inline void > -print_banner(FILE *f) > +print_banner(FILE *f, bool dispatch) > { > - if > (rte_graph_worker_model_get(STAILQ_FIRST(graph_list_head_get())->graph) > =3D=3D > - RTE_GRAPH_MODEL_MCORE_DISPATCH) > + if (dispatch) > print_banner_dispatch(f); > else > print_banner_default(f); > } >=20 > static inline void > -print_node(FILE *f, const struct rte_graph_cluster_node_stats *stat) > +print_node(FILE *f, const struct rte_graph_cluster_node_stats *stat, > +bool dispatch) > { > double objs_per_call, objs_per_sec, cycles_per_call, ts_per_hz; > const uint64_t prev_calls =3D stat->prev_calls; @@ -104,8 +104,7 @@ > print_node(FILE *f, const struct rte_graph_cluster_node_stats *stat) > objs_per_sec =3D ts_per_hz ? (objs - prev_objs) / ts_per_hz : 0; > objs_per_sec /=3D 1000000; >=20 > - if > (rte_graph_worker_model_get(STAILQ_FIRST(graph_list_head_get())->graph) > =3D=3D > - RTE_GRAPH_MODEL_MCORE_DISPATCH) { > + if (dispatch) { > fprintf(f, > "|%-31s|%-15" PRIu64 "|%-15" PRIu64 "|%-15" PRIu64 > "|%-15" PRIu64 "|%-15" PRIu64 > @@ -123,20 +122,17 @@ print_node(FILE *f, const struct > rte_graph_cluster_node_stats *stat) } >=20 > static int > -graph_cluster_stats_cb(bool is_first, bool is_last, void *cookie, > +graph_cluster_stats_cb(bool dispatch, bool is_first, bool is_last, void > +*cookie, > const struct rte_graph_cluster_node_stats *stat) { > FILE *f =3D cookie; > - int model; > - > - model =3D > rte_graph_worker_model_get(STAILQ_FIRST(graph_list_head_get())->graph); >=20 > if (unlikely(is_first)) > - print_banner(f); > + print_banner(f, dispatch); > if (stat->objs) > - print_node(f, stat); > + print_node(f, stat, dispatch); > if (unlikely(is_last)) { > - if (model =3D=3D RTE_GRAPH_MODEL_MCORE_DISPATCH) > + if (dispatch) > boarder_model_dispatch(); > else > boarder(); > @@ -145,6 +141,20 @@ graph_cluster_stats_cb(bool is_first, bool is_last, > void *cookie, > return 0; > }; >=20 > +static int > +graph_cluster_stats_cb_rtc(bool is_first, bool is_last, void *cookie, > + const struct rte_graph_cluster_node_stats *stat) { > + return graph_cluster_stats_cb(false, is_first, is_last, cookie, stat); > +}; > + > +static int > +graph_cluster_stats_cb_dispatch(bool is_first, bool is_last, void *cooki= e, > + const struct rte_graph_cluster_node_stats > *stat) { > + return graph_cluster_stats_cb(true, is_first, is_last, cookie, stat); > +}; > + > static struct rte_graph_cluster_stats * stats_mem_init(struct cluster *= cluster, > const struct rte_graph_cluster_stats_param *prm) @@ -157,8 > +167,16 @@ stats_mem_init(struct cluster *cluster, >=20 > /* Fix up callback */ > fn =3D prm->fn; > - if (fn =3D=3D NULL) > - fn =3D graph_cluster_stats_cb; > + if (fn =3D=3D NULL) { > + for (int i =3D 0; i < cluster->nb_graphs; i++) { > + const struct rte_graph *graph =3D cluster->graphs[i]- > >graph; > + if (graph->model =3D=3D > RTE_GRAPH_MODEL_MCORE_DISPATCH) > + fn =3D graph_cluster_stats_cb_dispatch; > + else > + fn =3D graph_cluster_stats_cb_rtc; > + break; > + } > + } >=20 Do we need loop here? Just take cluster->graphs[0] and remove break? > cluster_node_size =3D sizeof(struct cluster_node); > /* For a given cluster, max nodes will be the max number of graphs > */ @@ -350,6 +368,8 @@ rte_graph_cluster_stats_create(const struct > rte_graph_cluster_stats_param *prm) > if (stats_mem_populate(&stats, graph_fp, > graph_node)) > goto realloc_fail; > } > + if (graph->graph->model =3D=3D > RTE_GRAPH_MODEL_MCORE_DISPATCH) > + stats->dispatch =3D true; > } >=20 > /* Finally copy to hugepage memory to avoid pressure on rte_realloc > */ @@ -375,20 +395,18 @@ rte_graph_cluster_stats_destroy(struct > rte_graph_cluster_stats *stat) } >=20 > static inline void > -cluster_node_arregate_stats(struct cluster_node *cluster) > +cluster_node_arregate_stats(struct cluster_node *cluster, bool > +dispatch) > { > uint64_t calls =3D 0, cycles =3D 0, objs =3D 0, realloc_count =3D 0; > struct rte_graph_cluster_node_stats *stat =3D &cluster->stat; > uint64_t sched_objs =3D 0, sched_fail =3D 0; > struct rte_node *node; > rte_node_t count; > - int model; >=20 > - model =3D > rte_graph_worker_model_get(STAILQ_FIRST(graph_list_head_get())->graph); > for (count =3D 0; count < cluster->nb_nodes; count++) { > node =3D cluster->nodes[count]; >=20 > - if (model =3D=3D RTE_GRAPH_MODEL_MCORE_DISPATCH) { > + if (dispatch) { > sched_objs +=3D node->dispatch.total_sched_objs; > sched_fail +=3D node->dispatch.total_sched_fail; > } > @@ -403,7 +421,7 @@ cluster_node_arregate_stats(struct cluster_node > *cluster) > stat->objs =3D objs; > stat->cycles =3D cycles; >=20 > - if (model =3D=3D RTE_GRAPH_MODEL_MCORE_DISPATCH) { > + if (dispatch) { > stat->dispatch.sched_objs =3D sched_objs; > stat->dispatch.sched_fail =3D sched_fail; > } > @@ -433,7 +451,7 @@ rte_graph_cluster_stats_get(struct > rte_graph_cluster_stats *stat, bool skip_cb) > cluster =3D stat->clusters; >=20 > for (count =3D 0; count < stat->max_nodes; count++) { > - cluster_node_arregate_stats(cluster); > + cluster_node_arregate_stats(cluster, stat->dispatch); > if (!skip_cb) > rc =3D stat->fn(!count, (count =3D=3D stat->max_nodes - 1), > stat->cookie, &cluster->stat); > -- > 2.44.0