disable :T2372: disable sub-interface if parent is #383
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
c-po
requested changes
Apr 29, 2020
python/vyos/configdict.py
Outdated
| @@ -98,192 +103,339 @@ def get_ethertype(ethertype_val): | |||
| raise ConfigError('invalid ethertype "{}"'.format(ethertype_val)) | |||
|
|
|||
|
|
|||
| def vlan_to_dict(conf): | |||
| vlan_default = { | |||
| # get the '100' in 'interfaces bonding bond0 vif-s 100' | |||
There was a problem hiding this comment.
What do you mean with this comment?
There was a problem hiding this comment.
leftover from line 109 ..
python/vyos/configdict.py
Outdated
| act_eui = conf.return_values('ipv6 address eui64') | ||
|
|
||
| # Determine interface addresses (currently effective) - to determine which | ||
| # address is no longer valid and needs to be removed from the bond |
There was a problem hiding this comment.
I guess bond here is just a leftover.
There was a problem hiding this comment.
Yes, this comment is also a left-over !
|
I really like the idea and besides those two small comments all looks good to me. Less duplicated code! |
thomas-mangin
commented
Apr 29, 2020
| """ | ||
| Common used function which will extract VLAN related information from config | ||
| and represent the result as Python dictionary. | ||
|
|
||
| Function call's itself recursively if a vif-s/vif-c pair is detected. | ||
| """ | ||
| vlan = { | ||
| 'id': conf.get_level()[-1], # get the '100' in 'interfaces bonding bond0 vif-s 100' |
There was a problem hiding this comment.
@c-po this left over.
|
@thomas-mangin one of us will need to rebase our changes as we were working on the same code. Look at my PR. Does this work fine regarding the link-local address code? |
|
@thomas-mangin can you please fox the leftovers? |
implement disable_state which looks if the current node, or some designated parent node are set are 'disable' and thefore should be ignored. break down the function vlan_to_dict in it multiple components add_to_dict which can parse vif, vif-s, or vif-c and add them to the configuration dictionary intf_to_dict which setup a base configuration dictionary from the interface Config() with addresses, arp, disable, ... it is used by vlan_to_dict but can and will be used by other interfaces
use intf_to_dict and add_to_dict to correctly implement disable.
use intf_to_dict and add_to_dict to correctly implement disable. keeping all interface code with VLAN the same.
use intf_to_dict and add_to_dict to correctly implement disable. keeping all interface code with VLAN the same.
|
I fixed the two comments |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The patchset rewrites the vlan_to_dict code so that if an interface is disabled, all the VLAN of that interface are also disabled (admin down).
The code creates a new helper function to look for the disable option in parent interfaces.
It also refactor much of the dictionary generation for interfaces and VLANs (as they are indicatical, and create a helper function to parse vif, vif-s, vif-c.