From owner-svn-src-head@freebsd.org Sun Feb 26 00:19:03 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DF76ACEDB8D; Sun, 26 Feb 2017 00:19:03 +0000 (UTC) (envelope-from jtl@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id A0601356; Sun, 26 Feb 2017 00:19:03 +0000 (UTC) (envelope-from jtl@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v1Q0J2O4012567; Sun, 26 Feb 2017 00:19:02 GMT (envelope-from jtl@FreeBSD.org) Received: (from jtl@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v1Q0J2Pm012566; Sun, 26 Feb 2017 00:19:02 GMT (envelope-from jtl@FreeBSD.org) Message-Id: <201702260019.v1Q0J2Pm012566@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jtl set sender to jtl@FreeBSD.org using -f From: "Jonathan T. Looney" Date: Sun, 26 Feb 2017 00:19:02 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r314286 - head/sys/net X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 26 Feb 2017 00:19:04 -0000 Author: jtl Date: Sun Feb 26 00:19:02 2017 New Revision: 314286 URL: https://svnweb.freebsd.org/changeset/base/314286 Log: Do some minimal work to better conform to the 802.3ad (LACP) standard. In particular, don't set the synchronized bit for the peer unless it truly appears to be synchronized to us. Also, don't set our own synchronized bit unless we have actually seen a remote system. Prior to this change, we were seeing some strange behavior, such as: 1. We send an advertisement with the Activity, Aggregation, and Default flags, followed by an advertisement with the Activity, Aggregation, Synchronization, and Default flags. However, we hadn't seen an advertisement from another peer and were still advertising the default (NULL) peer. A closer examination of the in-kernel data structures (using kgdb) showed that the system had added the default (NULL) peer as a valid aggregator for the segment. 2. We were receiving an advertisement from a peer that included the default (NULL) peer instead of including our system information. However, we responded with an advertisement that included the Synchronization flag for both our system and the peer. (Since the peer's advertisement did not include our system information, we shouldn't add the synchronization bit for the peer.) This patch corrects those two items. Reviewed by: smh MFC after: 2 weeks Sponsored by: Netflix Differential Revision: https://reviews.freebsd.org/D9485 Modified: head/sys/net/ieee8023ad_lacp.c Modified: head/sys/net/ieee8023ad_lacp.c ============================================================================== --- head/sys/net/ieee8023ad_lacp.c Sat Feb 25 23:06:10 2017 (r314285) +++ head/sys/net/ieee8023ad_lacp.c Sun Feb 26 00:19:02 2017 (r314286) @@ -1331,6 +1331,10 @@ lacp_select(struct lacp_port *lp) return; } + /* If we haven't heard from our peer, skip this step. */ + if (lp->lp_state & LACP_STATE_DEFAULTED) + return; + KASSERT(!LACP_TIMER_ISARMED(lp, LACP_TIMER_WAIT_WHILE), ("timer_wait_while still active")); @@ -1686,7 +1690,15 @@ lacp_sm_rx_record_pdu(struct lacp_port * LACP_STATE_AGGREGATION) && !lacp_compare_peerinfo(&lp->lp_actor, &du->ldu_partner)) || (du->ldu_partner.lip_state & LACP_STATE_AGGREGATION) == 0)) { - /* XXX nothing? */ + /* + * XXX Maintain legacy behavior of leaving the + * LACP_STATE_SYNC bit unchanged from the partner's + * advertisement if lsc_strict_mode is false. + * TODO: We should re-examine the concept of the "strict mode" + * to ensure it makes sense to maintain a non-strict mode. + */ + if (lp->lp_lsc->lsc_strict_mode) + lp->lp_partner.lip_state |= LACP_STATE_SYNC; } else { lp->lp_partner.lip_state &= ~LACP_STATE_SYNC; } @@ -1701,10 +1713,6 @@ lacp_sm_rx_record_pdu(struct lacp_port * sizeof(buf)))); } - /* XXX Hack, still need to implement 5.4.9 para 2,3,4 */ - if (lp->lp_lsc->lsc_strict_mode) - lp->lp_partner.lip_state |= LACP_STATE_SYNC; - lacp_sm_ptx_update_timeout(lp, oldpstate); }