Skip routing rules for default mac interface#71
Conversation
nmeyerhans
left a comment
nmeyerhans
left a comment
There was a problem hiding this comment.
Can you also expand a bit on the commit message? Ideally it'll at least contain the same explanation of the change that the PR overview has.
nmeyerhans
left a comment
nmeyerhans
left a comment
There was a problem hiding this comment.
Changes themselves look ready to go.
Before we merge, can you:
- Squash this down to a single commit
- Ensure the commit subject (first line of the commit message) is <50 characters long
- Flesh out the commit body a bit more. Describe the problem and how you fixed it
You can then force-push the squashed commit to your existing branch.
There are a bunch of resources, online for writing commit messages, but many of them are too more prosciptive than we need. This one is fairly lightweight and worth a quick read, though.
Changed lib.sh setup_interface() to fetch the default mac and the device- number for the interface being setup from IMDS. Also changed to skip writing the routing rules table entry for the current interface if it's device-number is 0 (zero) and it's mac matches the default mac for the instance. Also fixed a bug in how $ifid is derived from the interface name by having it also remove uppercase characters from the name. This change was made to resolve a problem where docker containers could not communicate with each other or with the host instance. Network traffic was exiting the container correctly but responses would be routed elsewhere instead of being sent back to the container.
brddan
changed the title
| for family in 4 6; do | ||
| changes+=$(create_rules "$iface" "$ifid" $family) | ||
| if [ "$device_number" -eq 0 ] && [ "$ether" = "$default_mac" ]; then | ||
| debug "Skipping ipv$family rules for default ENI $iface $ether $default_mac $device_number" |
There was a problem hiding this comment.
Thinking about this a little more, I realized that we're still creating the extra routing tables in the create_if_overrides function. I don't think this is a blocker, because the important thing here is that we're skipping rule creation and that the main table has everything we need.
This should be revisited in the future though, as we shouldn't be creating unused resources like this.
2 participants
Issue #, if available: N/A
Description of changes:
Changed lib.sh setup_interface() to fetch the default mac and the device-number for the interface being setup from IMDS. Also changed to skip writing the routing rules table entry for the current interface if it's device-number is 0 (zero) and it's mac matches the default mac for the instance. Also fixed a bug in how $ifid is derived from the interface name by having it also remove uppercase characters from the name.
This change was made to resolve a problem where docker containers could not communicate with each other or with the host instance. Network traffic was exiting the container correctly but responses would be routed elsewhere instead of being sent back to the container.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.