[dpdk-dev] [PATCH] net/bond: lacp slave deactivate slave behavioural fix

[dpdk-dev] [PATCH] net/bond: lacp slave deactivate slave behavioural fix

[Declan Doherty]

During a link down event of a port participating in a LACP 802.3ad
bond the current behaviour can cause all ports to be unselected
and temporarily stop all traffic on the bond, causing unexpected
traffic loss across all ports and not just the port which was
affected by the link down event.

Fixes: 46fb43683679 ("bond: add mode 4")

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 37 +++++++++++++------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index 20b5a89..e15580f 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1021,37 +1021,30 @@ bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev, uint8_t slave_id)
 }
 
 int
-bond_mode_8023ad_deactivate_slave(struct rte_eth_dev *bond_dev,
+bond_mode_8023ad_deactivate_slave(struct rte_eth_dev *bond_dev __rte_unused,
 		uint8_t slave_id)
 {
-	struct bond_dev_private *internals = bond_dev->data->dev_private;
 	void *pkt = NULL;
-	struct port *port;
-	uint8_t i;
+	struct port *port = NULL;
+	uint8_t old_partner_state;
 
-	/* Given slave must be in active list */
-	RTE_ASSERT(find_slave_by_id(internals->active_slaves,
-	internals->active_slave_count, slave_id) < internals->active_slave_count);
+	port = &mode_8023ad_ports[slave_id];
 
-	/* Exclude slave from transmit policy. If this slave is an aggregator
-	 * make all aggregated slaves unselected to force selection logic
-	 * to select suitable aggregator for this port. */
-	for (i = 0; i < internals->active_slave_count; i++) {
-		port = &mode_8023ad_ports[internals->active_slaves[i]];
-		if (port->aggregator_port_id != slave_id)
-			continue;
+	ACTOR_STATE_CLR(port, AGGREGATION);
+	port->selected = UNSELECTED;
 
-		port->selected = UNSELECTED;
+	old_partner_state = port->partner_state;
+	record_default(port);
 
-		/* Use default aggregator */
-		port->aggregator_port_id = internals->active_slaves[i];
-	}
+	/* If partner timeout state changes then disable timer */
+	if (!((old_partner_state ^ port->partner_state) &
+			STATE_LACP_SHORT_TIMEOUT))
+		timer_cancel(&port->current_while_timer);
 
-	port = &mode_8023ad_ports[slave_id];
-	port->selected = UNSELECTED;
-	port->actor_state &= ~(STATE_SYNCHRONIZATION | STATE_DISTRIBUTING |
-			STATE_COLLECTING);
+	PARTNER_STATE_CLR(port, AGGREGATION);
+	ACTOR_STATE_CLR(port, EXPIRED);
 
+	/* flush rx/tx rings */
 	while (rte_ring_dequeue(port->rx_ring, &pkt) == 0)
 		rte_pktmbuf_free((struct rte_mbuf *)pkt);
 
-- 
2.9.4

Re: [dpdk-dev] [PATCH] net/bond: lacp slave deactivate slave behavioural fix

[Ferruh Yigit]

On 10/6/2017 10:21 AM, Declan Doherty wrote:
> During a link down event of a port participating in a LACP 802.3ad
> bond the current behaviour can cause all ports to be unselected
> and temporarily stop all traffic on the bond, causing unexpected
> traffic loss across all ports and not just the port which was
> affected by the link down event.
> 
> Fixes: 46fb43683679 ("bond: add mode 4")
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

Applied to dpdk-next-net/master, thanks.

(There was a merge conflict because of port_id patch, can you please
confirm the final commit?)