Make LACP behavior more closely conform to the standard
ClosedPublic

Authored by jtl on Feb 8 2017, 1:31 PM.

Details

Summary

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 this:

  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 systgem 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
MFC After: 2 weeks

Test Plan

Run LACP on test systems and monitor startup. (So far, it appears that the system now behaves more in keeping with the standard.)

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jtl retitled this revision from to Make LACP behavior more closely conform to the standard.Feb 8 2017, 1:31 PM
jtl updated this object.
jtl edited the test plan for this revision. (Show Details)
jtl added reviewers: network, smh, glebius.
jtl added a subscriber: gallatin.
jtl added a comment.Feb 23 2017, 2:03 PM

*bump*

This has been lingering for a while. Any comments before I commit it?

smh accepted this revision.Feb 23 2017, 5:39 PM

LGTM

sys/net/ieee8023ad_lacp.c
1698 ↗(On Diff #24868)

Not sure its been fixed, but when I added the sysctl we definitely needed to disable strict mode to get LACP to work properly:
https://svnweb.freebsd.org/base?view=revision&revision=290450

Unfortunately I can't for the life of me remember what happened without it :(

This revision is now accepted and ready to land.Feb 23 2017, 5:39 PM
This revision was automatically updated to reflect the committed changes.