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

T2241: fix interfaces falling out of bridge #384

Merged
merged 63 commits into from May 5, 2020

Conversation

jjakob
Copy link
Contributor

@jjakob jjakob commented May 1, 2020

Untested. If there are too many commits I can join them.

Edit: I think me and @thomas-mangin were working on some of the same code. I need to look at his PR changes and see if there is any duplication.

@c-po
Copy link
Member

c-po commented May 1, 2020

I like small commits :)

@jjakob
Copy link
Contributor Author

jjakob commented May 1, 2020

Ok. Yeah, looking at #383 one of us will definitely need to rebase as the vlan code has substantial changes in both.

@c-po
Copy link
Member

c-po commented May 2, 2020

This still a WIP?

@jjakob
Copy link
Contributor Author

jjakob commented May 2, 2020

Yes, still testing

@jjakob
Copy link
Contributor Author

jjakob commented May 2, 2020

I'll rebase after #388 is merged

@jjakob
Copy link
Contributor Author

jjakob commented May 2, 2020 via email

@jjakob jjakob force-pushed the bridge-fix-T2241 branch 3 times, most recently from 4e85db0 to dbb62e3 Compare May 4, 2020 18:57
jjakob added 9 commits May 4, 2020 20:58
Remove one unnecessary call to conf.get_level()
This is needed as later functions depend on it
- rewrite the function to support both bridge and bonding interface types,
  if the type is passed it searches only that type, otherwise it searches
  both
- move is_member check out of the deleted condition
- move is_member check to intf_from_dict for interfaces that use it
Function that parses the config of a bridge member into a dict that is
needed to apply all port config when adding a port to a bridge.
Needed because other interfaces will be adding themselves to the bridge
outside of the bridge conf_mode script and they need a common place to
get their config.
Can't be put as method of BridgeIf as we can't invoke it without it
creating the bridge (create=False raises an exception), we need to
get the configuration before we create the interface.
Was previously moved out of this script.
jjakob added 10 commits May 4, 2020 22:25
Will be called by all interface scripts to re-add themselves to a bridge
after deleting and recreating themselves.
Previously, set_vrf was always called, which uses the same master and nomaster
commands as bridge, so it removed the interface from the bridge.
- add checks to make VRF and bridge membership mutually exclusive
- always re-add the interface back to any bridge it is part of in
  case it is deleted and recreated (e.g. changing egress/ingress-qos)
Bridge members should not have addresses assigned.
Bridge members should not have addresses assigned.
Bridge members should not have addresses assigned.
- use is_member function instead of checking config directly
- make error output more user friendly
- replace .format with f-strings
- split into lines less than ~80 characters long
Previously, set_vrf was always called, which uses the same master and nomaster
commands as bridge, so it removed the interface from the bridge.
- add checks to make VRF and bridge membership mutually exclusive
- always re-add the interface back to any bridge it is part of in
  case it is deleted and recreated
jjakob added 26 commits May 4, 2020 22:59
- make error output more user friendly
- replace .format with f-strings
- split into lines less than ~80 characters long
…xclusive

Bridge members should not have any addresses assigned.
Previously, the interface was always deleted and recreated, which removed it
from the bridge.
- add checks to make VRF and bridge membership mutually exclusive
- always re-add the interface back to any bridge it is part of
- make error output more user friendly
- replace .format with f-strings
- split into lines less than ~80 characters long
Bridge members should not have any addresses assigned.
Previously, set_vrf was always called, which uses the same master and nomaster
commands as bridge, so it removed the interface from the bridge.
- add checks to make VRF and bridge membership mutually exclusive
- make error output more user friendly
- replace .format with f-strings
- split into lines less than ~80 characters long
Bridge members should not have any addresses assigned.
Previously, the interface was always deleted and recreated, which
removed it from the bridge.
- always re-add the interface back to any bridge it is part of
- make error output more user friendly
- replace .format with f-strings
- split into lines less than ~80 characters long
Bridge members should not have any addresses assigned.
Previously, set_vrf was always called, which uses the same master and nomaster
commands as bridge, so it removed the interface from the bridge.
- add checks to make VRF and bridge membership mutually exclusive
- make error output more user friendly
- replace .format with f-strings
- split into lines less than ~80 characters long
Bridge members should not have any addresses assigned.
Previously, set_vrf was always called, which uses the same master and nomaster
commands as bridge, so it removed the interface from the bridge.
- add checks to make VRF and bridge membership mutually exclusive
- make error output more user friendly
- replace .format with f-strings
- split into lines less than ~80 characters long
…lusive

Bridge members should not have any addresses assigned.
Previously, the interface was always deleted and recreated, which
removed it from the bridge.
- always re-add the interface back to any bridge it is part of
- make error output more user friendly
- replace .format with f-strings
- split into lines less than ~80 characters long
We've already verified that all member interfaces don't have any
addresses configured, so it should be safe to simply call 'ip addr flush' on
them to flush the remaining addresses (e.g. IPv6 link-local)
We've already verified that all member interfaces don't have any
addresses configured, so it should be safe to simply call 'ip addr flush' on
them to flush the remaining addresses (e.g. IPv6 link-local)
@jjakob jjakob changed the title WIP: T2241: fix interfaces falling out of bridge T2241: fix interfaces falling out of bridge May 4, 2020
@jjakob
Copy link
Contributor Author

jjakob commented May 4, 2020

Ready to merge - tested basic functionality for ethernet, bridge, bond, vif. Other interface types need testing.

There is a weird bug with the ipv6 link-local code but it is present without this PR too - it started after PR #383 - I'll investigate and create a task for it after this is merged

@c-po c-po merged commit 9afb06a into vyos:current May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants