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 D5A3043249; Sun, 5 Nov 2023 06:50:17 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C60424029E; Sun, 5 Nov 2023 06:50:17 +0100 (CET) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 79B0840278 for ; Sun, 5 Nov 2023 06:50:16 +0100 (CET) Received: from dggpeml100024.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4SNNlv5G7tzrTJX; Sun, 5 Nov 2023 13:47:07 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml100024.china.huawei.com (7.185.36.115) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Sun, 5 Nov 2023 13:50:14 +0800 Subject: Re: [PATCH v3 0/5] fix segment fault when parse args To: Ferruh Yigit , CC: , References: <20230314124813.39521-1-fengchengwen@huawei.com> <20231103073811.13196-1-fengchengwen@huawei.com> From: fengchengwen Message-ID: <3aa13a7d-cc88-866b-8db7-6ed8dd46f625@huawei.com> Date: Sun, 5 Nov 2023 13:50:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpeml100024.china.huawei.com (7.185.36.115) X-CFilter-Loop: Reflected 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 Ferruh, On 2023/11/3 21:41, Ferruh Yigit wrote: > On 11/3/2023 7:38 AM, Chengwen Feng wrote: >> The rte_kvargs_process() was used to parse key-value (e.g. socket_id=0), >> it also supports to parse only-key (e.g. socket_id). But many drivers's >> callback can only handle key-value, it will segment fault if handles >> only-key. so the patchset [1] was introduced. >> >> Because the patchset [1] modified too much drivers, therefore: >> 1) A new API rte_kvargs_process_opt() was introduced, it inherits the >> function of rte_kvargs_process() which could parse both key-value and >> only-key. >> 2) Constraint the rte_kvargs_process() can only parse key-value. >> >> This patchset also include one bugfix for kvargs of mvneta driver. >> >> [1] https://patches.dpdk.org/project/dpdk/patch/20230320092110.37295-1-fengchengwen@huawei.com/ >> >> Chengwen Feng (5): >> kvargs: add one new process API >> net/af_packet: use new API to parse kvargs >> net/sfc: use new API to parse kvargs >> net/tap: use new API to parse kvargs >> net/mvneta: fix possible out-of-bounds write >> > > Hi Chengwen, > > I checked the driver code updates above, but it is hard to know if there > are more missing, each requires investigating one by one. > Perhaps it can be easier to trace back from your original patch [1] and > update the ones that doesn't need "value == NULL" check, I assume this > is what you did. There are 51 files modified in v2 [1], there will about 80 rte_kvargs_process() invoke in current git (stats by command [2]). Exclude kvargs lib and it's test and part comment, there are 30 file was not in v2: drivers/baseband/null/bbdev_null.c ---already treat NULL value as a error drivers/baseband/turbo_sw/bbdev_turbo_software.c ---already treat NULL value as a error drivers/bus/ifpga/ifpga_bus.c ---already treat NULL value as a error drivers/common/nfp/nfp_common_pci.c ---could handle NULL value drivers/common/sfc_efx/sfc_efx.c ---could handle NULL value drivers/dma/skeleton/skeleton_dmadev.c ---already treat NULL value as a error drivers/ml/cnxk/cn10k_ml_dev.c ---part treat NULL value as a error, other segment fault when with NULL value drivers/ml/cnxk/mvtvm_ml_dev.c ---segment fault when with NULL value drivers/net/af_packet/rte_eth_af_packet.c ---don't care about value, suggested don't change in v3's comment drivers/net/bnxt/bnxt_ethdev.c ---already treat NULL value as a error drivers/net/bonding/rte_eth_bond_pmd.c ---already treat NULL value as a error drivers/net/cpfl/cpfl_ethdev.c ---segment fault when with NULL value drivers/net/failsafe/failsafe_args.c ---already treat NULL value as a error drivers/net/hns3/hns3_common.c ---already treat NULL value as a error drivers/net/ixgbe/ixgbe_ethdev.c ---already treat NULL value as a error drivers/net/null/rte_eth_null.c ---already treat NULL value as a error drivers/net/softnic/rte_eth_softnic.c ---already treat NULL value as a error drivers/net/tap/rte_eth_tap.c ---could handle NULL value drivers/net/txgbe/txgbe_ethdev.c ---already treat NULL value as a error drivers/net/vhost/rte_eth_vhost.c ---already treat NULL value as a error drivers/net/virtio/virtio_ethdev.c ---already treat NULL value as a error drivers/net/virtio/virtio_pci_ethdev.c ---already treat NULL value as a error drivers/net/virtio/virtio_user_ethdev.c ---already treat NULL value as a error drivers/raw/ifpga/ifpga_rawdev.c ---already treat NULL value as a error drivers/raw/skeleton/skeleton_rawdev.c ---already treat NULL value as a error drivers/vdpa/ifc/ifcvf_vdpa.c ---already treat NULL value as a error drivers/vdpa/sfc/sfc_vdpa_filter.c ---already treat NULL value as a error lib/compressdev/rte_compressdev_pmd.c ---already treat NULL value as a error lib/cryptodev/cryptodev_pmd.c ---already treat NULL value as a error lib/ethdev/rte_ethdev_telemetry.c ---already treat NULL value as a error so we should only process three drivers: common/nfp, sfc, tap, and these are what v4 doing. [1] https://patches.dpdk.org/project/dpdk/patch/20230320092110.37295-1-fengchengwen@huawei.com/ [2] grep -rn "rte_kvargs_process(" | cut -d ":" -f 1 | sort | uniq -c | wc -l Thanks Chengwen > > . >