Update DHCP broadcast interface handling: #88
Conversation
jacobweinstock
requested review from
chrisdoherty4
and removed request for
chrisdoherty4
Use 127.1.1.1/32 for the DHCP broadcast interface instead of the load balancer IP. Using the load balancer IP can cause instability with that address and routing to Kubernetes services. Make the DHCP broadcast interface name static. The dynamic number added to the name Helm to restarts on every Helm deploy. Add ipvlan support for the DHCP broadcast interface. This allows deployment where creating and broadcasting a new mac address is prohibited. Vmware for example. Signed-off-by: Jacob Weinstock <[email protected]>
jacobweinstock
force-pushed
the
update-dhcp-broadcast-interface
branch
from
a00845f
to
4128489
Compare
fail Helm if an invalid mode is specified. Co-authored-by: Chris Doherty <[email protected]>
Signed-off-by: Jacob Weinstock <[email protected]>
jacobweinstock
force-pushed
the
update-dhcp-broadcast-interface
branch
from
177d0d4
to
0af9007
Compare
In environments where a DHCP relay agent is employed it can be useful to not listen for broadcast traffic at all. Signed-off-by: Jacob Weinstock <[email protected]>
tinkerbell/stack/values.yaml
Outdated
| listenBroadcastTraffic: true | ||
| # interfaceMode determines how we create the interface needed to listen for DHCP broadcast traffic. | ||
| # by default macvlan is used. ipvlan is the only other option. | ||
| # interfaceMode: ipvlan |
There was a problem hiding this comment.
Static defaults should generally reside in the values.yaml so consumers don't need to hunt through template code to find it.
In an actual chart, all static default values should live in the values.yaml, and should not be repeated using the default command (otherwise they would be redundant). However, the default command is perfect for computed values, which cannot be declared inside values.yaml
There was a problem hiding this comment.
Ah, nice. Didn't know this. Thanks for sharing. I'll update.
Helm recommends setting the default value in the values.yaml. Signed-off-by: Jacob Weinstock <[email protected]>
jacobweinstock
force-pushed
the
update-dhcp-broadcast-interface
branch
from
8ead223
to
aa9ae1c
Compare
|
Hey @chrisdoherty4, I think this is ready now. Please and thank you! |
Correct, the "reply" for DHCP local broadcast traffic is not based on source IP and is just another local broadcast. Correct again, relays work in unicast. |
Description
Use 127.1.1.1/32 for the DHCP broadcast interface instead of the load balancer IP. Using the load balancer IP can cause instability with that address and routing to Kubernetes services. And as we only send broadcast packets from this interface the IP is needed but doesn't need to be in the subnet.
Make the DHCP broadcast interface name static. The dynamic number that was added to the name causes restarts on every Helm install/upgrade and is not needed.
Add ipvlan support for the DHCP broadcast interface. This allows deployment where creating and broadcasting a new Mac address can be prohibited or rejected. VMware for example by default sets forged transmits to reject and causes packets for the macvlan interface to be dropped.
Allow enabling/disabling of listening for DHCP broadcast traffic. In environments where a DHCP relay agent is employed, it can be useful to not listen for broadcast traffic at all.
Why is this needed
Fixes: #
How Has This Been Tested?
How are existing users impacted? What migration steps/scripts do we need?
Checklist:
I have: