From patchwork Fri Jan 16 15:57:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Toppins X-Patchwork-Id: 429914 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 0CD1E14027C for ; Sat, 17 Jan 2015 02:57:54 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756547AbbAPP5q (ORCPT ); Fri, 16 Jan 2015 10:57:46 -0500 Received: from mail-qg0-f48.google.com ([209.85.192.48]:57968 "EHLO mail-qg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751604AbbAPP5o (ORCPT ); Fri, 16 Jan 2015 10:57:44 -0500 Received: by mail-qg0-f48.google.com with SMTP id j5so16877300qga.7 for ; Fri, 16 Jan 2015 07:57:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=kaxH+ARZpkbbij10sWb5BH3TmDD/pp2439DWuRVdTjM=; b=TWxgKqfhFsa+PgLdIHcn0rItMUxGYzSE9HwZCbVVOeA/vQctOVSpA29bEplZOQ8yTB 8ODO6SXNzScQHaxou5+r+aQRDVfnmyFhOYrzvP3iUvx9Zy+3blehVAxiWHJIIQZ79U2w E16PKq+22ar8LDu6HcW+Nyj3Ts+Tvut+S+1vI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=kaxH+ARZpkbbij10sWb5BH3TmDD/pp2439DWuRVdTjM=; b=AbhfQQZmNyjd4U46Hqrtuq9pXtR5frGCxbl07/A6wIrwJUOv7iNuYuMv4AJfWW8a4c fdQWotEEQUoc0BWiYsPGiSMWzr9Zt8eGSVwUR4Nw/v4HbARQqPxHzK5+syH4gwSUD86b BiMznne6s2ko5fMVFJ1ZtwnMBWUG6oQwgNw3Zz/ldFbDy/hbvLN+SwC8aTpKGR4O3hmX 2y0lWX3YnsNRzM5hAYdMfzV9970MI3doC9YIRJkvCMNiJHoJQe3HAdiI8H3RtpvwhNQc 9u84GAGKWoGTOwqAZQD3O7wctNXQNSlH9bhjGj/kwxqT7o67Yg1bqOe4VgHdpTnb6E1t C6RA== X-Gm-Message-State: ALoCoQl8O/dNwdIJIFEEIBf9NwbpR9dJsjZk53WcgMIXSND5WaGupfEmY8AAZUjzlQOVjLlwHohz X-Received: by 10.224.96.199 with SMTP id i7mr26197778qan.77.1421423863601; Fri, 16 Jan 2015 07:57:43 -0800 (PST) Received: from debian-devel.rtplab.test (rrcs-24-171-185-114.midsouth.biz.rr.com. [24.171.185.114]) by mx.google.com with ESMTPSA id y95sm4475377qgy.14.2015.01.16.07.57.42 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Jan 2015 07:57:43 -0800 (PST) From: Jonathan Toppins To: netdev@vger.kernel.org Cc: Andy Gospodarek , Wilson Kok , Jonathan Toppins Subject: [PATCH net-next 3/5] bonding: fix incorrect lacp mux state when agg not active Date: Fri, 16 Jan 2015 10:57:26 -0500 Message-Id: <1421423848-414-4-git-send-email-jtoppins@cumulusnetworks.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1421423848-414-1-git-send-email-jtoppins@cumulusnetworks.com> References: <1421423848-414-1-git-send-email-jtoppins@cumulusnetworks.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Wilson Kok This patch attempts to fix the following problems when an actor or partner's aggregator is not active: 1. a slave's lacp port state is marked as AD_STATE_SYNCHRONIZATION even if it is attached to an inactive aggregator. LACP advertises this state to the partner, making the partner think he can move into COLLECTING_DISTRIBUTING state even though this link will not pass traffic on the local side 2. a slave goes into COLLECTING_DISTRIBUTING state without checking if the aggregator is actually active 3. when in COLLECTING_DISTRIBUTING state, the partner parameters may change, e.g. the partner_oper_port_state.SYNCHRONIZATION. The local mux machine is not reacting to the change and continue to keep the slave and bond up 4. When bond slave leaves an inactive aggregator and joins an active aggregator, the actor oper port state need to update to SYNC state. Cc: Andy Gospodarek Signed-off-by: Wilson Kok Signed-off-by: Jonathan Toppins --- drivers/net/bonding/bond_3ad.c | 44 ++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index e9b706f..52a8772 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -471,10 +471,13 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port) * and the port is matched */ if ((port->sm_vars & AD_PORT_MATCHED) - && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION)) + && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION)) { partner->port_state |= AD_STATE_SYNCHRONIZATION; - else + pr_debug("%s partner sync=1\n", port->slave->dev->name); + } else { partner->port_state &= ~AD_STATE_SYNCHRONIZATION; + pr_debug("%s partner sync=0\n", port->slave->dev->name); + } } } @@ -729,6 +732,8 @@ static inline void __update_lacpdu_from_port(struct port *port) lacpdu->actor_port_priority = htons(port->actor_port_priority); lacpdu->actor_port = htons(port->actor_port_number); lacpdu->actor_state = port->actor_oper_port_state; + pr_debug("update lacpdu: %s, actor port state %x\n", + port->slave->dev->name, port->actor_oper_port_state); /* lacpdu->reserved_3_1 initialized * lacpdu->tlv_type_partner_info initialized @@ -901,7 +906,9 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr) if ((port->sm_vars & AD_PORT_SELECTED) && (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) && !__check_agg_selection_timer(port)) { - port->sm_mux_state = AD_MUX_COLLECTING_DISTRIBUTING; + if (port->aggregator->is_active) + port->sm_mux_state = + AD_MUX_COLLECTING_DISTRIBUTING; } else if (!(port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY)) { /* if UNSELECTED or STANDBY */ @@ -913,12 +920,18 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr) */ __set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator)); port->sm_mux_state = AD_MUX_DETACHED; + } else if (port->aggregator->is_active) { + port->actor_oper_port_state |= + AD_STATE_SYNCHRONIZATION; } break; case AD_MUX_COLLECTING_DISTRIBUTING: if (!(port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY) || - !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION)) { + !(port->partner_oper.port_state & + AD_STATE_SYNCHRONIZATION) || + !(port->actor_oper_port_state & + AD_STATE_SYNCHRONIZATION)) { port->sm_mux_state = AD_MUX_ATTACHED; } else { /* if port state hasn't changed make @@ -940,8 +953,10 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr) /* check if the state machine was changed */ if (port->sm_mux_state != last_state) { - pr_debug("Mux Machine: Port=%d, Last State=%d, Curr State=%d\n", - port->actor_port_number, last_state, + pr_debug("Mux Machine: Port=%d (%s), Last State=%d, Curr State=%d\n", + port->actor_port_number, + port->slave->dev->name, + last_state, port->sm_mux_state); switch (port->sm_mux_state) { case AD_MUX_DETACHED: @@ -956,7 +971,12 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr) port->sm_mux_timer_counter = __ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0); break; case AD_MUX_ATTACHED: - port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION; + if (port->aggregator->is_active) + port->actor_oper_port_state |= + AD_STATE_SYNCHRONIZATION; + else + port->actor_oper_port_state &= + ~AD_STATE_SYNCHRONIZATION; port->actor_oper_port_state &= ~AD_STATE_COLLECTING; port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING; ad_disable_collecting_distributing(port, @@ -966,6 +986,7 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr) case AD_MUX_COLLECTING_DISTRIBUTING: port->actor_oper_port_state |= AD_STATE_COLLECTING; port->actor_oper_port_state |= AD_STATE_DISTRIBUTING; + port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION; ad_enable_collecting_distributing(port, update_slave_arr); port->ntt = true; @@ -1047,8 +1068,10 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port) /* check if the State machine was changed or new lacpdu arrived */ if ((port->sm_rx_state != last_state) || (lacpdu)) { - pr_debug("Rx Machine: Port=%d, Last State=%d, Curr State=%d\n", - port->actor_port_number, last_state, + pr_debug("Rx Machine: Port=%d (%s), Last State=%d, Curr State=%d\n", + port->actor_port_number, + port->slave->dev->name, + last_state, port->sm_rx_state); switch (port->sm_rx_state) { case AD_RX_INITIALIZE: @@ -1397,6 +1420,9 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr) aggregator = __get_first_agg(port); ad_agg_selection_logic(aggregator, update_slave_arr); + + if (!port->aggregator->is_active) + port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION; } /* Decide if "agg" is a better choice for the new active aggregator that