From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9C537A058A; Fri, 17 Apr 2020 10:58:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7054D1DEAA; Fri, 17 Apr 2020 10:58:39 +0200 (CEST) Received: from mail.chinasoftinc.com (unknown [114.113.233.8]) by dpdk.org (Postfix) with ESMTP id 8D1231DEAA for ; Fri, 17 Apr 2020 10:58:05 +0200 (CEST) Received: from [192.168.1.199] (139.159.243.11) by INCCAS001.ito.icss (10.168.0.60) with Microsoft SMTP Server id 14.3.487.0; Fri, 17 Apr 2020 16:58:02 +0800 From: "Wei Hu (Xavier)" To: Chas Williams <3chas3@gmail.com> References: <20200417081918.14029-1-huwei013@chinasoftinc.com> <20200417081918.14029-3-huwei013@chinasoftinc.com> CC: Message-ID: <51349b04-6b23-69d8-69f8-bde7f54c866e@chinasoftinc.com> Date: Fri, 17 Apr 2020 16:58:01 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200417081918.14029-3-huwei013@chinasoftinc.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [139.159.243.11] Subject: Re: [dpdk-dev] [PATCH v2 2/2] net/bonding: fix MAC address when one port resets X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Hi, Chas Williams Thanks for your comments on Patch V1. now we have sent Patch V2. Could you please give some suggestion on them? Thanks. Best Regards Xavier On 2020/4/17 16:19, Wei Hu (Xavier) wrote: > From: "Wei Hu (Xavier)" > > Currently, based on a active-backup bond device, in the following 2 cases: > 1) The primary port resets. The link status of the primary port changes > from up to down. > 2) When switching the active port, one slave port resets at the same time. > one slave port changes to the primary port, but the new primary port's MAC > address probably cannot change to the bond device's MAC address. And we > can't continue receive packets whose destination MAC addresses are the same > as the bond devices's MAC address. > > The current bonding PMD driver call mac_address_slaves_update function to > modify the MAC address of all slaves devices. In mac_address_slaves_update > function, the rte_eth_dev_default_mac_addr_set API function is called to > set the MAC address of the slave devices in turn in the for loop statement. > > When one port reset, calling rte_eth_dev_default_mac_addr_set API fails > because the firmware will not respond to the commands from the driver, > and exit the loop, so other slave devices cannot continue to update the > MAC address. > > This patch fixes the issue by avoid exiting the loop when calling > rte_eth_dev_default_mac_addr_set fails. > > Fixes: 2efb58cbab6e ("bond: new link bonding library") > Cc: stable@dpdk.org > > Signed-off-by: Hongbo Zheng > Signed-off-by: Wei Hu (Xavier) > Signed-off-by: Chunsong Feng > Signed-off-by: Xuan Li > --- > v1 -> v2: > Ignore the failure when updating salves's MAC address in the > mac_address_slaves_update function, because it doesn't affect > the bond's functional characteristics. The related link about > the discussion: > http://patches.dpdk.org/patch/66033/ > --- > drivers/net/bonding/rte_eth_bond_pmd.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index ddae3518c..01c0f6eb1 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1502,6 +1502,7 @@ int > mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev) > { > struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; > + bool setted; > int i; > > /* Update slave devices MAC addresses */ > @@ -1529,6 +1530,7 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev) > case BONDING_MODE_TLB: > case BONDING_MODE_ALB: > default: > + setted = true; > for (i = 0; i < internals->slave_count; i++) { > if (internals->slaves[i].port_id == > internals->current_primary_port) { > @@ -1537,7 +1539,7 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev) > bonded_eth_dev->data->mac_addrs)) { > RTE_BOND_LOG(ERR, "Failed to update port Id %d MAC address", > internals->current_primary_port); > - return -1; > + setted = false; > } > } else { > if (rte_eth_dev_default_mac_addr_set( > @@ -1545,10 +1547,11 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev) > &internals->slaves[i].persisted_mac_addr)) { > RTE_BOND_LOG(ERR, "Failed to update port Id %d MAC address", > internals->slaves[i].port_id); > - return -1; > } > } > } > + if (!setted) > + return -1; > } > > return 0; >