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 BE157A0A0C; Thu, 15 Apr 2021 09:09:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A67B9162079; Thu, 15 Apr 2021 09:09:49 +0200 (CEST) Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by mails.dpdk.org (Postfix) with ESMTP id A047416206F for ; Thu, 15 Apr 2021 09:09:48 +0200 (CEST) Received: from DGGEMS404-HUB.china.huawei.com (unknown [172.30.72.59]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4FLVlj1KXQz9y40; Thu, 15 Apr 2021 15:07:29 +0800 (CST) Received: from localhost.localdomain (10.69.192.56) by DGGEMS404-HUB.china.huawei.com (10.3.19.204) with Microsoft SMTP Server id 14.3.498.0; Thu, 15 Apr 2021 15:09:39 +0800 From: "Min Hu (Connor)" To: CC: , , Date: Thu, 15 Apr 2021 15:09:54 +0800 Message-ID: <1618470594-64612-1-git-send-email-humin29@huawei.com> X-Mailer: git-send-email 2.7.4 MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.69.192.56] X-CFilter-Loop: Reflected Subject: [dpdk-dev] [PATCH] net/bonding: fix add bonded device itself as its slave 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" From: Chengchang Tang Adding the bond device as its own slave should be forbidden. This will cause a recursive endless loop in many subsequent operations, and eventually lead to coredump. This problem was found in testpmd, the related logs are as follows: testpmd> create bonded device 1 0 Created new bonded device net_bonding_testpmd_0 on (port 4). testpmd> add bonding slave 4 4 Segmentation fault (core dumped) The call stack is as follows: 0x000000000064eb90 in rte_eth_dev_info_get () 0x00000000006df4b4 in bond_ethdev_info () 0x000000000064eb90 in rte_eth_dev_info_get () 0x00000000006df4b4 in bond_ethdev_info () 0x000000000064eb90 in rte_eth_dev_info_get () 0x0000000000564e58 in eth_dev_info_get_print_err () 0x000000000055e8a4 in init_port_config () 0x000000000052730c in cmd_add_bonding_slave_parsed () 0x0000000000646f60 in cmdline_parse () 0x0000000000645e08 in cmdline_valid_buffer () 0x000000000064956c in rdline_char_in () 0x0000000000645ee0 in cmdline_in () 0x00000000006460a4 in cmdline_interact () 0x0000000000531904 in prompt () 0x000000000051cca8 in main () Fixes: 2efb58cbab6e ("bond: new link bonding library") Cc: stable@dpdk.org Signed-off-by: Chengchang Tang Signed-off-by: Min Hu (Connor) --- drivers/net/bonding/eth_bond_private.h | 2 +- drivers/net/bonding/rte_eth_bond_api.c | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/net/bonding/eth_bond_private.h b/drivers/net/bonding/eth_bond_private.h index 75fb8dc..fc179a2 100644 --- a/drivers/net/bonding/eth_bond_private.h +++ b/drivers/net/bonding/eth_bond_private.h @@ -212,7 +212,7 @@ int valid_bonded_port_id(uint16_t port_id); int -valid_slave_port_id(uint16_t port_id, uint8_t mode); +valid_slave_port_id(struct bond_dev_private *internals, uint16_t port_id); void deactivate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id); diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c index 17e6ff8..eb8d15d 100644 --- a/drivers/net/bonding/rte_eth_bond_api.c +++ b/drivers/net/bonding/rte_eth_bond_api.c @@ -56,19 +56,25 @@ check_for_master_bonded_ethdev(const struct rte_eth_dev *eth_dev) } int -valid_slave_port_id(uint16_t port_id, uint8_t mode) +valid_slave_port_id(struct bond_dev_private *internals, uint16_t slave_port_id) { - RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1); + RTE_ETH_VALID_PORTID_OR_ERR_RET(slave_port_id, -1); - /* Verify that port_id refers to a non bonded port */ - if (check_for_bonded_ethdev(&rte_eth_devices[port_id]) == 0 && - mode == BONDING_MODE_8023AD) { + /* Verify that slave_port_id refers to a non bonded port */ + if (check_for_bonded_ethdev(&rte_eth_devices[slave_port_id]) == 0 && + internals->mode == BONDING_MODE_8023AD) { RTE_BOND_LOG(ERR, "Cannot add slave to bonded device in 802.3ad" " mode as slave is also a bonded device, only " "physical devices can be support in this mode."); return -1; } + if (internals->port_id == slave_port_id) { + RTE_BOND_LOG(ERR, + "Cannot add the bonded device itself as its slave."); + return -1; + } + return 0; } @@ -456,7 +462,7 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id) bonded_eth_dev = &rte_eth_devices[bonded_port_id]; internals = bonded_eth_dev->data->dev_private; - if (valid_slave_port_id(slave_port_id, internals->mode) != 0) + if (valid_slave_port_id(internals, slave_port_id) != 0) return -1; slave_eth_dev = &rte_eth_devices[slave_port_id]; @@ -605,13 +611,15 @@ rte_eth_bond_slave_add(uint16_t bonded_port_id, uint16_t slave_port_id) int retval; - /* Verify that port id's are valid bonded and slave ports */ if (valid_bonded_port_id(bonded_port_id) != 0) return -1; bonded_eth_dev = &rte_eth_devices[bonded_port_id]; internals = bonded_eth_dev->data->dev_private; + if (valid_slave_port_id(internals, slave_port_id) != 0) + return -1; + rte_spinlock_lock(&internals->lock); retval = __eth_bond_slave_add_lock_free(bonded_port_id, slave_port_id); @@ -635,7 +643,7 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, bonded_eth_dev = &rte_eth_devices[bonded_port_id]; internals = bonded_eth_dev->data->dev_private; - if (valid_slave_port_id(slave_port_id, internals->mode) < 0) + if (valid_slave_port_id(internals, slave_port_id) < 0) return -1; /* first remove from active slave list */ @@ -783,7 +791,7 @@ rte_eth_bond_primary_set(uint16_t bonded_port_id, uint16_t slave_port_id) internals = rte_eth_devices[bonded_port_id].data->dev_private; - if (valid_slave_port_id(slave_port_id, internals->mode) != 0) + if (valid_slave_port_id(internals, slave_port_id) != 0) return -1; internals->user_defined_primary_port = 1; -- 2.7.4