From patchwork Thu Apr 18 20:02:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tyler Retzlaff X-Patchwork-Id: 139498 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id CF64243EA8; Thu, 18 Apr 2024 22:03:01 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 76D9A402F0; Thu, 18 Apr 2024 22:02:49 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id E2A3A400D6 for ; Thu, 18 Apr 2024 22:02:44 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 1F43420FD933; Thu, 18 Apr 2024 13:02:43 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1F43420FD933 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1713470564; bh=ekuA32pKWJcFcg5x5MrrHxsjFybKilu2BvKjLDDaR+c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GNmeN6wcnPEe4TKmoULvAqWeyakEl1xaQoAued2nFImqVCw2BzSrmAuluLGAqlWW4 AnKtqxDbohHzh+h05BtH9t+7fxWTe/9ynIsjmIf8dp4owUE+7fZQkAUwkR3qnS+haR v7fKCJPAXC9uAAABpPUUnGYrlcpFz9YXxFSrfeSg= From: Tyler Retzlaff To: dev@dpdk.org Cc: =?utf-8?q?Morten_Br=C3=B8rup?= , Akhil Goyal , Aman Singh , Anatoly Burakov , Andrew Rybchenko , Bruce Richardson , Chengwen Feng , Dariusz Sosnowski , Dmitry Kozlyuk , Fan Zhang , Ferruh Yigit , Harman Kalra , Harry van Haaren , Honnappa Nagarahalli , Jiayu Hu , Jingjing Wu , Kevin Laatz , Konstantin Ananyev , Matan Azrad , Ori Kam , Pallavi Kadam , Reshma Pattan , Sameh Gobriel , Suanming Mou , Thomas Monjalon , Tyler Retzlaff , Viacheslav Ovsiienko , Vladimir Medvedkin , Volodymyr Fialko , Yipeng Wang , Konstantin Ananyev Subject: [PATCH v2 04/19] ethdev: remove use of VLAs for Windows built code Date: Thu, 18 Apr 2024 13:02:27 -0700 Message-Id: <1713470562-17415-5-git-send-email-roretzla@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1713470562-17415-1-git-send-email-roretzla@linux.microsoft.com> References: <1713397319-26135-1-git-send-email-roretzla@linux.microsoft.com> <1713470562-17415-1-git-send-email-roretzla@linux.microsoft.com> MIME-Version: 1.0 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 From: Konstantin Ananyev 1) ./lib/ethdev/rte_ethdev.c:3244:16 : warning: ISO C90 forbids variable length array ‘xstats_names’ 2) ./lib/ethdev/rte_ethdev.c:3345:17 : warning: ISO C90 forbids variable length array ‘ids_copy’ 3) ./lib/ethdev/rte_ethdev.c:3538:16 : warning: ISO C90 forbids variable length array ‘xstats’ 4) ./lib/ethdev/rte_ethdev.c:3554:17 : warning: ISO C90 forbids variable length array ‘ids_copy’ For 1) and 3) - just replaced VLA with arrays allocated from heap. As I understand xstats extraction belongs to control-path, so extra calloc/free is hopefully acceptable. Also ethdev xstats already doing that within rte_eth_xstats_get_id_by_name(). For 2) and 4) changed the code to use fixed size array and call appropriate devops function several times, if needed. Signed-off-by: Konstantin Ananyev --- lib/ethdev/rte_ethdev.c | 183 ++++++++++++++++++++++++++++++------------------ 1 file changed, 113 insertions(+), 70 deletions(-) diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index f1c658f..e462f3d 100644 --- a/lib/ethdev/rte_ethdev.c +++ b/lib/ethdev/rte_ethdev.c @@ -36,6 +36,8 @@ #include "ethdev_trace.h" #include "sff_telemetry.h" +#define ETH_XSTATS_ITER_NUM 0x100 + struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS]; /* public fast-path API */ @@ -3215,7 +3217,8 @@ enum { rte_eth_xstats_get_id_by_name(uint16_t port_id, const char *xstat_name, uint64_t *id) { - int cnt_xstats, idx_xstat; + int cnt_xstats, idx_xstat, rc; + struct rte_eth_xstat_name *xstats_names; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); @@ -3241,26 +3244,33 @@ enum { } /* Get id-name lookup table */ - struct rte_eth_xstat_name xstats_names[cnt_xstats]; + xstats_names = calloc(cnt_xstats, sizeof(xstats_names[0])); + if (xstats_names == NULL) { + RTE_ETHDEV_LOG_LINE(ERR, "Can't allocate memory"); + return -ENOMEM; + } if (cnt_xstats != rte_eth_xstats_get_names_by_id( port_id, xstats_names, cnt_xstats, NULL)) { RTE_ETHDEV_LOG_LINE(ERR, "Cannot get xstats lookup"); + free(xstats_names); return -1; } + rc = -EINVAL; for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) { if (!strcmp(xstats_names[idx_xstat].name, xstat_name)) { *id = idx_xstat; rte_eth_trace_xstats_get_id_by_name(port_id, xstat_name, *id); - - return 0; + rc = 0; + break; }; } - return -EINVAL; + free(xstats_names); + return rc; } /* retrieve basic stats names */ @@ -3306,6 +3316,38 @@ enum { return cnt_used_entries; } +static int +eth_xstats_get_by_name_by_id(struct rte_eth_dev *dev, const uint64_t *ids, + struct rte_eth_xstat_name *xstats_names, uint32_t size, + uint32_t basic_count) +{ + int32_t rc; + uint32_t i, k, m, n; + uint64_t ids_copy[ETH_XSTATS_ITER_NUM]; + + m = 0; + for (n = 0; n != size; n += k) { + + k = RTE_MIN(size - n, RTE_DIM(ids_copy)); + + /* + * Convert ids to xstats ids that PMD knows. + * ids known by user are basic + extended stats. + */ + for (i = 0; i < k; i++) + ids_copy[i] = ids[n + i] - basic_count; + + rc = (*dev->dev_ops->xstats_get_names_by_id)(dev, ids_copy, + xstats_names + m, k); + if (rc < 0) + return rc; + m += rc; + } + + return m; +} + + /* retrieve ethdev extended statistics names */ int rte_eth_xstats_get_names_by_id(uint16_t port_id, @@ -3313,9 +3355,8 @@ enum { uint64_t *ids) { struct rte_eth_xstat_name *xstats_names_copy; - unsigned int no_basic_stat_requested = 1; - unsigned int no_ext_stat_requested = 1; unsigned int expected_entries; + unsigned int nb_basic_stats; unsigned int basic_count; struct rte_eth_dev *dev; unsigned int i; @@ -3341,27 +3382,18 @@ enum { if (ids && !xstats_names) return -EINVAL; - if (ids && dev->dev_ops->xstats_get_names_by_id != NULL && size > 0) { - uint64_t ids_copy[size]; - - for (i = 0; i < size; i++) { - if (ids[i] < basic_count) { - no_basic_stat_requested = 0; - break; - } - - /* - * Convert ids to xstats ids that PMD knows. - * ids known by user are basic + extended stats. - */ - ids_copy[i] = ids[i] - basic_count; - } - - if (no_basic_stat_requested) - return (*dev->dev_ops->xstats_get_names_by_id)(dev, - ids_copy, xstats_names, size); + nb_basic_stats = 0; + if (ids != NULL) { + for (i = 0; i < size; i++) + nb_basic_stats += (ids[i] < basic_count); } + /* no baisc stats requested, devops function provided */ + if (nb_basic_stats == 0 && ids != NULL && size != 0 && + dev->dev_ops->xstats_get_names_by_id != NULL) + return eth_xstats_get_by_name_by_id(dev, ids, xstats_names, + size, basic_count); + /* Retrieve all stats */ if (!ids) { int num_stats = rte_eth_xstats_get_names(port_id, xstats_names, @@ -3380,17 +3412,8 @@ enum { return -ENOMEM; } - if (ids) { - for (i = 0; i < size; i++) { - if (ids[i] >= basic_count) { - no_ext_stat_requested = 0; - break; - } - } - } - /* Fill xstats_names_copy structure */ - if (ids && no_ext_stat_requested) { + if (ids && nb_basic_stats == size) { eth_basic_stats_get_names(dev, xstats_names_copy); } else { ret = rte_eth_xstats_get_names(port_id, xstats_names_copy, @@ -3514,17 +3537,47 @@ enum { return count; } +static int +eth_xtats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids, + uint64_t *values, uint32_t size, uint32_t basic_count) +{ + int32_t rc; + uint32_t i, k, m, n; + uint64_t ids_copy[ETH_XSTATS_ITER_NUM]; + + m = 0; + for (n = 0; n != size; n += k) { + + k = RTE_MIN(size - n, RTE_DIM(ids_copy)); + + /* + * Convert ids to xstats ids that PMD knows. + * ids known by user are basic + extended stats. + */ + for (i = 0; i < k; i++) + ids_copy[i] = ids[n + i] - basic_count; + + rc = (*dev->dev_ops->xstats_get_by_id)(dev, ids_copy, + values + m, k); + if (rc < 0) + return rc; + m += rc; + } + + return m; +} + /* retrieve ethdev extended statistics */ int rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids, uint64_t *values, unsigned int size) { - unsigned int no_basic_stat_requested = 1; - unsigned int no_ext_stat_requested = 1; + unsigned int nb_basic_stats; unsigned int num_xstats_filled; unsigned int basic_count; uint16_t expected_entries; struct rte_eth_dev *dev; + struct rte_eth_xstat *xstats; unsigned int i; int ret; @@ -3535,7 +3588,6 @@ enum { if (ret < 0) return ret; expected_entries = (uint16_t)ret; - struct rte_eth_xstat xstats[expected_entries]; basic_count = eth_dev_get_xstats_basic_count(dev); /* Return max number of stats if no ids given */ @@ -3549,51 +3601,41 @@ enum { if (ids && !values) return -EINVAL; - if (ids && dev->dev_ops->xstats_get_by_id != NULL && size) { - unsigned int basic_count = eth_dev_get_xstats_basic_count(dev); - uint64_t ids_copy[size]; - - for (i = 0; i < size; i++) { - if (ids[i] < basic_count) { - no_basic_stat_requested = 0; - break; - } - - /* - * Convert ids to xstats ids that PMD knows. - * ids known by user are basic + extended stats. - */ - ids_copy[i] = ids[i] - basic_count; - } - - if (no_basic_stat_requested) - return (*dev->dev_ops->xstats_get_by_id)(dev, ids_copy, - values, size); + nb_basic_stats = 0; + if (ids != NULL) { + for (i = 0; i < size; i++) + nb_basic_stats += (ids[i] < basic_count); } - if (ids) { - for (i = 0; i < size; i++) { - if (ids[i] >= basic_count) { - no_ext_stat_requested = 0; - break; - } - } + /* no baisc stats requested, devops function provided */ + if (nb_basic_stats == 0 && ids != NULL && size != 0 && + dev->dev_ops->xstats_get_by_id != NULL) + return eth_xtats_get_by_id(dev, ids, values, size, basic_count); + + xstats = calloc(expected_entries, sizeof(xstats[0])); + if (xstats == NULL) { + RTE_ETHDEV_LOG_LINE(ERR, "Can't allocate memory"); + return -ENOMEM; } /* Fill the xstats structure */ - if (ids && no_ext_stat_requested) + if (ids && nb_basic_stats == size) ret = eth_basic_stats_get(port_id, xstats); else ret = rte_eth_xstats_get(port_id, xstats, expected_entries); - if (ret < 0) + if (ret < 0) { + free(xstats); return ret; + } num_xstats_filled = (unsigned int)ret; /* Return all stats */ if (!ids) { for (i = 0; i < num_xstats_filled; i++) values[i] = xstats[i].value; + + free(xstats); return expected_entries; } @@ -3601,14 +3643,15 @@ enum { for (i = 0; i < size; i++) { if (ids[i] >= expected_entries) { RTE_ETHDEV_LOG_LINE(ERR, "Id value isn't valid"); - return -1; + break; } values[i] = xstats[ids[i]].value; } rte_eth_trace_xstats_get_by_id(port_id, ids, values, size); - return size; + free(xstats); + return (i == size) ? (int32_t)size : -1; } int