-
Notifications
You must be signed in to change notification settings - Fork 798
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
Allow user to optionally disable IPV6 duplicate address detection via runtime configuration for bridge plugin #688
Conversation
8bb2a15
to
8bcbe1d
Compare
plugins/main/bridge/bridge.go
Outdated
@@ -63,10 +63,12 @@ type NetConf struct { | |||
Cni BridgeArgs `json:"cni,omitempty"` | |||
} `json:"args,omitempty"` | |||
RuntimeConfig struct { | |||
Mac string `json:"mac,omitempty"` | |||
Mac string `json:"mac,omitempty"` | |||
DisableIPV6DAD bool `json:"disableipv6dad,omitempty"` |
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.
Lars had proposed 'omitDAD' vs 'DisableIPV6DAD' I actually like the omitDAD.
f9bfa3f
to
b6f7bca
Compare
Signed-off-by: Michael Zappa <Michael.Zappa@stateless.net>
8f7ad3b
to
2cb0436
Compare
Signed-off-by: Michael Zappa <Michael.Zappa@stateless.net>
4d43fdb
to
3f0778e
Compare
Question: do we need DAD at all? Can we disable it by default? Or replace it with optimistic dad? |
So, I had another think about this, and I'm not sure there ever will be a situation when we can't just enable optimistic DAD. See, the only automatically configured interfaces - that is to say, ones that don't come from IPAM - will be autoconfigured link-local ones when we don't care about v6. That means that the address is uninteresting, and optimistic DAD is sufficient, since it will renumber the interface. All that will do is ensure no annoying log messages get to the kernel buffer. So, what if we just set the sysctl |
@squeed I can investigate this today and let you know! @squeed I did test optimistic_dad = 1 on the container side veth and the time for setup was unaffected took between 1500-2100 ms each run. I am still digging into this however that was initial results |
@@ -58,12 +58,14 @@ type NetConf struct { | |||
PromiscMode bool `json:"promiscMode"` | |||
Vlan int `json:"vlan"` | |||
MacSpoofChk bool `json:"macspoofchk,omitempty"` | |||
OmitDad bool `json:"omitdad,omitempty"` |
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.
One thing I am noticing is that accept_dad is an integer that has 0,1,2 values. I am not sure how that escaped me. I will need to resolve that
3846c74
to
3f0778e
Compare
Hello, this PR is for #680 and more documentation exists around the motivation for this PR. An effort is underway to speed up pod creation time and the SettleAddresses method was the culprit and more specifically it was the duplicate address detection feature of IPV6. This PR allows the accept_dad setting to be set to "0" as a runtime configuration which disables that and allows the bridge setup to take less than 100 ms vs 1800-2200ms. Follow-on PR's will be submitted to go-cni,containerd and k8s as well. Look forward to hearing from you!
{ "cniVersion":"0.4.0", "name":"containerd-net", "plugins":[ { "type":"bridge", "bridge":"cni0", "isGateway":true, "ipMasq":true, "promiscMode":true, "runtimeConfig":{ "omitdad":true }, "ipam":{ "type":"host-local", "ranges":[ [ { "subnet":"10.88.0.0/16" } ], [ { "subnet":"2001:4860:4860::/64" } ] ], "routes":[ { "dst":"0.0.0.0/0" }, { "dst":"::/0" } ] } }, { "type":"portmap", "capabilities":{ "portMappings":true } } ] }
{ "cniVersion":"0.4.0", "name":"containerd-net", "plugins":[ { "type":"bridge", "bridge":"cni0", "isGateway":true, "ipMasq":true, "promiscMode":true, "omitdad":true, "ipam":{ "type":"host-local", "ranges":[ [ { "subnet":"10.88.0.0/16" } ], [ { "subnet":"2001:4860:4860::/64" } ] ], "routes":[ { "dst":"0.0.0.0/0" }, { "dst":"::/0" } ] } }, { "type":"portmap", "capabilities":{ "portMappings":true } } ] }
Related PR for Documentation: containernetworking/cni.dev#95 Can submit another PR to update the documentation for the runtime configurations when implemented into containerd/k8s