disable broadcast if broadcast is set to net.IPv4zero #1037
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@aboch Do you have time to take a look at this change? What's your opinion? |
aboch
reviewed
Dec 30, 2024
Collaborator
|
Thank you @WeidiDeng. Let's do the right change, let's make the behavior of the addr add/del methods in line with iproute2 behavior. |
Contributor
Author
|
I think it's OK. |
Contributor
Author
|
@aboch I removed the comment when deleting an address since it makes no sense. |
Collaborator
|
Please squash the commits into one |
remove comments about broadcast when deleting address remove another comment about broadcast auto calculation
Contributor
Author
|
@aboch all commits are squashed |
Collaborator
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another go at fixing 329.
According to comment, broadcast address auto-calculation is now a feature, and there is no intention to revert that. However, sometimes users do not want broadcast to be enabled.
Currently this can be done by setting
Broadcasttonet.IPv4zero. But that's not a public api of the netlink. And in my test, it will cause an error to be generated in the kernel log likeattribute type 4 has an invalid length..I think we can use this as a special value to disable broadcast since it already works. But now we explicitly set it to nil when interacting with netlink.
This is an alternative to introducing a new function or extending the
Addrstruct.