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 A7AB7A0560; Tue, 18 Oct 2022 12:34:52 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 493104021E; Tue, 18 Oct 2022 12:34:52 +0200 (CEST) Received: from mail-qt1-f178.google.com (mail-qt1-f178.google.com [209.85.160.178]) by mails.dpdk.org (Postfix) with ESMTP id 64A4E4021D for ; Tue, 18 Oct 2022 12:34:50 +0200 (CEST) Received: by mail-qt1-f178.google.com with SMTP id hh9so9329283qtb.13 for ; Tue, 18 Oct 2022 03:34:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=hTS/2LLxeYWW85p8voaNr0oN1dFjhgSZHWOel/Ykd+M=; b=jJoDjDt3HQezZupE2rxu5zEy6q1sMX2PF+YCRX8u2LxoL2Pue47ZkFLmZT5gMsVrZb g+30vbzDAGIjSmog3eSp/TgnI8h4VbKD5zO96QFGYVBovl2pA7PScWKKsZBlj0HksQDp Fa5/orwQQhuv7bemMCrr0DrgN7+RhNJ30luF/xoHZolcG2muTC+g+hvTQXDn91/jx4aE gzxrO6BYZApR+BGNBNYYPKbvmgtHRgr2IxN1/dQF/a9CO6F5Z2Qq2lutSb5+YCIcwLci UlqNkmWZJrSjnggUI9AW0oKyPH7c3DsAhbXqJfN4XqEKMk/C/ZmSx3vaQVGGjKyR+w3H D3OQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=hTS/2LLxeYWW85p8voaNr0oN1dFjhgSZHWOel/Ykd+M=; b=QrmmBNYhHx0HSBn5FglCkoKLuIAH87a6wczgcaQ/G4cdiFYDJgHwgYj0LD1r8KFS7U Yh7d6S4fRqFYK/WMTjnxtW3rHCv5kkmBOAnMcKaZv7bm+GgUU/jfZ/dObdjTijeWZWaX E1mTX/Q+fVIfzt56v+/PTS/iwIGRj2h1mO53QVfpSvXV5mN48DYoqMwAED2s4kuRqFNS yxUHBtctWx+fSghyWEeH3M0aaZPGGSBxJCD+7/f88CG5OLXmNhVCf3PvKbuAbNq5EEai 4mDlG72Pwt0Rs+rWvZE7K+kNeu0AwCrpp0lGswNho3O3gGXUY5MWfPEx1ehIf/MgMRcA r9Tw== X-Gm-Message-State: ACrzQf0Kh/UFZfyuiiwleqBBab03MN9ngHQ9sTkYVYb2FCnJE1lkfLfo j9O4V1AgO8RtAtp00eOcIKg= X-Google-Smtp-Source: AMsMyM4/edLYe5hXGgv3GJ0HUlcz5Qt4oJg/egWhO0cs0dytW9UdgjGRzZRbWAiMxRji4a4hFY3zDg== X-Received: by 2002:a05:622a:7:b0:39c:14ff:32bc with SMTP id x7-20020a05622a000700b0039c14ff32bcmr1421780qtw.125.1666089289944; Tue, 18 Oct 2022 03:34:49 -0700 (PDT) Received: from ?IPV6:2600:4040:225b:ea00:6063:8c9b:774a:6cf4? ([2600:4040:225b:ea00:6063:8c9b:774a:6cf4]) by smtp.googlemail.com with ESMTPSA id m2-20020a05620a290200b006ce40fbb8f6sm2323637qkp.21.2022.10.18.03.34.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 18 Oct 2022 03:34:49 -0700 (PDT) Message-ID: Date: Tue, 18 Oct 2022 06:34:48 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH] net/bonding: make bonded device configure method re-entrant Content-Language: en-US To: Andrew Rybchenko , Chas Williams , "Min Hu (Connor)" Cc: Anatoly Burakov , Ivan Malov , dev@dpdk.org References: <20220911122405.3995083-1-ivan.malov@oktetlabs.ru> <5138b2cb-2366-4e54-12fe-5a7f9c1af0d9@oktetlabs.ru> From: Chas Williams <3chas3@gmail.com> In-Reply-To: <5138b2cb-2366-4e54-12fe-5a7f9c1af0d9@oktetlabs.ru> 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 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. >>> >