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 D4AA4A00C3; Sat, 16 May 2020 23:03:59 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 898931D6E1; Sat, 16 May 2020 23:03:42 +0200 (CEST) Received: from huawei.com (szxga05-in.huawei.com [45.249.212.191]) by dpdk.org (Postfix) with ESMTP id 84B811D540 for ; Sat, 16 May 2020 11:31:42 +0200 (CEST) Received: from DGGEMS404-HUB.china.huawei.com (unknown [172.30.72.59]) by Forcepoint Email with ESMTP id D99074522E783660C9CD; Sat, 16 May 2020 17:31:25 +0800 (CST) Received: from [127.0.0.1] (10.69.31.206) by DGGEMS404-HUB.china.huawei.com (10.3.19.204) with Microsoft SMTP Server id 14.3.487.0; Sat, 16 May 2020 17:31:17 +0800 To: References: <750fc71a-e4fc-d5a7-9bca-551d7ed4b32c@intel.com> <1f71bc2e-e00c-8700-2850-19a5e42b8b94@163.com> CC: <3chas3@gmail.com>, , , , From: "Wei Hu (Xavier)" Message-ID: <99f38f50-7f43-5a8f-2e7c-eb4cce4fe5a2@huawei.com> Date: Sat, 16 May 2020 17:31:18 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <1f71bc2e-e00c-8700-2850-19a5e42b8b94@163.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.69.31.206] X-CFilter-Loop: Reflected X-Mailman-Approved-At: Sat, 16 May 2020 23:03:38 +0200 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" On 2020/5/16 16:59, Wei Hu (Xavier) wrote: > > > > -------- Forwarded Message -------- > Subject: Re: [dpdk-dev] [PATCH v2 2/2] net/bonding: fix MAC address > when one port resets > Date: Fri, 15 May 2020 11:46:30 +0100 > From: Ferruh Yigit > To: Wei Hu (Xavier) , Chas Williams > <3chas3@gmail.com> > CC: dev@dpdk.org, Thomas Monjalon , David > Marchand > > On 5/15/2020 4:11 AM, Wei Hu (Xavier) wrote: >> Hi, Ferruh Yigit & Chas Williams >> Could you please give any suggestion? > > Hi Xavier, > > Unfortunately we are missing reviews in bonding patches. There are a > few more > waiting other than this one. > > If you are already working on it and have enough information about the > code, > what do you think adding you as the additional maintainer? > Hi, Ferruh Yigit I'm glad to do something like this. Thanks, Xavier > Thanks, > ferruh > >> 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; >>>> > > . >