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 5A2FAA0560; Mon, 17 Oct 2022 14:32:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EBAB54021D; Mon, 17 Oct 2022 14:32:23 +0200 (CEST) Received: from mail-qv1-f54.google.com (mail-qv1-f54.google.com [209.85.219.54]) by mails.dpdk.org (Postfix) with ESMTP id 68F5640143 for ; Mon, 17 Oct 2022 14:32:22 +0200 (CEST) Received: by mail-qv1-f54.google.com with SMTP id f14so7294034qvo.3 for ; Mon, 17 Oct 2022 05:32:22 -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=pWJCzEDikA8xkTk+qb9g/AlcNuLuqPO/JgYVubeOGIE=; b=hAyFgnVGEl2iL+TMqE2HzpHhzsbJMLOKQPV8txMk6+oQycqfKVX9QyhQ6l2+tdPsvk ymB8m1nunnNyOMeJbcLlYLT1sPlcKc4o1RcyYYeaEPvi2GZcjKP9FoIaWiaTSd5rx6xx fxPHYsFNzFmrxfcRc0R9VKHELqjdjOTD6qpsQ+gU70j3vZHOyl3ZNv+K6YMryPNlL8Uh loSUZtk3aghyh7RBeHw/C08GUE2w/wmLnWfN0S/cQb5+UEcgdUJRCoVeU5l4Zp51hts+ rquCz0F5vSOVKegu91Mb7MB7vjLOiD7s8iRcr0k3pCnzk/Ac0CGVGxc/v+4LxmHwSWOB ZY+g== 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=pWJCzEDikA8xkTk+qb9g/AlcNuLuqPO/JgYVubeOGIE=; b=Q9P6vyGujZ3ckabKs+NmDkTiCOnVUwmKB6f/QlgHkSIHTXGSDsOJaChSkIttM4kTPi KnWYqCoR2/Le9+LGQzJYsBFGbbhjjo+sTW9S608/nx3gllbYx+NCjB5LszoTP1DMZxzx fK1YOQub0sWqHk48692dmG4St2lT+5lmhhQtJ8ruDCaUennNhmfZvYDpWh2eE0hcapJv zvUEMdpQE1UdPqKIkZabmSozpCOad84ns4csKdIS0GAbfFVkDdQt3DVJS50xrQxRzKcN ZmdU2kRqj3W6c+f+4H2rABd0/uA9Q+qEaRxrkTS08EzooKAQV8lnvRQSyUcMOQpQ4lIP 1XaA== X-Gm-Message-State: ACrzQf0nRw3hQDjxtjm3JgydeCuS1FfiXIpr8eMgy9wYGturvsk52eeI JWcxGn+Jx/5W3y7AgxZve/A= X-Google-Smtp-Source: AMsMyM78ZNPJt/6w/LwcRXGm+suzTpOTpc+35OlTt1VJYL7Np4VDTGwRVA6M720rYRz5qYPynolzGw== X-Received: by 2002:a05:6214:2349:b0:4b4:11bf:a743 with SMTP id hu9-20020a056214234900b004b411bfa743mr8246388qvb.95.1666009941662; Mon, 17 Oct 2022 05:32:21 -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 66-20020a370745000000b006cfc1d827cbsm8975173qkh.9.2022.10.17.05.32.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 17 Oct 2022 05:32:20 -0700 (PDT) Message-ID: Date: Mon, 17 Oct 2022 08:32:20 -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> From: Chas Williams <3chas3@gmail.com> 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 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. >>       /* >>        * If RSS is enabled, fill table with default values and >>        * set key to the value specified in port RSS configuration. >