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 636EBA0560; Mon, 17 Oct 2022 16:10:21 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 48C4B4021D; Mon, 17 Oct 2022 16:10:21 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id EAED540143 for ; Mon, 17 Oct 2022 16:10:19 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 7A08784; Mon, 17 Oct 2022 17:10:19 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 7A08784 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1666015819; bh=84pkT7Cph/QAO7Xi+ddbme8m8XGesDa406Nmf/EsfWk=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=qz9BbdHHucbbU4sbWuSS4c/u/Y0R408suBeU64dB71t8hotBAEbM/S1KyTh9Lvb80 W4ikiGZMzrleWVaApMyi15PQxSAhDqbzyOrEZB3xiD8yh9KhRM6R+zCahXL+wuKXRv 91j6ub1iekhY7YjeRTNHt6QnpVAZ8uvU8kHG/63M= Message-ID: <5138b2cb-2366-4e54-12fe-5a7f9c1af0d9@oktetlabs.ru> Date: Mon, 17 Oct 2022 17:10:19 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.0 Subject: Re: [PATCH] net/bonding: make bonded device configure method re-entrant Content-Language: en-US To: Chas Williams <3chas3@gmail.com>, Chas Williams , "Min Hu (Connor)" Cc: Anatoly Burakov , Ivan Malov , dev@dpdk.org References: <20220911122405.3995083-1-ivan.malov@oktetlabs.ru> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 On 10/17/22 15:32, Chas Williams wrote: > This appears to be correct. A minor comment inline. > > On 10/17/22 04:42, Andrew Rybchenko wrote: >> Chas, Cornor, could you review the patch, please. >> >> Thanks, >> Andrew. >> >> On 9/11/22 15:24, Ivan Malov wrote: >>> According to the documentation, rte_eth_dev_configure() >>> can be invoked repeatedly while in stopped state. >>> The current implementation in the bonding driver >>> allows for that (technically), but the user sees >>> warnings which say that back-end devices have >>> already been harnessed. Re-factor the code >>> to have cleanup before each (re-)configure. >>> >>> Signed-off-by: Ivan Malov >>> Reviewed-by: Andrew Rybchenko >>> --- >>>   drivers/net/bonding/rte_eth_bond_pmd.c | 27 ++++++++++++++++++-------- >>>   1 file changed, 19 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c >>> b/drivers/net/bonding/rte_eth_bond_pmd.c >>> index d01c954296..92a33b45bd 100644 >>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c >>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c >>> @@ -2143,18 +2143,13 @@ bond_ethdev_stop(struct rte_eth_dev *eth_dev) >>>       return 0; >>>   } >>> -int >>> -bond_ethdev_close(struct rte_eth_dev *dev) >>> +static void >>> +bond_ethdev_cfg_cleanup(struct rte_eth_dev *dev) >>>   { >>>       struct bond_dev_private *internals = dev->data->dev_private; >>>       uint16_t bond_port_id = internals->port_id; >>> -    int skipped = 0; >>>       struct rte_flow_error ferror; >>> - >>> -    if (rte_eal_process_type() != RTE_PROC_PRIMARY) >>> -        return 0; >>> - >>> -    RTE_BOND_LOG(INFO, "Closing bonded device %s", dev->device->name); >>> +    int skipped = 0; >>>       /* Flush flows in all back-end devices before removing them */ >>>       bond_flow_ops.flush(dev, &ferror); >>> @@ -2176,6 +2171,20 @@ bond_ethdev_close(struct rte_eth_dev *dev) >>>               skipped++; >>>           } >>>       } >>> +} >>> + >>> +int >>> +bond_ethdev_close(struct rte_eth_dev *dev) >>> +{ >>> +    struct bond_dev_private *internals = dev->data->dev_private; >>> + >>> +    if (rte_eal_process_type() != RTE_PROC_PRIMARY) >>> +        return 0; >>> + >>> +    RTE_BOND_LOG(INFO, "Closing bonded device %s", dev->device->name); >>> + >>> +    bond_ethdev_cfg_cleanup(dev); >>> + >>>       bond_ethdev_free_queues(dev); >>>       rte_bitmap_reset(internals->vlan_filter_bmp); >>>       rte_bitmap_free(internals->vlan_filter_bmp); >>> @@ -3606,6 +3615,8 @@ bond_ethdev_configure(struct rte_eth_dev *dev) >>>       unsigned i, j; >>> +    bond_ethdev_cfg_cleanup(dev); >>> + > > You might want a space after the variable declaration section. It makes > it easier to read. I can fix it on applying. May I add your ack? > >>>       /* >>>        * If RSS is enabled, fill table with default values and >>>        * set key to the value specified in port RSS configuration. >>