svn commit: r332834 - stable/11/sys/net

[Jonathan T. Looney]

Author: jtl
Date: Fri Apr 20 18:37:19 2018
New Revision: 332834
URL: https://svnweb.freebsd.org/changeset/base/332834

Log:
  MFC r314286:
    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.
  
  Sponsored by:	Netflix, Inc.

Modified:
  stable/11/sys/net/ieee8023ad_lacp.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/net/ieee8023ad_lacp.c
==============================================================================
--- stable/11/sys/net/ieee8023ad_lacp.c	Fri Apr 20 18:20:55 2018	(r332833)
+++ stable/11/sys/net/ieee8023ad_lacp.c	Fri Apr 20 18:37:19 2018	(r332834)
@@ -1308,6 +1308,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"));
 
@@ -1663,7 +1667,15 @@ lacp_sm_rx_record_pdu(struct lacp_port *lp, const stru
 	    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;
 	}
@@ -1677,10 +1689,6 @@ lacp_sm_rx_record_pdu(struct lacp_port *lp, const stru
 		    lacp_format_state(lp->lp_partner.lip_state, buf,
 		    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);
 }