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

Fix precedence order of vlan.mode to match documentation #578

Closed
wants to merge 1 commit into from

Conversation

jbemmel
Copy link
Collaborator

@jbemmel jbemmel commented Oct 14, 2022

return get_from_box(intf,'vlan.mode') or \

return get_from_box(intf,'vlan.mode') or \                           # 1 + 2
         get_from_box(node,f'vlans.{vlan}.mode') or \                # 3
         get_from_box(topology,f'vlans.{vlan}.mode') or \            # 5
         get_from_box(node,'vlan.mode') or \                         # 4   <-- too late
         get_from_box(topology,'vlan.mode') or 'irb'                 # 6

According to the documentation:
https://github.com/ipspace/netlab/blob/dev/docs/module/vlan.md#vlan-forwarding-modes

The precedence of various vlan.mode parameters (from highest to lowest) is as follows:

1. Interface vlan.mode, potentially inherited from parent interface vlan.mode or from parent interface vlan.trunk dictionary.
2. Link vlan.mode, potentially inherited from parent link vlan.trunk dictionary
3. mode set in node vlans definition
4. Node vlan.mode setting
5. mode set in global vlans definition
6. Global vlan.mode setting

https://github.com/ipspace/netlab/blob/e3ce92d62e1462b976928ff10e51b60c8edbe8d4/netsim/modules/vlan.py#L118
```
return get_from_box(intf,'vlan.mode') or \                           # 1 + 2
         get_from_box(node,f'vlans.{vlan}.mode') or \              # 3
         get_from_box(topology,f'vlans.{vlan}.mode') or \        # 5
         get_from_box(node,'vlan.mode') or \                           # 4
         get_from_box(topology,'vlan.mode') or 'irb'                # 6
```

According to the documentation:
https://github.com/ipspace/netlab/blob/dev/docs/module/vlan.md#vlan-forwarding-modes
```
The precedence of various vlan.mode parameters (from highest to lowest) is as follows:

1. Interface vlan.mode, potentially inherited from parent interface vlan.mode or from parent interface vlan.trunk dictionary.
2. Link vlan.mode, potentially inherited from parent link vlan.trunk dictionary
3. mode set in node vlans definition
4. Node vlan.mode setting
5. mode set in global vlans definition
6. Global vlan.mode setting
```
@jbemmel
Copy link
Collaborator Author

jbemmel commented Oct 14, 2022

Test cases would need fixing, first wanted to see if you agree this should be changed

@ipspace
Copy link
Owner

ipspace commented Oct 14, 2022

I absolutely agree that the code should match the documentation, and I have no strong preference for current or correct order, although it does make your life easier if you can set node vlan.mode on a router-on-a-stick and have it apply to all VLANs.

What do you think makes more sense?

@jbemmel
Copy link
Collaborator Author

jbemmel commented Oct 14, 2022

I prefer the documented order, hit this issue in a topology (and switched the order before reading the docs)

@ipspace
Copy link
Owner

ipspace commented Oct 14, 2022

Looks like we're in agreement on what to do. Could you please add a commit with fixed tests results and I'll merge it.

@ipspace
Copy link
Owner

ipspace commented Oct 16, 2022

Fixed in #581

@ipspace ipspace closed this Oct 16, 2022
@ipspace ipspace deleted the jbemmel-vlan-mode-precedence-fix branch December 5, 2022 13:06
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.

None yet

2 participants