-
Notifications
You must be signed in to change notification settings - Fork 1.8k
OSDOCS-14356-New: Added bond best practices info to networking docs #93160
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
base: main
Are you sure you want to change the base?
Conversation
|
🤖 Tue Dec 02 17:17:59 - Prow CI generated the docs preview: |
3296fa2 to
318fd00
Compare
|
/retest |
318fd00 to
1b5443b
Compare
1b5443b to
5b7d324
Compare
02430e1 to
58716d3
Compare
10a409f to
f780fb6
Compare
|
The This is because your PR targets the If the update in your PR does NOT apply to version 4.21 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main. |
cybertron
left a comment
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.
Okay, after some more investigation into OVS bond configs I have a couple more comments.
| Consider the following architectural layout for OVS bridges that interact with OVS interfaces: | ||
|
|
||
| * A network interface uses a bridge MAC address for managing protocol-level traffic and other administrative tasks, such as IP address assignment. | ||
| * The physical MAC addresses of physical interfaces do not handle traffic. |
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.
Actually for OVS bonding I think my comment may be invalid. We don't clone the MAC address in those configs.
f57d4fc to
4bab646
Compare
| Consider that an OVS bond with `balance-slb` mode enabled might experience issues if the bond forwards unknown unicast traffic from one physical network interface controller (NIC) into the phsycial network through another NIC. This situation can result in an Layer 2 loop, or _bridge loop_, that in turn causes _MAC flapping_. | ||
| MAC flapping causes the same MAC address to exist in many network locations for a period of time. For example, a remote switch does not learn the MAC address for the destination of a unicast packet and this causes the packet to exist on all links available on the SLB bond configuration. As a workaround for this issue, you can set the bond to `active-backup` mode during MAC address assignment and then switch the bond to use `balance-slb` mode. | ||
| ==== |
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.
We can delete this.
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.
Thanks. Deleted:
[NOTE]
====
Consider that an OVS bond withbalance-slbmode enabled might experience issues if the bond forwards unknown unicast traffic from one physical network interface controller (NIC) into the phsycial network through another NIC. This situation can result in an Layer 2 loop, or bridge loop, that in turn causes MAC flapping.
MAC flapping causes the same MAC address to exist in many network locations for a period of time. For example, a remote switch does not learn the MAC address for the destination of a unicast packet and this causes the packet to exist on all links available on the SLB bond configuration. As a workaround for this issue, you can set the bond to
active-backupmode during MAC address assignment and then switch the bond to usebalance-slbmode.
====
| [source,terminal] | ||
| ---- | ||
| bond=bond0:em1,em2:mode=active-backup | ||
| ip=bond0:dhcp |
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 doesn't work. bond= with DHCP does not support MAC cloning, so primary slave fail and reboot will get new DHCP lease with secondary slave MAC.
We need to remove bond= kernel args.
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.
Maybe there is an extra kwarg to set secondary slave MAC.
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.
Removed:
bond=bond0:em1,em2:mode=active-backup
| [source,terminal] | ||
| ---- | ||
| bond=bond0:em1,em2:mode=active-backup | ||
| ip=10.10.10.2::10.10.10.254:255.255.255.0:core0.example.com:bond0:none |
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.
bond= with static IP kinda works, but it also doesn't support cloned-macs, but it doesn't necessarily matter for static IP.
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.
Removed:
bond=bond0:em1,em2:mode=active-backup
4bab646 to
d70dd73
Compare
759b474 to
bad17ba
Compare
cybertron
left a comment
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.
/lgtm
We'll probably end up tweaking and adding content here, but this is a good start.
| [NOTE] | ||
| ==== | ||
| As a postinstallation task, you cannot make configuration changes to the `br-ex` bridge or its underlying interfaces. As a workaround, use a secondary network interface connected to your host or switch. |
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.
Not a blocker for this patch, but if we're going to include these notes everywhere an NNCP is mentioned, we might want to explicitly state that it is in relation to Kubernetes-NMState. It is possible to make some changes to the underlying interfaces via other methods.
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.
✔️
| Do not use the Kubernetes NMState Operator or a `NodeNetworkConfigurationPolicy` (NNCP) manifest file to define the additional interface. Ensure that the additional interface or sub-interfaces when defining a `bond` interface are not used by an existing `br-ex` OVN Kubernetes network deployment. | ||
|
|
||
| Also ensure that the additional interface or sub-interfaces when defining a `bond` interface are not used by an existing `br-ex` OVN Kubernetes network deployment. | ||
| As a postinstallation task, you cannot make configuration changes to the `br-ex` bridge or its underlying interfaces. As a workaround, use a secondary network interface connected to your host or switch. |
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.
Again not specific to this patch, but in this context we should probably include br-ex1 as a do-not-touch interface. I don't think we need to mention it everywhere, but since this doc is discussion the secondary br-ex1 bridge it would be worth including here.
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.
✔️
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.
Added br-ex1 to the list two "The following list of interface names are reserved and you cannot use the names with NMstate configurations:" notes.
bad17ba to
61c5737
Compare
|
New changes are detected. LGTM label has been removed. |
08e4444 to
9570f0b
Compare
9570f0b to
2cfe455
Compare
|
@dfitzmau: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Checklist of items:
Version(s):
4.12+
Issue:
OSDOCS-14356
Link to docs preview: