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 ADC8BA034F; Fri, 15 May 2020 05:12:11 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9AB081D9D9; Fri, 15 May 2020 05:12:10 +0200 (CEST) Received: from mail.chinasoftinc.com (unknown [114.113.233.8]) by dpdk.org (Postfix) with ESMTP id 01D8D1D9D8 for ; Fri, 15 May 2020 05:12:04 +0200 (CEST) Received: from [192.168.1.199] (139.159.243.11) by INCCAS002.ito.icss (10.168.0.60) with Microsoft SMTP Server id 14.3.487.0; Fri, 15 May 2020 11:11:55 +0800 From: "Wei Hu (Xavier)" To: Chas Williams <3chas3@gmail.com>, CC: References: <20200417081918.14029-1-huwei013@chinasoftinc.com> <20200417081918.14029-3-huwei013@chinasoftinc.com> <51349b04-6b23-69d8-69f8-bde7f54c866e@chinasoftinc.com> Message-ID: <24fd0081-edf3-326c-2a14-58c246bd24f7@chinasoftinc.com> Date: Fri, 15 May 2020 11:11:55 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <51349b04-6b23-69d8-69f8-bde7f54c866e@chinasoftinc.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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, Ferruh Yigit & Chas Williams Could you please give any suggestion? Thanks. Best Regards Xavier On 2020/4/17 16:58, Wei Hu (Xavier) wrote: > 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; >>