Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for static & lacp link aggragetes #873

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Add support for static & lacp link aggragetes #873

wants to merge 25 commits into from

Conversation

troglobit
Copy link
Contributor

@troglobit troglobit commented Dec 16, 2024

Description

Add support for link aggregation, both static (balance-xor) and LACP modes supported using the Linux bond driver.

See the docs here:

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Test changes
    • Checked in changed Readme.adoc (make test-spec)
    • Added new test to group Readme.adoc and yaml file
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
    • ChangeLog updated (for major changes)
  • Other (please describe):

@troglobit troglobit force-pushed the lag branch 2 times, most recently from bbec2cb to 2a39a38 Compare December 16, 2024 14:20
Copy link
Contributor

@wkz wkz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superb! 🔥

src/confd/yang/infix-if-lag.yang Outdated Show resolved Hide resolved
src/confd/yang/infix-if-lag.yang Outdated Show resolved Hide resolved
src/confd/yang/infix-if-lag.yang Outdated Show resolved Hide resolved
src/confd/yang/infix-if-lag.yang Outdated Show resolved Hide resolved
src/confd/yang/infix-if-lag.yang Outdated Show resolved Hide resolved
src/confd/src/ietf-interfaces.c Outdated Show resolved Hide resolved
src/confd/src/ietf-interfaces.h Outdated Show resolved Hide resolved
src/confd/src/infix-if-lag.c Outdated Show resolved Hide resolved
src/confd/src/infix-if-lag.c Outdated Show resolved Hide resolved
Copy link
Contributor

@jovatn jovatn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I primarily reviewed documentation. Impressive!

doc/networking.md Outdated Show resolved Hide resolved
@troglobit troglobit force-pushed the lag branch 2 times, most recently from 92c4c41 to 96ee219 Compare January 16, 2025 12:13
Latest libite-dev in Ubuntu does not have the latest libite features
needed by confd et al.

Signed-off-by: Joachim Wiberg <[email protected]>
Simplify name, follow new srop tool naming convention.

Signed-off-by: Joachim Wiberg <[email protected]>
The speed/duplex of a virtio interface is not eenforced in any way.  The
driver initializes them to unknown/unkown:

https://github.com/torvalds/linux/blob/dccbe2047a5b0859de24bf463dae9eeea8e01c1e/drivers/net/virtio_net.c#L5375-L5381

It is however possible to set them since there are kernel subsystems
that rely on them, e.g., miimon in the bond driver, which in turn is
required for use with the 802.3ad (lacp) mode.

If we do not initialize speed/duplex the miimon will complain in the log
on virtual test systems every second:

Dec 13 22:34:08 laggy kernel: lag0: (slave e1): failed to get link speed/duplex

Signed-off-by: Joachim Wiberg <[email protected]>
Do not log error if no tarballs exist.

Signed-off-by: Joachim Wiberg <[email protected]>
 - Avoid unnecessary error messages in syslog when build w/o containers
 - Simplify log messages slightly

Signed-off-by: Joachim Wiberg <[email protected]>
Signed-off-by: Joachim Wiberg <[email protected]>
Signed-off-by: Joachim Wiberg <[email protected]>
Some callbacks may run twice, e.g., lag_gen_ports().  Check if the link
already exists, and is the same, then we can exit silently.  However, in
case the link exists and does *not* point to the same target, we log an
error with current target for the post mortem.

Signed-off-by: Joachim Wiberg <[email protected]>
When reading lag status, like LACP actor system prio, port key,
etc., we need CAP_NET_ADMIN.  See net/bonding/bond_netlink.c

Signed-off-by: Joachim Wiberg <[email protected]>
Avoid setting mv88e6xxx ports in disabled state, because that prevents
protocols that do link qualification to pass frames, instead set ports
in blocking.  This allows, e.g., LACPDUs to pass in/out link aggregate
member ports when the lag itself is set as a bridge port.

Signed-off-by: Joachim Wiberg <[email protected]>
Example use:

    @infamy.test_argument("--mode", help="one of 'static' or 'lacp'")
    class TestArgs(infamy.ArgumentParser):
        pass

Signed-off-by: Joachim Wiberg <[email protected]>
When first starting up the YANG models are downloaded.  Initially all
were printed on the test console, so this was later changed to become
a sort of progress output.  However, that broke tap output, causing a
missing leading '#' for the final "YANG models downloaded." message.

Thist patch drops the output entirely from the netconf transport, it
was never added to the restconf transport.

Signed-off-by: Joachim Wiberg <[email protected]>
This commit adds two SVGs for previewing the virtual test topologies.

quad: fix a lot of copy-paste mistakes, rename nodes and ports to their
      correct names.  Replace shell comments with C++ style comments,
      not all tools, e.g., dotty, support shell comments.  The neato
      absolute positioning has also been fixed to make it possible to
      see all the edges, to facilitate this the used ports have been
      swapped around a abit.  Colors are used according to the schema
      agreed on for tests.  Finally, a second link for lag tests have
      been added between dut2 and dut3.

dual: add coloring similar to quad.

Signed-off-by: Joachim Wiberg <[email protected]>
Follow-up to e4535aa, allowing tests to be even simpler.

Signed-off-by: Joachim Wiberg <[email protected]>
Unclear if the libyang merge operation actually does it's job of merging
all the data properly in the restconf backend.

Signed-off-by: Joachim Wiberg <[email protected]>
Verify connectivity from host to the second DUT via the first, over a
link aggregate.  The lag starts in static mode and then changes to an
LACP aggregate.  This verifies not just basic aggregate functionality,
but also changing mode, which is quite tricky to get right.

Signed-off-by: Joachim Wiberg <[email protected]>
Verify connectivity from host to the second DUT via the first, over a
link aggregate.  Both aggregate member links are connected to a TPMR
link breaker to hide link down from the DUTs.

Signed-off-by: Joachim Wiberg <[email protected]>
Signed-off-by: Joachim Wiberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants