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

STP #867

Merged
merged 10 commits into from
Jan 8, 2025
Merged

STP #867

merged 10 commits into from
Jan 8, 2025

Conversation

wkz
Copy link
Contributor

@wkz wkz commented Dec 10, 2024

Description

Add basic config support for STP/RSTP.

Status reporting will be added in a follow-up PR.

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

Copy link
Contributor

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

Looks great, really nice work on this! Only a minor comment from me.

configs/x86_64_defconfig Outdated Show resolved Hide resolved
@wkz wkz force-pushed the stp branch 4 times, most recently from f45f8ec to d65fee6 Compare December 26, 2024 22:22
@wkz wkz requested review from troglobit and mattiaswal January 8, 2025 08:40
troglobit and others added 4 commits January 8, 2025 09:57
- Helper to get a set of nodes, using a formatted path
- Helper to get a single node, using a formatted path
Copy link
Contributor

@troglobit troglobit left a comment

Choose a reason for hiding this comment

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

✅ Flawless! 🌟 ✨

@troglobit troglobit marked this pull request as ready for review January 8, 2025 09:11
wkz added 6 commits January 8, 2025 10:14
This convention is now well established, so it seems silly to have to
repeat it in every test.
The somewhat clumsy construct of resolving a logical port name to its
physical counterpart has been with us from Infamy's inception:

    _, physical_port = env.ltop.xlate(logical_node, logical_port)

This is needlessly verbose. Since devices already have access to the
mappings which applies to it, make them easy to access:

    target = env.attach("target")

    # Get physical port names via the subscript operator
    target["data0"]
Using `sys.argv[0]` does not quite achieve the effect we want, namely
that any (test-case, $PYTHONHASHSEED) tuple should always default to
the same transport.

For example, the following two examples should always use the same
transport:

    $ make PYTHONHASHSEED=1337 test-sh
    infamy0:test$ ./case/ietf_system/hostname/test.py

    $ make PYTHONHASHSEED=1337 test-sh
    infamy0:test$ cd ./case/ietf_system/hostname
    infamy0:hostname$ ./test.py

But, since their argv[0]'s are different, they don't.

Therefore use the _last two components_ of argv[0]'s full path
instead. This should:

- Spread the allocation over an entire suite run, since the
  penultimate component is usually the test name.

- Keep the allocation stable when test code is modified (which is
  common during debugging of failing tests)

- Avoid instabilities stemming from the local storage location (i.e.,
  home directory names etc.)
Copy link
Contributor

@mattiaswal mattiaswal left a comment

Choose a reason for hiding this comment

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

Excellent work 🌟

@wkz wkz merged commit ac0acfe into main Jan 8, 2025
6 checks passed
@wkz wkz deleted the stp branch January 8, 2025 10:25
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