When a bridge-domain has a BVI, it will not sync/add the interfaces to
the bridge. As an example:
bridgedomains:
bd1001:
bvi: loop1001
interfaces: [ tap1001 ]
mtu: 9000
taps:
tap1001:
host:
bridge: br0
mac: 02:fe:83:97:f4:7f
mtu: 9000
name: vpp-bd1001
Before this change, the 'tap1001' would get created, but if the BVI
'loop1001' was correctly set up on the bridge, the code would continue
and skip over enumerating config_bridge_iface. After this change,
both the BVI will be checked (and added if not present), AND all
interfaces will be enumerated (and added if not present).
Add a reasonably tolerant .pylintrc and fix most pylint errors and
warnings.
------------------------------------------------------------------
Your code has been rated at 9.78/10
Used:
$ flynt -a -tc . vppcfg
Execution time: 0.216s
Files checked: 24
Files modified: 13
Character count reduction: 632 (0.36%)
Per expression type:
Old style (`%`) expressions attempted: 209/211 (99.1%)
No `.format(...)` calls attempted.
No concatenations attempted.
F-string expressions created: 205
Ran an integration test before and after. No diffs.
- add __vxlan_tunnel_has_diff() to align with the other _has_diff()
methods
- factor out a few repetitive occurences of l2xc creations
- strictly make "cli=" the pattern instead of "cli = ", to help grepping :)
- based on previously submitted schema and validation, can add a TAP
with host netns, bridge and MTU.
- detect diffs in __tap_has_diff(), used to prune TAPs that must change
- add prune_taps() and create_taps() in the reconciler
- add Dumper() logic to emit YAML config for TAPs
- Move tap_is_lcp() into the VPPApi() class, so it can be reused
Add lots of test cases in intest/*.yaml and example.yaml - full
regression and integration and unit/YAML tests pass on this change.
Special care is taken for bondethernet, where the MAC changes when
the first member is added to it. BondEthernet requires its MAC to
be set in the bondethernets section, disallowing the MAC of individual
members to be set.
Also write a dumper for MACs of all types. Update integration test
cases to stress the MAC changes on loops, bonds, and phys.
This requires a schema change, adding 'mode' and 'load-balance' fields,
a semantic invariant that 'load-balance' can only be set in the case of
LACP and XOR bonds, a mapper from the mode/lb strings, ie.
"round-robin" to their VPP numeric counterparts, a bunch of unit tests.
Then in the reconciler, changing bonds (__bond_has_diff()) will
invalidate any LCP or sub-interfaces built on them, so those will have
to be pruned. create_bondethernet() will now create (or re-create)
the bond with the correct flags.
Unit-tests, YAML tests and the integration test all pass.
Updated config-guide.
Bridges can be created with default settings, with specific settings,
and they can be sync'd at runtime with all of the settings in this
change.
Notably missing are two features:
- unknown unicast flooding into specific interfaces (as opposed to
on/off on the bridge)
- learn-limit, which does not have an API getter, only a setter.
This makes it clearer what its purpose is, in preparation of actual VPP
dataplane configuration changes. For example remove_lcp() refers
currently to the removal of the LCP from the cache, not the VPP
dataplane itself, so rename it and its siblings cache_remove_*()
instead.
In a future commit, the call remove_*() will refer to the removal of an
object _in VPP_.
Instead of writing the CLI calls to stderr as INFO loglines, now add
them to a list of messages by prune/create/sync.
Always close the VPP connection after finishing, by adding a destructor
to the VPPApi class.
At the end of each phase, print out what was gathered to stdout. Later,
I will make this more advanced (output to file, output directly to a
pipe of a running vppctl binary, etc).
The handling of BVI is awkward, with the autoderived interface name
"bviXX" based on the bridgedomain bd_id. Lots of special casing happens
on account of this decision, and to make matters worse there is poor
interaction (leading to VPP crashes) when BVIs and Loopbacks are used
at the same time: https://lists.fd.io/g/vpp-dev/message/21116
In this commit, I reintroduce the ability to set bridgedomain virtual
interfaces by means of the 'bvi' keyword. The 'bvi' must:
- be a Loopback interface
- must be used at most once (bvi_unique())
When pruning, I now need to prune bridgedomains before pruning
loopbacks, because any given loopback might be a BVI for a bridge. So,
I'll remove the loop/BVI from the bridge (by setting it to L3) and only
then removing the loopback from VPP.
In the reconciler, remove BVIs that have changed in prune_bridgedomains()
and set it in sync_bridgedomains().
The handling of BVI is awkward, with the autoderived interface name
"bviXX" based on the bridgedomain bd_id. Lots of special casing happens
on account of this decision, and to make matters worse there is poor
interaction (leading to VPP crashes) when BVIs and Loopbacks are used
at the same time: https://lists.fd.io/g/vpp-dev/message/21116
This is step one of a refactor of the logic. In this commit, I'm
removing all of the BVI logic from the codebase, rendering bridgedomains
unable to have IP interfaces. In the next commit, I will introduce new
behavior in the schema, allowing for 'bvi' to be a loopback
interfacename which will be used as BVI for a bridgedomain, restoring
the ability to use bridgedomains with IP interfaces (using a loop).
What this means is that the synchronization will run based on loops over
the configuration, rather than over the VPP state. The benefit of this
approach is that no additional API calls have to be made after the
initial VPP configuration is read, making pre-flight checks and diffing
more obvious.
Further, make sync_link_mtu() direction-aware; first we will raise the
link_mtu for interfaces that are growing, and after setting all the
children up/down with sync_mtu_direction() we'll finally shrink
sync_link_mtu() where needed.
We can now do an entire reconciliation run with only one API read at the
beginning!
Include special caveat on LCP MAC changes, for which I'll put in a
TODO for now with a VPP comment {} with the to be run command.
Also make the user aware of a quick in BondEthernets not being able
to have link_mtu != 9000 so if a packet MTU > 9000 is set, this will
work but is an undesirable configuration. Issue a warning in this
case.
This is rather straight forward: for each object (in correct order),
if the object exists in VPP, we can skip it. We know that it will exist
only if it was valid (ie correct encapsulation, tc). If it does not
exist in VPP, issue the correct creation request to VPP.
Implement the creation of all types, in the following order:
- create_loopbacks() and create_bvis()
- create_bondethernets()
- create_vxlan_tunnels()
- create_sub_interfaces() first 1-tag, then 2-tag
- create_bridgedomains()
- create_lcps() bottomsup: first 0-tag, then 1-tag, then 2-tag names.
Add a placeholder TODO to fix a bug with prune_sub_interfaces() which
should allow for TAPs belonging to LCPs; will fix in followup commit.
After pruning elements from the VPP configuration, also remove them
from the configuration. The purpose of this is to leave a reasonable
representation of the VPP state in the configuration, so that subsequent
creates and syncs do not have to query the VPP API repeatedly.
The goal of this change is to be able to plan a complete path from
prune, create and sync, with only reading the initial VPP configuration
once, not multiple times.
- Add a get_sub_interfaces() call to fetch all DPDK / Bond sub-ints
- In prune_bvis(), prune_loopbacks() and prune_sub_interfaces(),
use sub_number_of_tags to go out-to-in (qinx, dot1x, untagged)
- Prune bridgedomains before pruning BVIs. The reason for this is that
prune_bridgedomains() will set the BVI to L3 mode, and if the BVI is
removed before the bridge is pruned, this is an error.
- When pruning bridge members, use the VPP configuration as the member
may not exist in the config, upon which the call to interface.is_sub()
will return False even if it is actually a VPP sub-int.
Update README.md, also take into account the previous change which calls
prune_addresses() before object deletion.
