-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add bonding and vlan tagging doc examples #1
base: develop
Are you sure you want to change the base?
Conversation
PROVISIONING-METAL.md
Outdated
@@ -105,7 +105,26 @@ Please keep in mind that when using static addresses, DNS information must be su | |||
* `via` (IP address): Gateway IP address. If no gateway is provided, a scope of `link` is assumed. | |||
* `route-metric` (integer): Relative route priority. | |||
|
|||
Example `net.toml` with comments: | |||
Version `3` adds in support for bonding and vlan tagging. The support is limited to mode `1` (`active-backup`) for [bonding](https://www.kernel.org/doc/Documentation/networking/bonding.txt) and `miimon` for monitoring options. Future support may include other bonding options - pull requests are welcome! Version `3` also brings in the concept of network devices vs network interfaces. Physical devices like ethernet adapters are considered interfaces, and network devices encompass other kinds of devices like bonds and vlans. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking about this wording a bit. 🤔
Perhaps rather than Version 3 brings in the concept of...
, we could say something like Version 3 adds the concept of devices vs. network interfaces. Physical devices like ethernet adapters are considered "interfaces", and... It is a *breaking change*, requiring the user to specify <something something device vs. interface>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, It felt awkward saying "adds" all the time but we can stay with the same wording. I do want to add a bit more context here. I think the breaking change thing actually deserves a little more discussion. I wanted to clarify how we warn users that their old config files need more than simply a version bump. Should this be broken out into a "moving to version 3" section all by itself, I don't want this detail buried in a long paragraph.
PROVISIONING-METAL.md
Outdated
|
||
Migrating to version `3` from version `2` is simple, changing a config from `[eno1]` to `[interface.eno1]` is all that is required. | ||
Bonding configuration is created via a network device (`netdev`): | ||
* `bond` (map): Defined by `type = "bond"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does Defined by type="bond"
mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a half-baked idea right now and this line shows it. I'm unsure how we actually want to shape the TOML and so this is clumsy at the moment. As your other comment mentioned, this might actually be [[device.bond0]]
rather than [device.bond0]
and therefore this is the primary key that "unlocks" all the configuration for bonding or vlans or whatever kind
a user defines here. So you are defining the actual device
type with this key. I think this makes sense when you see an example, but I'm struggling for the right wording to have this fit into the rest of the docs.
PROVISIONING-METAL.md
Outdated
@@ -152,6 +171,57 @@ from = "192.168.14.5" | |||
via = "192.168.14.25" | |||
``` | |||
|
|||
Example `net.toml` version `3` with comments: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit conflicted about providing an additional example for version 3. Previously I've opted to update the existing example to push users towards using the latest config format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My primary concern is historical knowledge, if someone is still using version 2 because they don't need these new things yet, or they just haven't had time to move to it, this might be jarring to see only version 3 which has backwards compatibly problems with version 2. I'm fine to shift this into more of a "moving from version 2 to version 3" as long as it provides enough data for someone to get what they need. I would hate for folks to have to dig through historical versions of this file just to reconfigure their existing system.
PROVISIONING-METAL.md
Outdated
dhcp4 = true | ||
|
||
# Multiple routes may be configured | ||
[[interface.eno3.route]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is technically invalid - eno3
needs some additional static configuration before it can accept routes. ^ See the example above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, you are right, I believe I missed a line of copy-paste, the intention was to have an ever-growing example file showcasing as much of the different options as possible but this is making me second guess myself. I wonder if we should instead offer several different examples since it's unlikely that we can craft a valid example that showcases everything we want in one file. Maybe I should just drop this from this particular file and add a separate one that uses version 3 syntax but otherwise leaves the example intact?
PROVISIONING-METAL.md
Outdated
version = 3 | ||
|
||
# A bond is a `device` that is of `type` `bond` | ||
[device.bond0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think since device
will be an array of devices it will need to have double brackets, i.e. [[device.bond0]]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I do believe so, I'll fix.
The net.toml version is bumped to 3 to accomidate the new functionality of bonding and vlans. Version 3 does require different naming than version 2 so version net.toml files are not drop in ready for version 3. This also specifies which options are support for bonding and vlans. Bonding is a complex topic and right now, we are making a deliberate choice to support only a subset of bonding before full support is enabled. This mostly comes in the form of only mode 1 (active-primary). VLAN tagging is supported by creating a virtual network device on top of another device. This allows mixing and matching of vlan tags with whatever configuration is required. A user can chose to only configure VLAN Tagged devices for addressing if that is required, or add in vlan tagging on a device for special handling.
* `miimon_frequency` (int in ms): MII Monitoring frequency in miliseconds | ||
* `miimon_updelay` (int in ms): MII Monitoring delay before the link is enabled after link is detected in milliseconds | ||
* `miimon_downdelay` (int in ms): MII Monitoring delay before the link is disabled after link is no longer detected in milliseconds | ||
* `arpmon_interval` (int in seconds): Number of seconds between intervals to determine link status, must be greater than 0 | ||
* `arpmon_validate` (enum of `all`, `none`, `active`, `backup`): What packets should be used to validate link | ||
* `arpmon_targets` (list of quoted IPv4 address including prefix): List of targets to use for validating ARP. Min = 1, Max = 16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: these should all be kebab-case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
# A bond is a `device` that is of `type` `bond` | ||
[[device.bond0]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# A bond is a `device` that is of `type` `bond` | |
[[device.bond0]] | |
# A bond is a `device` that is of `type` `bond` | |
[device.bond0] |
type = "bond" | ||
# In this case, the vlan will have addressing, the bond is simply there for use in the vlan | ||
dhcp4 = false | ||
dhcp6 = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these the defaults? Can omit if so.
# The first interface in the array is considered `primary` by default | ||
interfaces = ["eno1", "eno2"] | ||
|
||
[bond0.parameters] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this?
[bond0.parameters] | |
[device.bond0.parameters] |
version = 3 | ||
|
||
# A bond is a `device` that is of `type` `bond` | ||
[[device.bond0]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[[device.bond0]] | |
[devices.bond0] |
Using devices
instead of device
might give a more natural deserialization:
struct Blah {
devices: HashMap<String, Device>
}
mode = "active-backup" | ||
min-links = 2 | ||
miimon_frequency = 100 # 100 ms | ||
miimon_updelay = 200 # 200 ms | ||
miimon_downdelay = 200 # 200 ms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum BondMon {
MiiMon(MiiMonConfig),
ArpMon(ArpMonConfg),
}
struct MiiMonConfig {
miimon_frequency = ...,
}
struct ArpMonConfig {
arp_blah = ...,
}
The net.toml version is bumped to 3 to accomidate the new functionality of bonding and vlans. Version 3 does require different naming than version 2 so version net.toml files are not drop in ready for version 3. This also specifies which options are support for bonding and vlans.
Bonding is a complex topic and right now, we are making a deliberate choice to support only a subset of bonding before full support is enabled. This mostly comes in the form of only mode 1 (active-primary) and MII monitoring.
VLAN tagging is supported by creating a virtual network device on top of another device. This allows mixing and matching of vlan tags with whatever configuration is required. A user can chose to only configure VLAN Tagged devices for addressing if that is required, or add in vlan tagging on a device for special handling.
Issue number: bottlerocket-os#2369
Closes #
Description of changes:
This is just an update to the docs defining what would need to go in version 3 of net.toml to get bonding and vlan support. The intention of this PR is to agree upon how the new configuration should look to the user. No actual functionality is in this PR.
Testing done:
This is mostly documenting the lessons learned by hand-configuring wicked and networkd on bottlerocket, SLES12, and Ubuntu 22.04.
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.