From patchwork Sat Apr 10 14:23:53 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xueming Li X-Patchwork-Id: 91033 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 544D2A0546; Sat, 10 Apr 2021 16:24:16 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DC0F01412A3; Sat, 10 Apr 2021 16:24:14 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by mails.dpdk.org (Postfix) with ESMTP id 32D25141291 for ; Sat, 10 Apr 2021 16:24:14 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE1 (envelope-from xuemingl@nvidia.com) with SMTP; 10 Apr 2021 17:24:12 +0300 Received: from nvidia.com (pegasus05.mtr.labs.mlnx [10.210.16.100]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 13AEO6Dx018265; Sat, 10 Apr 2021 17:24:12 +0300 From: Xueming Li To: Thomas Monjalon , Gaetan Rivet Cc: dev@dpdk.org, xuemingl@nvidia.com, Asaf Penso , Wenzhuo Lu , Beilei Xing , Bernard Iremonger , Gaetan Rivet , Anatoly Burakov , Dmitry Kozlyuk , Narcisa Ana Maria Vasile , Dmitry Malloy , Pallavi Kadam , Ray Kinsella , Neil Horman , Ferruh Yigit , Andrew Rybchenko Date: Sat, 10 Apr 2021 14:23:53 +0000 Message-Id: <1618064637-16413-2-git-send-email-xuemingl@nvidia.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> References: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> In-Reply-To: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> References: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> Subject: [dpdk-dev] [PATCH v4 1/5] devargs: unify scratch buffer storage 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 Sender: "dev" In current design, legacy parser rte_devargs_parse() saved scratch buffer to devargs.args while new parser rte_devargs_layers_parse() saved to devargs.data. Code using devargs had to know the difference and cleaned up memory accordingly - error prone. This patch unifies scratch buffer to data field, introduces rte_devargs_reset() function to wrap the memory clean up logic. Signed-off-by: Xueming Li Acked-by: Ray Kinsella Reviewed-by: Gaetan Rivet --- app/test-pmd/config.c | 3 +- app/test-pmd/testpmd.c | 5 +-- drivers/bus/vdev/vdev.c | 9 +++--- drivers/net/failsafe/failsafe_args.c | 3 +- drivers/net/failsafe/failsafe_eal.c | 2 +- examples/multi_process/hotplug_mp/commands.c | 6 ++-- lib/librte_eal/common/eal_common_dev.c | 9 +++--- lib/librte_eal/common/eal_common_devargs.c | 34 +++++++++++--------- lib/librte_eal/common/hotplug_mp.c | 6 ++-- lib/librte_eal/include/rte_devargs.h | 18 ++++++++--- lib/librte_eal/rte_eal_exports.def | 1 + lib/librte_eal/version.map | 1 + lib/librte_ethdev/rte_ethdev.c | 8 ++--- 13 files changed, 59 insertions(+), 46 deletions(-) diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index ef0b9784d0..d774610419 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -509,8 +509,6 @@ device_infos_display(const char *identifier) if (rte_devargs_parsef(&da, "%s", identifier)) { printf("cannot parse identifier\n"); - if (da.args) - free(da.args); return; } @@ -558,6 +556,7 @@ device_infos_display(const char *identifier) } } }; + rte_devargs_reset(&da); } void diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 96d2e0fcec..d4be23f8f8 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -3015,8 +3015,6 @@ detach_devargs(char *identifier) memset(&da, 0, sizeof(da)); if (rte_devargs_parsef(&da, "%s", identifier)) { printf("cannot parse identifier\n"); - if (da.args) - free(da.args); return; } @@ -3025,6 +3023,7 @@ detach_devargs(char *identifier) if (ports[port_id].port_status != RTE_PORT_STOPPED) { printf("Port %u not stopped\n", port_id); rte_eth_iterator_cleanup(&iterator); + rte_devargs_reset(&da); return; } port_flow_flush(port_id); @@ -3034,6 +3033,7 @@ detach_devargs(char *identifier) if (rte_eal_hotplug_remove(da.bus->name, da.name) != 0) { TESTPMD_LOG(ERR, "Failed to detach device %s(%s)\n", da.name, da.bus->name); + rte_devargs_reset(&da); return; } @@ -3042,6 +3042,7 @@ detach_devargs(char *identifier) printf("Device %s is detached\n", identifier); printf("Now total ports is %d\n", nb_ports); printf("Done\n"); + rte_devargs_reset(&da); } void diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c index 9a673347ae..d075409942 100644 --- a/drivers/bus/vdev/vdev.c +++ b/drivers/bus/vdev/vdev.c @@ -245,13 +245,14 @@ alloc_devargs(const char *name, const char *args) devargs->bus = &rte_vdev_bus; if (args) - devargs->args = strdup(args); + devargs->data = strdup(args); else - devargs->args = strdup(""); + devargs->data = strdup(""); + devargs->args = devargs->data; ret = strlcpy(devargs->name, name, sizeof(devargs->name)); if (ret < 0 || ret >= (int)sizeof(devargs->name)) { - free(devargs->args); + rte_devargs_reset(devargs); free(devargs); return NULL; } @@ -305,7 +306,7 @@ insert_vdev(const char *name, const char *args, return 0; fail: - free(devargs->args); + rte_devargs_reset(devargs); free(devargs); free(dev); return ret; diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c index 707490b94c..b203e02d9a 100644 --- a/drivers/net/failsafe/failsafe_args.c +++ b/drivers/net/failsafe/failsafe_args.c @@ -451,8 +451,7 @@ failsafe_args_free(struct rte_eth_dev *dev) sdev->cmdline = NULL; free(sdev->fd_str); sdev->fd_str = NULL; - free(sdev->devargs.args); - sdev->devargs.args = NULL; + rte_devargs_reset(&sdev->devargs); } } diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c index b9fc508673..cb4a2abc02 100644 --- a/drivers/net/failsafe/failsafe_eal.c +++ b/drivers/net/failsafe/failsafe_eal.c @@ -79,7 +79,7 @@ fs_bus_init(struct rte_eth_dev *dev) rte_eth_devices[pid].device->devargs; /* Take control of probed device. */ - free(da->args); + rte_devargs_reset(da); memset(da, 0, sizeof(*da)); if (probed_da != NULL) snprintf(devstr, sizeof(devstr), "%s,%s", diff --git a/examples/multi_process/hotplug_mp/commands.c b/examples/multi_process/hotplug_mp/commands.c index a8a39d07f7..48fd329583 100644 --- a/examples/multi_process/hotplug_mp/commands.c +++ b/examples/multi_process/hotplug_mp/commands.c @@ -121,8 +121,6 @@ static void cmd_dev_attach_parsed(void *parsed_result, if (rte_devargs_parsef(&da, "%s", res->devargs)) { cmdline_printf(cl, "cannot parse devargs\n"); - if (da.args) - free(da.args); return; } @@ -131,6 +129,7 @@ static void cmd_dev_attach_parsed(void *parsed_result, else cmdline_printf(cl, "failed to attached device %s\n", da.name); + rte_devargs_reset(&da); } cmdline_parse_token_string_t cmd_dev_attach_attach = @@ -168,8 +167,6 @@ static void cmd_dev_detach_parsed(void *parsed_result, if (rte_devargs_parsef(&da, "%s", res->devargs)) { cmdline_printf(cl, "cannot parse devargs\n"); - if (da.args) - free(da.args); return; } @@ -180,6 +177,7 @@ static void cmd_dev_detach_parsed(void *parsed_result, else cmdline_printf(cl, "failed to dettach device %s\n", da.name); + rte_devargs_reset(&da); } cmdline_parse_token_string_t cmd_dev_detach_detach = diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c index 8a3bd3100a..148a23830a 100644 --- a/lib/librte_eal/common/eal_common_dev.c +++ b/lib/librte_eal/common/eal_common_dev.c @@ -185,10 +185,8 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev) return ret; err_devarg: - if (rte_devargs_remove(da) != 0) { - free(da->args); - free(da); - } + if (rte_devargs_remove(da) != 0) + rte_devargs_reset(da); return ret; } @@ -586,7 +584,8 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, it->bus_str = NULL; it->cls_str = NULL; - devargs.data = dev_str; + /* Setting data field implies no malloc in parsing. */ + devargs.data = (void *)(intptr_t)dev_str; if (rte_devargs_layers_parse(&devargs, dev_str)) goto get_out; diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c index fcf3d9a3cc..48f85ee9c0 100644 --- a/lib/librte_eal/common/eal_common_devargs.c +++ b/lib/librte_eal/common/eal_common_devargs.c @@ -150,7 +150,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, * their parsing afterward. */ if (devargs->data != devstr) { - char *s = (void *)(intptr_t)(devargs->data); + char *s = devargs->data; while ((s = strchr(s, '/'))) { *s = '\0'; @@ -219,13 +219,14 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev) da->bus = bus; /* Parse eventual device arguments */ if (devname[i] == ',') - da->args = strdup(&devname[i + 1]); + da->data = strdup(&devname[i + 1]); else - da->args = strdup(""); - if (da->args == NULL) { + da->data = strdup(""); + if (da->data == NULL) { RTE_LOG(ERR, EAL, "not enough memory to parse arguments\n"); return -ENOMEM; } + da->drv_str = da->data; return 0; } @@ -260,6 +261,16 @@ rte_devargs_parsef(struct rte_devargs *da, const char *format, ...) return ret; } +void +rte_devargs_reset(struct rte_devargs *da) +{ + if (da == NULL) + return; + if (da->data) + free(da->data); + da->data = NULL; +} + int rte_devargs_insert(struct rte_devargs **da) { @@ -276,15 +287,8 @@ rte_devargs_insert(struct rte_devargs **da) if (strcmp(listed_da->bus->name, (*da)->bus->name) == 0 && strcmp(listed_da->name, (*da)->name) == 0) { /* device already in devargs list, must be updated */ - listed_da->type = (*da)->type; - listed_da->policy = (*da)->policy; - free(listed_da->args); - listed_da->args = (*da)->args; - listed_da->bus = (*da)->bus; - listed_da->cls = (*da)->cls; - listed_da->bus_str = (*da)->bus_str; - listed_da->cls_str = (*da)->cls_str; - listed_da->data = (*da)->data; + rte_devargs_reset(listed_da); + *listed_da = **da; /* replace provided devargs with found one */ free(*da); *da = listed_da; @@ -326,7 +330,7 @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str) fail: if (devargs) { - free(devargs->args); + rte_devargs_reset(devargs); free(devargs); } @@ -346,7 +350,7 @@ rte_devargs_remove(struct rte_devargs *devargs) if (strcmp(d->bus->name, devargs->bus->name) == 0 && strcmp(d->name, devargs->name) == 0) { TAILQ_REMOVE(&devargs_list, d, next); - free(d->args); + rte_devargs_reset(d); free(d); return 0; } diff --git a/lib/librte_eal/common/hotplug_mp.c b/lib/librte_eal/common/hotplug_mp.c index ee791903b3..ae6010e8f8 100644 --- a/lib/librte_eal/common/hotplug_mp.c +++ b/lib/librte_eal/common/hotplug_mp.c @@ -95,6 +95,7 @@ __handle_secondary_request(void *param) tmp_req = *req; + memset(&da, 0, sizeof(da)); if (req->t == EAL_DEV_REQ_TYPE_ATTACH) { ret = local_dev_probe(req->devargs, &dev); if (ret != 0) { @@ -118,8 +119,6 @@ __handle_secondary_request(void *param) ret = rte_devargs_parse(&da, req->devargs); if (ret != 0) goto finish; - free(da.args); /* we don't need those */ - da.args = NULL; ret = eal_dev_hotplug_request_to_secondary(&tmp_req); if (ret != 0) { @@ -176,6 +175,7 @@ __handle_secondary_request(void *param) if (ret) RTE_LOG(ERR, EAL, "failed to send response to secondary\n"); + rte_devargs_reset(&da); free(bundle->peer); free(bundle); } @@ -283,7 +283,7 @@ static void __handle_primary_request(void *param) ret = local_dev_remove(dev); quit: - free(da->args); + rte_devargs_reset(da); free(da); break; default: diff --git a/lib/librte_eal/include/rte_devargs.h b/lib/librte_eal/include/rte_devargs.h index 296f19324f..134b44a887 100644 --- a/lib/librte_eal/include/rte_devargs.h +++ b/lib/librte_eal/include/rte_devargs.h @@ -60,16 +60,16 @@ struct rte_devargs { /** Name of the device. */ char name[RTE_DEV_NAME_MAX_LEN]; RTE_STD_C11 - union { - /** Arguments string as given by user or "" for no argument. */ - char *args; + union { /**< driver-related part of device string. */ + const char *args; /**< legacy name. */ const char *drv_str; }; struct rte_bus *bus; /**< bus handle. */ struct rte_class *cls; /**< class handle. */ const char *bus_str; /**< bus-related part of device string. */ const char *cls_str; /**< class-related part of device string. */ - const char *data; /**< Device string storage. */ + char *data; + /**< Raw string including bus, class and driver arguments. */ }; /** @@ -145,6 +145,16 @@ rte_devargs_parsef(struct rte_devargs *da, const char *format, ...) __rte_format_printf(2, 0); +/** + * Free resources in devargs. + * + * @param da + * The devargs structure holding the device information. + */ +__rte_experimental +void +rte_devargs_reset(struct rte_devargs *da); + /** * Insert an rte_devargs in the global list. * diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def index c320077547..357de89ffc 100644 --- a/lib/librte_eal/rte_eal_exports.def +++ b/lib/librte_eal/rte_eal_exports.def @@ -30,6 +30,7 @@ EXPORTS rte_devargs_parse rte_devargs_parsef rte_devargs_remove + rte_devargs_reset rte_devargs_type_count rte_dump_physmem_layout rte_dump_stack diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map index e23745ae6e..174c548297 100644 --- a/lib/librte_eal/version.map +++ b/lib/librte_eal/version.map @@ -411,6 +411,7 @@ EXPERIMENTAL { rte_power_pause; # added in 21.05 + rte_devargs_reset; rte_thread_key_create; rte_thread_key_delete; rte_thread_value_get; diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 3059aa55b3..e11a95558f 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -193,13 +193,14 @@ int rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str) { int ret; - struct rte_devargs devargs = {.args = NULL}; + struct rte_devargs devargs; const char *bus_param_key; char *bus_str = NULL; char *cls_str = NULL; int str_size; memset(iter, 0, sizeof(*iter)); + memset(&devargs, 0, sizeof(devargs)); /* * The devargs string may use various syntaxes: @@ -244,8 +245,6 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str) goto error; } iter->cls_str = cls_str; - free(devargs.args); /* allocated by rte_devargs_parse() */ - devargs.args = NULL; iter->bus = devargs.bus; if (iter->bus->dev_iterate == NULL) { @@ -278,13 +277,14 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str) end: iter->cls = rte_class_find_by_name("eth"); + rte_devargs_reset(&devargs); return 0; error: if (ret == -ENOTSUP) RTE_ETHDEV_LOG(ERR, "Bus %s does not support iterating.\n", iter->bus->name); - free(devargs.args); + rte_devargs_reset(&devargs); free(bus_str); free(cls_str); return ret; From patchwork Sat Apr 10 14:23:54 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xueming Li X-Patchwork-Id: 91034 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 F3D3DA0546; Sat, 10 Apr 2021 16:24:22 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1FC171412B2; Sat, 10 Apr 2021 16:24:21 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by mails.dpdk.org (Postfix) with ESMTP id 444F91412B0 for ; Sat, 10 Apr 2021 16:24:19 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE1 (envelope-from xuemingl@nvidia.com) with SMTP; 10 Apr 2021 17:24:14 +0300 Received: from nvidia.com (pegasus05.mtr.labs.mlnx [10.210.16.100]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 13AEO6E0018265; Sat, 10 Apr 2021 17:24:14 +0300 From: Xueming Li To: Thomas Monjalon , Gaetan Rivet Cc: dev@dpdk.org, xuemingl@nvidia.com, Asaf Penso , gaetan.rivet@6wind.com, stable@dpdk.org, Shreyansh Jain Date: Sat, 10 Apr 2021 14:23:54 +0000 Message-Id: <1618064637-16413-3-git-send-email-xuemingl@nvidia.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> References: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> In-Reply-To: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> References: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> Subject: [dpdk-dev] [PATCH v4 2/5] devargs: fix memory leak on parsing error 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 Sender: "dev" This patch fixes memory leak in parsing error handling. Fixes: 338327d731e6 ("devargs: add function to parse device layers") Cc: gaetan.rivet@6wind.com Cc: stable@dpdk.org Signed-off-by: Xueming Li Reviewed-by: Gaetan Rivet --- lib/librte_eal/common/eal_common_devargs.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c index 48f85ee9c0..e40b91ea66 100644 --- a/lib/librte_eal/common/eal_common_devargs.c +++ b/lib/librte_eal/common/eal_common_devargs.c @@ -60,6 +60,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, size_t nblayer; size_t i = 0; int ret = 0; + bool allocated_data = false; /* Split each sub-lists. */ nblayer = devargs_layer_count(devstr); @@ -81,6 +82,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, ret = -ENOMEM; goto get_out; } + allocated_data = true; s = devargs->data; } @@ -163,8 +165,14 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, if (layers[i].kvlist) rte_kvargs_free(layers[i].kvlist); } - if (ret != 0) + if (ret != 0) { + if (allocated_data) { + /* Free duplicated data. */ + free(devargs->data); + devargs->data = NULL; + } rte_errno = -ret; + } return ret; } From patchwork Sat Apr 10 14:23:55 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xueming Li X-Patchwork-Id: 91035 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 A10C9A0546; Sat, 10 Apr 2021 16:24:29 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 730DD1412BD; Sat, 10 Apr 2021 16:24:25 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by mails.dpdk.org (Postfix) with ESMTP id 43BCE1412B8 for ; Sat, 10 Apr 2021 16:24:24 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE1 (envelope-from xuemingl@nvidia.com) with SMTP; 10 Apr 2021 17:24:21 +0300 Received: from nvidia.com (pegasus05.mtr.labs.mlnx [10.210.16.100]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 13AEO6E1018265; Sat, 10 Apr 2021 17:24:20 +0300 From: Xueming Li To: Thomas Monjalon , Gaetan Rivet Cc: dev@dpdk.org, xuemingl@nvidia.com, Asaf Penso , Olivier Matz , Ray Kinsella , Neil Horman Date: Sat, 10 Apr 2021 14:23:55 +0000 Message-Id: <1618064637-16413-4-git-send-email-xuemingl@nvidia.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> References: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> In-Reply-To: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> References: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> Subject: [dpdk-dev] [PATCH v4 3/5] kvargs: add get by key function 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 Sender: "dev" Adds a new function to get value of a specific key from kvargs list. Signed-off-by: Xueming Li Reviewed-by: Gaetan Rivet --- lib/librte_kvargs/rte_kvargs.c | 20 ++++++++++++++++++++ lib/librte_kvargs/rte_kvargs.h | 21 +++++++++++++++++++++ lib/librte_kvargs/version.map | 3 +++ 3 files changed, 44 insertions(+) diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c index ffae8914cf..40e7670ab3 100644 --- a/lib/librte_kvargs/rte_kvargs.c +++ b/lib/librte_kvargs/rte_kvargs.c @@ -203,6 +203,26 @@ rte_kvargs_free(struct rte_kvargs *kvlist) free(kvlist); } +/* Lookup a value in an rte_kvargs list by its key. */ +const char * +rte_kvargs_get(const struct rte_kvargs *kvlist, const char *key) +{ + unsigned int i; + + if (!kvlist) + return NULL; + for (i = 0; i < kvlist->count; ++i) { + /* Allows key to be NULL. */ + if (!key && !kvlist->pairs[i].key) + return kvlist->pairs[i].value; + if (!key || !kvlist->pairs[i].key) + continue; + if (!strcmp(kvlist->pairs[i].key, key)) + return kvlist->pairs[i].value; + } + return NULL; +} + /* * Parse the arguments "key=value,key=value,..." string and return * an allocated structure that contains a key/value list. Also diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h index eff598e08b..cb3ea99850 100644 --- a/lib/librte_kvargs/rte_kvargs.h +++ b/lib/librte_kvargs/rte_kvargs.h @@ -114,6 +114,27 @@ struct rte_kvargs *rte_kvargs_parse_delim(const char *args, */ void rte_kvargs_free(struct rte_kvargs *kvlist); +/** + * Get the value associated with a given key. + * + * If the key is NULL, the first value from the list is returned. + * If multiple key matches, the value of the first one is returned. + * + * The memory returned is allocated as part of the rte_kvargs structure, + * it must never be modified. + * + * @param kvlist + * A list of rte_kvargs pair of 'key=value'. + * @param key + * The matching key. + + * @return + * NULL if no key matches the input, a value associated with a matching + * key otherwise. + */ +__rte_experimental +const char *rte_kvargs_get(const struct rte_kvargs *kvlist, const char *key); + /** * Call a handler function for each key/value matching the key * diff --git a/lib/librte_kvargs/version.map b/lib/librte_kvargs/version.map index ed375bf4a3..fb9bed4f93 100644 --- a/lib/librte_kvargs/version.map +++ b/lib/librte_kvargs/version.map @@ -15,4 +15,7 @@ EXPERIMENTAL { rte_kvargs_parse_delim; rte_kvargs_strcmp; + # added in 21.05 + rte_kvargs_get; + }; From patchwork Sat Apr 10 14:23:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xueming Li X-Patchwork-Id: 91036 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 0E65BA0546; Sat, 10 Apr 2021 16:24:36 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A8EF81412CE; Sat, 10 Apr 2021 16:24:30 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by mails.dpdk.org (Postfix) with ESMTP id 46EE01412CA for ; Sat, 10 Apr 2021 16:24:29 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE1 (envelope-from xuemingl@nvidia.com) with SMTP; 10 Apr 2021 17:24:27 +0300 Received: from nvidia.com (pegasus05.mtr.labs.mlnx [10.210.16.100]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 13AEO6E2018265; Sat, 10 Apr 2021 17:24:27 +0300 From: Xueming Li To: Thomas Monjalon , Gaetan Rivet Cc: dev@dpdk.org, xuemingl@nvidia.com, Asaf Penso Date: Sat, 10 Apr 2021 14:23:56 +0000 Message-Id: <1618064637-16413-5-git-send-email-xuemingl@nvidia.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> References: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> In-Reply-To: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> References: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> Subject: [dpdk-dev] [PATCH v4 4/5] bus: add device arguments name parsing API 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 Sender: "dev" For device probe and iterator, devargs name was key information, parsed by rte_devargs_parse. In legacy parser, devargs name was extracted after bus name: bus:name,kv_arguments,,, Example: pci:83:00.0,arguments,... vdev:pcap0,... To be compatible with legacy parser, this patch introduces new bus driver API devargs_parse to parse devargs and update devargs name. If devargs_parse not implemented by bus driver, the new syntax parser rte_devargs_layers_parse default will resolve devargs name from bus's "name" argument. Different bus driver might choose different keys from arguments with unified format. The PCI bus implementation fills the devargs name with the "addr" argument, example: -a bus=pci,addr=83:00.0/class=eth/driver=mlx5,... name: 0000:03:00.0 -a bus=vdev,name=pcap0/class=eth/driver=pcap,... name:pcap0 Signed-off-by: Xueming Li Reviewed-by: Gaetan Rivet --- drivers/bus/pci/pci_common.c | 1 + drivers/bus/pci/pci_params.c | 47 ++++++++++++++++++++++ drivers/bus/pci/private.h | 14 +++++++ lib/librte_eal/common/eal_common_devargs.c | 31 ++++++++++++++ lib/librte_eal/include/rte_bus.h | 18 +++++++++ 5 files changed, 111 insertions(+) diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c index 9b8d769287..61d3f51452 100644 --- a/drivers/bus/pci/pci_common.c +++ b/drivers/bus/pci/pci_common.c @@ -760,6 +760,7 @@ struct rte_pci_bus rte_pci_bus = { .dev_iterate = rte_pci_dev_iterate, .hot_unplug_handler = pci_hot_unplug_handler, .sigbus_handler = pci_sigbus_handler, + .devargs_parse = rte_pci_devargs_parse, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list), diff --git a/drivers/bus/pci/pci_params.c b/drivers/bus/pci/pci_params.c index 3192e9c967..c0c282e948 100644 --- a/drivers/bus/pci/pci_params.c +++ b/drivers/bus/pci/pci_params.c @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include "private.h" @@ -76,3 +78,48 @@ rte_pci_dev_iterate(const void *start, rte_kvargs_free(kvargs); return dev; } + +int +rte_pci_devargs_parse(struct rte_devargs *da) +{ + struct rte_kvargs *kvargs; + const char *addr_str; + struct rte_pci_addr addr; + int ret; + + if (da == NULL) + return 0; + RTE_ASSERT(da->bus_str != NULL); + + kvargs = rte_kvargs_parse(da->bus_str, NULL); + if (kvargs == NULL) { + RTE_LOG(ERR, EAL, "cannot parse argument list: %s\n", + da->bus_str); + ret = -ENODEV; + goto out; + } + + addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]); + if (addr_str == NULL) { + RTE_LOG(ERR, EAL, "No PCI address specified using '%s=' in: %s\n", + pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str); + ret = -ENODEV; + goto out; + } + + ret = rte_pci_addr_parse(addr_str, &addr); + if (ret != 0) { + RTE_LOG(ERR, EAL, "PCI address invalid: %s\n", da->bus_str); + ret = -EINVAL; + goto out; + } + + rte_pci_device_name(&addr, da->name, sizeof(da->name)); + +out: + if (kvargs != NULL) + rte_kvargs_free(kvargs); + if (ret != 0) + rte_errno = -ret; + return ret; +} diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h index f566943f5e..8bc5140e97 100644 --- a/drivers/bus/pci/private.h +++ b/drivers/bus/pci/private.h @@ -267,4 +267,18 @@ rte_pci_dev_iterate(const void *start, const char *str, const struct rte_dev_iterator *it); +/* + * Parse device arguments and update name. + * + * @param da + * device arguments to parse. + * + * @return + * 0 on success. + * -EINVAL: kvargs string is invalid and cannot be parsed. + * -ENODEV: no key matching a device ID is found in the kv list. + */ +int +rte_pci_devargs_parse(struct rte_devargs *da); + #endif /* _PCI_PRIVATE_H_ */ diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c index e40b91ea66..3a62521e05 100644 --- a/lib/librte_eal/common/eal_common_devargs.c +++ b/lib/librte_eal/common/eal_common_devargs.c @@ -19,6 +19,7 @@ #include #include #include +#include #include "eal_private.h" /** user device double-linked queue type definition */ @@ -40,6 +41,28 @@ devargs_layer_count(const char *s) return i; } +/* Resolve devargs name from bus arguments. */ +static int +devargs_bus_parse_default(struct rte_devargs *devargs, + struct rte_kvargs *bus_args) +{ + const char *name; + + /* Parse devargs name from bus key-value list. */ + name = rte_kvargs_get(bus_args, "name"); + if (name == NULL) { + RTE_LOG(INFO, EAL, "devargs name not found: %s\n", + devargs->data); + return 0; + } + if (rte_strscpy(devargs->name, name, sizeof(devargs->name)) < 0) { + RTE_LOG(ERR, EAL, "devargs name too long: %s\n", + devargs->data); + return -E2BIG; + } + return 0; +} + int rte_devargs_layers_parse(struct rte_devargs *devargs, const char *devstr) @@ -118,6 +141,8 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, if (layers[i].kvlist == NULL) continue; kv = &layers[i].kvlist->pairs[0]; + if (kv->key == NULL) + continue; if (strcmp(kv->key, "bus") == 0) { bus = rte_bus_find_by_name(kv->value); if (bus == NULL) { @@ -160,6 +185,12 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, } } + /* Resolve devarg's name. */ + if (bus && bus->devargs_parse) + ret = bus->devargs_parse(devargs); + else if (layers[0].kvlist != NULL) + ret = devargs_bus_parse_default(devargs, layers[0].kvlist); + get_out: for (i = 0; i < RTE_DIM(layers); i++) { if (layers[i].kvlist) diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h index 80b154fb98..0a99f3b5a3 100644 --- a/lib/librte_eal/include/rte_bus.h +++ b/lib/librte_eal/include/rte_bus.h @@ -210,6 +210,23 @@ typedef int (*rte_bus_hot_unplug_handler_t)(struct rte_device *dev); */ typedef int (*rte_bus_sigbus_handler_t)(const void *failure_addr); +/** + * Parse device arguments, setting the device name in the devargs as a result. + * + * On error rte_errno is set. + * + * @param da + * Pointer to the devargs to parse. + * The 'bus_str' field must be set. + * + * @return + * 0 on successful parsing. + * -EINVAL: on parsing error. + * -ENODEV: if no key matching a device argument is specified. + * -E2BIG: device name is too long. + */ +typedef int (*rte_bus_devargs_parse_t)(struct rte_devargs *da); + /** * Bus scan policies */ @@ -267,6 +284,7 @@ struct rte_bus { /**< handle hot-unplug failure on the bus */ rte_bus_sigbus_handler_t sigbus_handler; /**< handle sigbus error on the bus */ + rte_bus_devargs_parse_t devargs_parse; /**< Parse device arguments */ }; From patchwork Sat Apr 10 14:23:57 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xueming Li X-Patchwork-Id: 91037 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 0814DA0546; Sat, 10 Apr 2021 16:24:44 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 53EF91412D5; Sat, 10 Apr 2021 16:24:35 +0200 (CEST) Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by mails.dpdk.org (Postfix) with ESMTP id 531A51412D7 for ; Sat, 10 Apr 2021 16:24:34 +0200 (CEST) Received: from Internal Mail-Server by MTLPINE1 (envelope-from xuemingl@nvidia.com) with SMTP; 10 Apr 2021 17:24:32 +0300 Received: from nvidia.com (pegasus05.mtr.labs.mlnx [10.210.16.100]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 13AEO6E3018265; Sat, 10 Apr 2021 17:24:32 +0300 From: Xueming Li To: Thomas Monjalon , Gaetan Rivet Cc: dev@dpdk.org, xuemingl@nvidia.com, Asaf Penso , Ferruh Yigit , Andrew Rybchenko Date: Sat, 10 Apr 2021 14:23:57 +0000 Message-Id: <1618064637-16413-6-git-send-email-xuemingl@nvidia.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> References: <1618064637-16413-1-git-send-email-xuemingl@nvidia.com> In-Reply-To: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> References: <1608304614-13908-2-git-send-email-xuemingl@nvidia.com> Subject: [dpdk-dev] [PATCH v4 5/5] devargs: parse global device syntax 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 Sender: "dev" When parsing a devargs, try to parse using the global device syntax first. Fallback on legacy syntax on error. Example of new global device syntax: -a bus=pci,addr=82:00.0/class=eth/driver=mlx5,dv_flow_en=1 Signed-off-by: Xueming Li Reviewed-by: Gaetan Rivet --- doc/guides/rel_notes/release_21_05.rst | 6 ++++++ lib/librte_eal/common/eal_common_devargs.c | 16 ++++++++++++---- lib/librte_eal/include/rte_devargs.h | 4 ++++ lib/librte_ethdev/rte_ethdev.c | 1 - 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst index 374d6d98ea..ff1459a4d1 100644 --- a/doc/guides/rel_notes/release_21_05.rst +++ b/doc/guides/rel_notes/release_21_05.rst @@ -131,6 +131,12 @@ New Features * Added command to display Rx queue used descriptor count. ``show port (port_id) rxq (queue_id) desc used count`` +* **Enabled new devargs parser.** + + * Unified devargs storage buffer usage. + * Added new bus driver api to allow bus driver contribute to devargs parsing. + * Try new devargs syntax parser first, fallback to legacy syntax parser. + Removed Items ------------- diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c index 3a62521e05..069d8f8499 100644 --- a/lib/librte_eal/common/eal_common_devargs.c +++ b/lib/librte_eal/common/eal_common_devargs.c @@ -125,7 +125,6 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, layers[i].str = s; layers[i].kvlist = rte_kvargs_parse_delim(s, NULL, "/"); if (layers[i].kvlist == NULL) { - RTE_LOG(ERR, EAL, "Could not parse %s\n", s); ret = -EINVAL; goto get_out; } @@ -143,7 +142,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, kv = &layers[i].kvlist->pairs[0]; if (kv->key == NULL) continue; - if (strcmp(kv->key, "bus") == 0) { + if (strcmp(kv->key, RTE_DEVARGS_KEY_BUS) == 0) { bus = rte_bus_find_by_name(kv->value); if (bus == NULL) { RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n", @@ -151,7 +150,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, ret = -EFAULT; goto get_out; } - } else if (strcmp(kv->key, "class") == 0) { + } else if (strcmp(kv->key, RTE_DEVARGS_KEY_CLASS) == 0) { cls = rte_class_find_by_name(kv->value); if (cls == NULL) { RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n", @@ -159,7 +158,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, ret = -EFAULT; goto get_out; } - } else if (strcmp(kv->key, "driver") == 0) { + } else if (strcmp(kv->key, RTE_DEVARGS_KEY_DRIVER) == 0) { /* Ignore */ continue; } @@ -224,6 +223,15 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev) if (da == NULL) return -EINVAL; + /* First parse according global device syntax. */ + if (rte_devargs_layers_parse(da, dev) == 0) { + if (da->bus != NULL || da->cls != NULL) + return 0; + rte_devargs_reset(da); + } + + /* Otherwise fallback to legacy syntax: */ + /* Retrieve eventual bus info */ do { devname = dev; diff --git a/lib/librte_eal/include/rte_devargs.h b/lib/librte_eal/include/rte_devargs.h index 134b44a887..39e34ea02e 100644 --- a/lib/librte_eal/include/rte_devargs.h +++ b/lib/librte_eal/include/rte_devargs.h @@ -25,6 +25,10 @@ extern "C" { #include #include +#define RTE_DEVARGS_KEY_BUS "bus" +#define RTE_DEVARGS_KEY_CLASS "class" +#define RTE_DEVARGS_KEY_DRIVER "driver" + /** * Type of generic device */ diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index e11a95558f..9e9b136438 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -207,7 +207,6 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str) * - 0000:08:00.0,representor=[1-3] * - pci:0000:06:00.0,representor=[0,5] * - class=eth,mac=00:11:22:33:44:55 - * A new syntax is in development (not yet supported): * - bus=X,paramX=x/class=Y,paramY=y/driver=Z,paramZ=z */