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.

Sign up for GitHub

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

Jump to bottom

Support STP parameters in bridge #628

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support STP parameters in bridge #628

wants to merge 1 commit into from

Conversation

daniel-noland
Copy link

Added basic support for some basic spanning tree protocol configuration
parameters on the bridge device.

@daniel-noland
Copy link
Author

Notes:

  1. I am very new to Go, so if I am deviating from any idioms or best practices please let me know and I will do my best to correct.
  2. When I run the full go-netlink test suite some of the tests fail due to parameter off-by-one issues. I am running linux kernel 5.11.10 at the time of writing. I don't know why this is happening for me but I will assume this is not happening elsewhere until I dig in a little more and find out what is going on. I mention this here because I deliberately wrote a couple more tests which fail for similar off-by-one issues under the assumption that they won't fail on other machines. I am leaving this as a draft PR till I find out what is going on.

Support STP parameters in bridge
3e8eb11 
Added support for some basic spanning tree protocol configuration
parameters on the bridge device.

* vlan protocol
* stp state
* forward delay
* max age
@daniel-noland daniel-noland marked this pull request as ready for review April 2, 2021 14:47
@daniel-noland
Copy link
Author

daniel-noland commented Apr 2, 2021 •
edited
Loading

Warning:

My colleague ran the tests I included in this patch on his machine (running linux kernel 5.4) and all tests pass.

I am upgrading this PR from draft in the hopes that others will run the tests included in this patch and confirm.

I suggest that we make a point of running this test suite on various linux kernels.

The tests fail for me on kernel 5.11.10

The failure symptom is that all timer values I specify are accepted by the kernel (assuming they were otherwise valid). However, on query every timer is reported as the specified value less one. E.g., if I specify the max age stp parameter as 200 then the operation is is accepted but on querying the current value it returns as 199.

aboch
aboch reviewed May 8, 2021
View reviewed changes
link_linux.go
return h.linkModify(bridge, unix.NLM_F_ACK)
}

func (h *Handle) BridgeSetStpState(link Link, state uint32) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What values can state take?

@aboch
Copy link
Collaborator

aboch commented May 8, 2021

also pls push again so CI is triggered

@aboch
Copy link
Collaborator

aboch commented Sep 18, 2021

ping...

@vishvananda vishvananda changed the base branch from master to main January 12, 2022 22:35
@vishvananda vishvananda reopened this Jan 12, 2022
@vishvananda
Copy link
Owner

Looks like the tests are failing in CI here

@aboch
Copy link
Collaborator

aboch commented Jul 3, 2024

@daniel-noland

1 similar comment
@aboch
Copy link
Collaborator

aboch commented Aug 23, 2024

@daniel-noland

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aboch aboch aboch left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@daniel-noland @aboch @vishvananda