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 B1642A0527; Sat, 18 Jul 2020 09:08:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DF97F1BF5B; Sat, 18 Jul 2020 09:08:38 +0200 (CEST) Received: from huawei.com (szxga06-in.huawei.com [45.249.212.32]) by dpdk.org (Postfix) with ESMTP id 3FE181BED0 for ; Sat, 18 Jul 2020 09:08:36 +0200 (CEST) Received: from DGGEMS412-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id A88AAF7E0C64FEE88615; Sat, 18 Jul 2020 15:08:29 +0800 (CST) Received: from [10.69.31.206] (10.69.31.206) by DGGEMS412-HUB.china.huawei.com (10.3.19.212) with Microsoft SMTP Server id 14.3.487.0; Sat, 18 Jul 2020 15:08:21 +0800 To: Weifeng Li References: <20200718040531.2164-1-liweifeng96@126.com> <20200718043538.2315-1-liweifeng96@126.com> CC: , "Wei Hu (Xavier)" , , , Ferruh Yigit , Chas Williams From: "Wei Hu (Xavier)" Message-ID: <4da342e4-431b-d0ee-0f6c-13d1f6080eab@huawei.com> Date: Sat, 18 Jul 2020 15:08:21 +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: <20200718043538.2315-1-liweifeng96@126.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.69.31.206] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v4] net/bonding: change the state machine to defaulted 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, Weifeng Li On 2020/7/18 12:35, Weifeng Li wrote: > A dpdk bonding 802.3ad network as follows: > +----------+ +-----------+ > |dpdk lacp |bond1.1 <------> bond2.1|switch lacp| > | |bond1.2 <------> bond2.2| | > +----------+ +-----------+ > If a fiber optic go wrong about single pass during normal running like > this: > bond1.2 -----> bond2.2 ok > bond1.2 <--x-- bond2.2 error: bond1.2 receive no LACPDU Some packets from > switch to dpdk will choose bond2.2 and lost. > > DPDK lacp state machine will transits to the expired state if no LACPDU > is received before the current_while_timer expires. But if no LACPDU is > received before the current_while_timer expires again, DPDK lacp state > machine has no change. Bond2.2 can not change to inactive depend on the > received LACPDU. > According to IEEE 802.3ad, if no lacpdu is received before the > current_while_timer expires again, the state machine should transits from > expired to defaulted. Bond2.2 will change to inactive depend on the LACPDU > with defaulted state. > > This patch adds a state machine change from expired to defaulted when no > lacpdu is received before the current_while_timer expires again according > to IEEE 802.3ad: > If no LACPDU is received before the current_while timer expires again, > the state machine transits to the DEFAULTED state. The record Default > function overwrites the current operational parameters for the Partner > with administratively configured values. This allows configuration of > aggregations and individual links when no protocol partner is present, > while still permitting an active partner to override default settings. > The update_Default_Selected function sets the Selected variable FALSE > if the Link Aggregation Group has changed. Since all operational parameters > are now set to locally administered values there can be no disagreement as > to the Link Aggregation Group, so the Matched variable is set TRUE. > > The relevant description is in the chapter 43.4.12 of the link below: > https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=850426 Fixes: 46fb43683679 ("bond: add mode 4") Cc: stable@dpdk.org > Signed-off-by: Weifeng Li Acked-by: Wei Hu (Xavier) Thanks you for fixing this issue. Xavier > --- > v1 -> v2: adjust the formate of commit log > --- > v2 -> v3: > 1. adjust coding style issue. > 2. add description about partner_admin from IEEE spec. > --- > v3 -> v4 > 1. adjust coding style issue: Missing Signed-off-by > --- > drivers/net/bonding/eth_bond_8023ad_private.h | 3 +++ > drivers/net/bonding/rte_eth_bond_8023ad.c | 21 +++++++++++++++++---- > 2 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/bonding/eth_bond_8023ad_private.h b/drivers/net/bonding/eth_bond_8023ad_private.h > index 6e44ffd..9b5738a 100644 > --- a/drivers/net/bonding/eth_bond_8023ad_private.h > +++ b/drivers/net/bonding/eth_bond_8023ad_private.h > @@ -50,6 +50,7 @@ > #define SM_FLAGS_MOVED 0x0100 > #define SM_FLAGS_PARTNER_SHORT_TIMEOUT 0x0200 > #define SM_FLAGS_NTT 0x0400 > +#define SM_FLAGS_EXPIRED 0x0800 > > #define BOND_LINK_FULL_DUPLEX_KEY 0x01 > #define BOND_LINK_SPEED_KEY_10M 0x02 > @@ -103,6 +104,8 @@ struct port { > > /** The operational Partner's port parameters */ > struct port_params partner; > + /** Partner administrative parameter values */ > + struct port_params partner_admin; > > /* Additional port parameters not listed in documentation */ > /** State machine flags */ > diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c > index 3991825..3c2c7bf 100644 > --- a/drivers/net/bonding/rte_eth_bond_8023ad.c > +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c > @@ -356,16 +356,28 @@ rx_machine(struct bond_dev_private *internals, uint16_t slave_id, > > timer_set(&port->current_while_timer, timeout); > ACTOR_STATE_CLR(port, EXPIRED); > + SM_FLAG_CLR(port, EXPIRED); > return; /* No state change */ > } > > /* If CURRENT state timer is not running (stopped or expired) > * transit to EXPIRED state from DISABLED or CURRENT */ > if (!timer_is_running(&port->current_while_timer)) { > - ACTOR_STATE_SET(port, EXPIRED); > - PARTNER_STATE_CLR(port, SYNCHRONIZATION); > - PARTNER_STATE_SET(port, LACP_SHORT_TIMEOUT); > - timer_set(&port->current_while_timer, internals->mode4.short_timeout); > + if (SM_FLAG(port, EXPIRED)) { > + port->selected = UNSELECTED; > + memcpy(&port->partner, &port->partner_admin, > + sizeof(struct port_params)); > + record_default(port); > + ACTOR_STATE_CLR(port, EXPIRED); > + timer_cancel(&port->current_while_timer); > + } else { > + SM_FLAG_SET(port, EXPIRED); > + ACTOR_STATE_SET(port, EXPIRED); > + PARTNER_STATE_CLR(port, SYNCHRONIZATION); > + PARTNER_STATE_SET(port, LACP_SHORT_TIMEOUT); > + timer_set(&port->current_while_timer, > + internals->mode4.short_timeout); > + } > } > } > > @@ -1021,6 +1033,7 @@ bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev, > port->actor.port_number = rte_cpu_to_be_16(slave_id + 1); > > memcpy(&port->partner, &initial, sizeof(struct port_params)); > + memcpy(&port->partner_admin, &initial, sizeof(struct port_params)); > > /* default states */ > port->actor_state = STATE_AGGREGATION | STATE_LACP_ACTIVE | STATE_DEFAULTED;