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 E97AD42CD9; Fri, 16 Jun 2023 13:57:38 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 23A494161A; Fri, 16 Jun 2023 13:57:38 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 4051440A7A; Fri, 16 Jun 2023 13:57:35 +0200 (CEST) Received: from kwepemi500012.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4QjHdx3hFMzqTFk; Fri, 16 Jun 2023 19:55:01 +0800 (CST) Received: from [10.78.231.32] (10.78.231.32) by kwepemi500012.china.huawei.com (7.221.188.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Fri, 16 Jun 2023 19:57:32 +0800 Message-ID: Date: Fri, 16 Jun 2023 19:57:32 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.0.3 Subject: Re: [PATCH] net/bonding: fix bond startup failure when NUMA is -1 To: Chaoyong He , "dev@dpdk.org" CC: oss-drivers , Niklas Soderlund , Zerun Fu , "stable@dpdk.org" , Nole Zhang , Long Wu References: <20230616032013.1275530-1-chaoyong.he@corigine.com> From: "humin (Q)" In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.78.231.32] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemi500012.china.huawei.com (7.221.188.12) 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 在 2023/6/16 14:08, Chaoyong He 写道: >> 在 2023/6/16 11:20, Chaoyong He 写道: >>> From: Zerun Fu >>> >>> After the mainline Linux kernel commit >>> "fe205d984e7730f4d21f6f8ebc60f0698404ac31" (ACPI: Remove side >> effect >>> of partly creating a node in acpi_map_pxm_to_online_node) by Jonathan >>> Cameron. When the system does not support NUMA architecture, the >>> "socket_id" is expected to be -1. The valid "socket_id" in BOND PMD is >>> greater than or equal to zero. So it will cause an error when DPDK >>> checks the validity of the "socket_id" when starting the bond. This >>> commit can fix this bug. >>> >>> Fixes: f294e04851fd ("net/bonding: fix socket ID check") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Zerun Fu >>> Reviewed-by: Peng Zhang >>> Reviewed-by: Chaoyong He >>> Reviewed-by: Long Wu >>> --- >>> drivers/net/bonding/rte_eth_bond_args.c | 6 ++++++ >>> drivers/net/bonding/rte_eth_bond_pmd.c | 2 +- >>> 2 files changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/bonding/rte_eth_bond_args.c >>> b/drivers/net/bonding/rte_eth_bond_args.c >>> index 6553166f5c..c137efd55f 100644 >>> --- a/drivers/net/bonding/rte_eth_bond_args.c >>> +++ b/drivers/net/bonding/rte_eth_bond_args.c >>> @@ -212,6 +212,12 @@ bond_ethdev_parse_socket_id_kvarg(const char >> *key __rte_unused, >>> if (*endptr != 0 || errno != 0) >>> return -1; >>> >>> + /* SOCKET_ID_ANY also consider a valid socket id */ >>> + if ((int8_t)socket_id == SOCKET_ID_ANY) { >>> + *(int *)extra_args = SOCKET_ID_ANY; >>> + return 0; >>> + } >>> + >>> /* validate socket id value */ >>> if (socket_id >= 0 && socket_id < RTE_MAX_NUMA_NODES) { >>> *(int *)extra_args = (int)socket_id; diff --git >>> a/drivers/net/bonding/rte_eth_bond_pmd.c >>> b/drivers/net/bonding/rte_eth_bond_pmd.c >>> index f0c4f7d26b..390a5b4271 100644 >>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c >>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c >>> @@ -3604,7 +3604,7 @@ static int >>> bond_alloc(struct rte_vdev_device *dev, uint8_t mode) >>> { >>> const char *name = rte_vdev_device_name(dev); >>> - uint8_t socket_id = dev->device.numa_node; >>> + int socket_id = dev->device.numa_node; >> Well, other point should be also modified, like : >> >> *** >> >> "socket %u.", name, bonding_mode, socket_id); >> >> *** >> >> %u -- > %d. > Okay, I will send a v2 patch fix this. > >> BTW, I think there is no need to add args like "socket_id=-1..." if we know >> this server does not support NUMA. >> >> Default socket id is -1, so this is meaningless. >> > We found this bug when running 'dperf' app, and it is the 'dperf' app add this 'socket_id=-1' args. > Maybe the 'dperf' app should change its logic? > Please help correct me if I misunderstood, thanks. I agree with your patch. What I mentioned is just for further discussion. > >>> struct bond_dev_private *internals = NULL; >>> struct rte_eth_dev *eth_dev = NULL; >>> uint32_t vlan_filter_bmp_size;