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 21482A0560; Tue, 18 Oct 2022 14:44:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CA55440395; Tue, 18 Oct 2022 14:44:48 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 718E54021E for ; Tue, 18 Oct 2022 14:44:47 +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) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id B3AA869; Tue, 18 Oct 2022 15:44:46 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru B3AA869 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1666097086; bh=vOH6kaM84ooP4Y1bBe9QMktbtXBizLhtdxap5sPiFl0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=XXAMCQKos3HZS3yL59QKxkp1SeRFEc8VR7Y/4XB+KNoY9H4cT8uAY49IB23iw6dJz Kgh7T4/aJMHlFbE5fqPoSDaKP1YUOL6exwMI7V2ta32Mlvul1i6Ce+xtPZbz4JC1mQ Ovf+IlYgupa8PrpaRUpVUzABtnqyAPyO1E3O65l4= Message-ID: Date: Tue, 18 Oct 2022 15:44:46 +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: Ivan Malov Cc: Anatoly Burakov , dev@dpdk.org, Chas Williams , Chas Williams <3chas3@gmail.com>, "Min Hu (Connor)" References: <20220911122405.3995083-1-ivan.malov@oktetlabs.ru> <5138b2cb-2366-4e54-12fe-5a7f9c1af0d9@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 Ivan, could you rebase the patch, please, and send a new version. Thanks, Andrew. On 10/18/22 13:34, Chas Williams wrote: > > > On 10/17/22 10:10, Andrew Rybchenko wrote: >> 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? > > Acked-by: Chas Williams <3chas3@gmail.com> > >>> >>>>>       /* >>>>>        * If RSS is enabled, fill table with default values and >>>>>        * set key to the value specified in port RSS configuration. >>>> >>