Add AddrAddWithoutCalculatedBroadcast #472
Conversation
Added as a separate func to not introduce an API breaking change. Closes vishvananda#471
|
@vishvananda @aboch @fcrisciani PTAL |
|
Thanks for the PR! Do you have other concerns to merge and release this one @vishvananda ? |
|
IMO, I believe it is worth investigating other approaches, trying hard to avoid adding such a specific method. |
|
Without adding new method then it will definitely cause API semantic change. Will it make sense to not automatically add brd address on /31 or /30? |
|
IMO it makes sense, but that would be breaking change. That's why I'm asking there also @vishvananda |
|
@vishvananda ping |
Since [vishvananda#248](vishvananda#248), adding an address automatically sets the broadcast if the broadcast address was not specified. This is undesirable when adding an IP with a prefixlen of /31 or /32. (Additional details in the issues linked below.) This changes the behavior so that the broadcast is only automatically set if the prefixlen is /30 or larger. Issue reported in: - vishvananda#329 - vishvananda#471 See also: - [RFC 3021](http://tools.ietf.org/html/rfc3021) Alternatives to this PR: A. vishvananda#472 - Adds `AddrAddWithoutCalculatedBroadcast`. B. jjastrze-ovh@9a85a61 - Breaking change to make auto-setting the broadcast address an opt-in feature. C. <no existing PR> - Suppress sending of the broadcast to netlink when `addr` has a broadcast set to `0.0.0.0`.
Since [vishvananda#248](vishvananda#248), adding an address automatically sets the broadcast if the broadcast address was not specified. This is undesirable when adding an IP with a prefixlen of /31 or /32. (Additional details in the issues linked below.) This changes the behavior so that the broadcast is only automatically set if the prefixlen is /30 or larger. Issue reported in: - vishvananda#329 - vishvananda#471 See also: - [RFC 3021](http://tools.ietf.org/html/rfc3021) Alternatives to this PR: A. vishvananda#472 - Adds `AddrAddWithoutCalculatedBroadcast`. B. jjastrze-ovh@9a85a61 - Breaking change to make auto-setting the broadcast address an opt-in feature. C. already works - Suppress setting the broadcast when addr's broadcast address is set to `0.0.0.0`. (This works today, but I'm not sure the behavior can be relied upon as a public API.)
|
@aboch @jellonek I just submitted PR #496 with a different approach mirroring @dannyk81's suggestion in #329 to no longer automatically set the broadcast address on subnets too small to logically have one (/32 and /31). As I mention in #496, another alternative – one that already works today – is to set the broadcast address to |
|
thanks @brandt, at the time I was meaning to do a similar PR and... life got the best of me 😅 |
Since [#248](#248), adding an address automatically sets the broadcast if the broadcast address was not specified. This is undesirable when adding an IP with a prefixlen of /31 or /32. (Additional details in the issues linked below.) This changes the behavior so that the broadcast is only automatically set if the prefixlen is /30 or larger. Issue reported in: - #329 - #471 See also: - [RFC 3021](http://tools.ietf.org/html/rfc3021) Alternatives to this PR: A. #472 - Adds `AddrAddWithoutCalculatedBroadcast`. B. jjastrze-ovh@9a85a61 - Breaking change to make auto-setting the broadcast address an opt-in feature. C. already works - Suppress setting the broadcast when addr's broadcast address is set to `0.0.0.0`. (This works today, but I'm not sure the behavior can be relied upon as a public API.)
|
I went ahead and merged the non-breaking change so going to close this for now. Please rebase and reopen if you think that we should also have this method. |
Since [vishvananda#248](vishvananda#248), adding an address automatically sets the broadcast if the broadcast address was not specified. This is undesirable when adding an IP with a prefixlen of /31 or /32. (Additional details in the issues linked below.) This changes the behavior so that the broadcast is only automatically set if the prefixlen is /30 or larger. Issue reported in: - vishvananda#329 - vishvananda#471 See also: - [RFC 3021](http://tools.ietf.org/html/rfc3021) Alternatives to this PR: A. vishvananda#472 - Adds `AddrAddWithoutCalculatedBroadcast`. B. jjastrze-ovh@9a85a61 - Breaking change to make auto-setting the broadcast address an opt-in feature. C. already works - Suppress setting the broadcast when addr's broadcast address is set to `0.0.0.0`. (This works today, but I'm not sure the behavior can be relied upon as a public API.)
6 participants
Added as a separate func to not introduce an API breaking change.
Closes #471