Skip to contentSkip to conversation

net/bond: lacp slave deactivate slave behavioural fix

PATCH Applied Declan Doherty 2 participants 2 messages Oct 6, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 net/bond: lacp slave deactivate slave behavioural fix

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")
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 37 +++++++++++++------------------
 1 file changed, 15 insertions(+), 22 deletions(-)
drivers/net/bonding/rte_eth_bond_8023ad.c +15 -22
index 20b5a89..e15580f 100644
@@ -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);
 
Signature · 1 line
2.9.4
Applied
#1 lore raw
Quoted 9 lines · 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")

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

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