Kea merge requestshttps://gitlab.isc.org/isc-projects/kea/-/merge_requests2024-01-26T10:58:10Zhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2199Resolve "vivso-suboptions not properly supported in Netconf"2024-01-26T10:58:10ZAndrei Pavelandrei@isc.orgResolve "vivso-suboptions not properly supported in Netconf"Closes #3198.
* [x] 60d98ae65d9d80867078c9e796aa92071a6fd3ca add data as key for option-data in YANG modules
Setting mandatory for keys is redundant as mentioned in RFC 7950 section
7.8.2: Any "mandatory" statements in the ...Closes #3198.
* [x] 60d98ae65d9d80867078c9e796aa92071a6fd3ca add data as key for option-data in YANG modules
Setting mandatory for keys is redundant as mentioned in RFC 7950 section
7.8.2: Any "mandatory" statements in the key leafs are ignored.
So they were removed. This now makes it consistent with how data is
declared in option-data as well.
* [x] 6d8816b26b7c5dd1f3db5eae52bee49005cb7b28 make data a key for option-data in code
- Add ability to set list element that only has keys in Translator::setItem.
- Explicitly set list elements in case they contain only keys which can
be more common now that data is a key since it is likely one can have
entries that only have code, space, and data.
- Handle no data as empty data when setting, and empty data as no data
when getting. This avoids the need to add an empty "data" element to
all options that lack it in all-options.json so that the unit tests
pass. But this goes to show that data-less entries may be encountered
in production as well, so more importantly this caters to that
scenario.
- Adjust data in kea4/all-options.json to not contain singlequotes.
There was only one occurrence of it. This is a limitation related
to unit testing only. Opened issue 3216 about it.
- Add missing tests that are not strictly related to the data key, but
they are related to option data:
- TranslatorOptionDataListTestv6.getEmpty
- TranslatorOptionDataListTestv4.get
- TranslatorOptionDataListTestv6.setEmpty
- TranslatorOptionDataListTestv4.set
- Add unit tests:
- TranslatorOptionDataListTestv4.optionsSameCodeAndSpace
- TranslatorOptionDataListTestv6.optionsSameCodeAndSpace
- Add snippet that tests setting of list element with keys only in
TranslatorTest.setItem.
* [x] e47615ea9eb43c9397d4521cccf211fdcdc30c1a bump up revisions for YANG modules
* [x] c5985b28f7c45b17e268c7e646a6ed5d7fb7d4e8 ignore errors when regenerating hashes
The script complains about hashes missing, but that is only temporary until they
are regenerated. A second call to check-hashes.sh will now properly check them
at the end.
* [x] 7c30099d647ed20491f8b8f64375456fb7485381 add ChangeLog entrykea2.5.5Andrei Pavelandrei@isc.orgAndrei Pavelandrei@isc.orghttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2198Resolve "fix asiolink using botan"2023-12-15T20:10:19ZRazvan BecheriuResolve "fix asiolink using botan"Closes #3191Closes #3191kea2.5.5Razvan BecheriuRazvan Becheriuhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2197Resolve "heap-use-after-free and invalid vptr on PingCheckMgr destruction"2024-03-05T08:00:04ZRazvan BecheriuResolve "heap-use-after-free and invalid vptr on PingCheckMgr destruction"Closes #3190
Some explanations about the changes:
1. Some IO objects require the IOService (boost::asio::io_service) to still be valid on destruction (e.g. boost::asio::ip::tcp::socket, boost::asio::ip::udp::socket). Most classes requ...Closes #3190
Some explanations about the changes:
1. Some IO objects require the IOService (boost::asio::io_service) to still be valid on destruction (e.g. boost::asio::ip::tcp::socket, boost::asio::ip::udp::socket). Most classes requiring this can be detected by the getInternalIOService call which gives them access to the reference of the boost::asio::io_service object.
2. The IOService object needs to be kept alive by objects which register some callback functions so that in case of IoServiceThreadPool, the IOService object is not the one run by the main thread but the one created by the instance. This means that when the IoServiceThreadPool instance goes out of scope, the objects using it are referencing a destructed object if they don't acquire the smart pointer to the IOService to keep it still valid.
3. For the callback functions (cancel/close/stop) to be called, the IOService restart and poll (always call in a try-catch block) functions must be called before the objects are destructed.
4. These objects must implement a cancel/close/stop function which needs to be called before the object is destroyed so that they are still valid when the callback is called. This is usually done by the parent 'Object' which has one of such type as a member, just before it can safely destroy the member (can be the destructor).
5. To keep the members alive, they can be added to a lambda function which uses a smart pointer to capture the object, but does not use it. It then must be added to the IOService queue using the post function. This ensured that after the cancel/close/stop callbacks have been called, the lambda is called and only at that stage the object captured is destroyed.
6. IOService restart, run, runOne, poll and pollOne can not be called on any context, they need to be called by the main thread either on the normal code path or on specific events like server reconfiguration or server shutdown.
7. Some IOService objects are created locally, so the same rationale applies here as well, the cancel/close/stop on respective objects using the IOServie must be called and then restart and poll to execute the handlers, just before the IOService is destroyed.
8. In the case of TlsStreamBase(const IOServicePtr& io_service, TlsContextPtr context), the StreamService(io_service, context), base class needed to be created because the IOServicePtr needs to be held valid until the destructor of the TlsStreamBase and TlsStreamImpl are called, so that the destructor and release of the IOService is done last, on the StreamService destructor.
9. Most of the changes are related to the fact that the IntervalTimer uses a smart pointer of IntervalTimerImpl which registers a callback function on the IOService. So even if the IntervalTimer instance is destroyed, the underlying IntervalTimerImpl is not released until the cancel callback has been called. This required that before every IntervalTimer goes out of scope, the cancel and IOService restart and poll are called, which in turn call the callbacks and release the reference to the IntervalTimerImpl.
10. Some objects like IOFetch and DNSClientImpl perpetuate the execution by registering the same instance on the IOService on the callbacks, so a cancel function needed to be created to set a 'stopped\_' flag to true so that when the callback it is executed, the cycle breaks and the function stops registering itself.
Insights:
An Object depending on an IOService must not generate an undefined behavior and must not leak memory.
The Object and IOService should be able to end their scope/visibility in any order, and RAII should properly handle their dependencies and lifetime.
* for an Object to be properly destroyed, it needs to have the IOService still valid
* any cancel/close/stop callback must be called before the Object's destructor, if the callbacks access any Object member (including non static functions)
* by doing so, it means that the IOService must call reset and poll before the Object is destroyed
* the IOService is destroyed when the Object is destroyed, at the earliest, if using the IOServicePtr RAII mechanics
* order of operations is: Object cancel/close/stop followed by IOService poll followed by Object destruction followed by IOService destruction
Conclusions:
* current implementation: the Object does not hold smart pointer to IOService
* wrong operations:
* destroying IOService before destroying the Object causes undefined behavior even if cancel/close/stop are called on the Object's destructor - even if the callbacks are not called
* destroy the Object (which calls cancel/close/stop) and then call restart and poll on the IOService causes use after free if the callbacks access any Object member (including non static functions)
* right operations:
* destroy the Object (which calls cancel/close/stop), and never access the Object members (including non static functions) in the callbacks on cancel/close/stop events
to be able to continue calling poll on the IOService and finally destroy the IOService
* proposed implementations: the Object does hold smart pointer to IOService
* wrong operations:
* not calling cancel/close/stop on the Object before the Object's destruction causes undefined behavior when IOService poll is called after the Object's destruction
* not calling restart and poll on IOService causes memory leak as the Object is destroyed and then IOServicePtr is forever captured by the callback which is still in the IOService event queue (e.g. IntervalTimer is destroyed when IntervalTimerPtr reaches reference count of 0, but the internal member IntervalTimerImplPtr never reaches reference count of 0 because it is also bound to the close callback, and also holds a reference of the IOService, so neither the IOService or the IntervalTimerImpl gets released)
* right operations:
* call cancel/stop/close on the Object, then call restart and poll on IOService, then destroy the Object, which in turn destroys IOService if it has the last reference
e.g:
```plaintext
// Stop the thread pool.
thread_pool_->stop();
// Stop the listener.
http_listener_->stop();
thread_io_service_->restart();
try {
thread_io_service_->poll();
} catch (...) {
}
// Get rid of the thread pool.
thread_pool_.reset();
// Get rid of the listener.
http_listener_.reset();
// Ditch the IOService.
thread_io_service_.reset();
```
Alternatives to IOService post:
Using boost::enable_shared_from_this might keep the members alive on the IOService queue until the callbacks are called, so the need for IOService post with the lambda capturing the smart pointer of the member can be avoided.
Possible problems with proposed implementations:
* There are some hook libraries which use objects with this problem, so registering the objects on the IOService post, the hook library must rely on the core functionality to call reset and poll, just before dlclose is used to unload the library symbols. This can be easily done on each server shutdown and on each reconfiguration even.
* A real problem is that a new requirement needs to be satisfied: all hook libraries must destroy all global objects which might have a reference to such objects on the unload hook point. If they don't release the objects by that stage, the unloading of the library might not work, or worse, the reference is destroyed on dlclose and all hanging references used by next call to IOService run/runOne/poll/pollOne will lead to a crash, or at least an undefined behavior. One example of a global instance with references to such objects is PingCheckMgr, but it is properly destroyed on unload hook point.kea2.5.7Razvan BecheriuRazvan Becheriuhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2196Resolve "Kea might not use getopt correctly on alpine"2024-01-26T18:01:51ZAndrei Pavelandrei@isc.orgResolve "Kea might not use getopt correctly on alpine"Closes #2788.
* [x] 11ad14d306cdc502189945b9c73d39a35ae9676a make all commandLineArgs tests more strict
* [x] 3571991e8067ae8c080300dcb936e741459a1064 exhaust options before throwing error
Prior to this change, if parseArgs() ...Closes #2788.
* [x] 11ad14d306cdc502189945b9c73d39a35ae9676a make all commandLineArgs tests more strict
* [x] 3571991e8067ae8c080300dcb936e741459a1064 exhaust options before throwing error
Prior to this change, if parseArgs() was called twice during the same
program lifetime and it stumbled on an unsupported option and throwed an
exception on the first call, the previous set of arguments lived on to
be parsed by the second call. This is a situation that likely arises
only in unit tests, but let us fix it properly to at least silence the unit
test failure on alpine, which was happening because of different
implementation of getopt from musl, and which motivated looking into how
getopt behaves. To make the bug evident even in a non-alpine environment, add an
EXPECT_THROW_MSG in DStubControllerTest.commandLineArgs when parsing argv3, and
see that it outputs "unsupported option: [s]" instead of
"extraneous command line information".
* [x] 22eb99788a5280a4e120eff199bafb6d076da42e reset optarg
optarg is not reset in musl's getopt and it leaks values to other flags.
Reset it for all systems because it cannot hurt. If you remove the
optarg reset, you should see the bug in action on alpine systems in
DstubControllerTest.commandLineArgs when parsing argv2:
```
[ RUN ] DStubControllerTest.commandLineArgs
d_controller_unittests.cc:102: Failure
Expected equality of these values:
std::string(ex.what())
Which is: "unsupported option: -b cfgName"
"unsupported option: -b"
[ FAILED ] DStubControllerTest.commandLineArgs (14 ms)
```kea2.5.5Andrei Pavelandrei@isc.orgAndrei Pavelandrei@isc.orghttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2194Resolve "Bulk Leasequery (BLQ) needs to be able to match PDs associated with ...2024-01-18T20:32:45ZFrancis DupontResolve "Bulk Leasequery (BLQ) needs to be able to match PDs associated with a link, even if the subnet of the PD is outside of the subnet of the link"Part of #3149 - reimplementPart of #3149 - reimplementkea2.5.5Francis DupontFrancis Duponthttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2193Resolve "Bulk Leasequery (BLQ) needs to be able to match PDs associated with ...2024-01-17T09:09:42ZFrancis DupontResolve "Bulk Leasequery (BLQ) needs to be able to match PDs associated with a link, even if the subnet of the PD is outside of the subnet of the link"Part of #3149 (requires tests to evaluate impact)Part of #3149 (requires tests to evaluate impact)kea2.5.5Francis DupontFrancis Duponthttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2192Resolve "Get rid of libdhcp_old_radius.so"2023-12-14T13:54:23ZFrancis DupontResolve "Get rid of libdhcp_old_radius.so"Closes #3168Closes #3168kea2.5.5Francis DupontFrancis Duponthttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2191Resolve "extend hammer to build aarch64 packages"2024-01-03T10:49:14ZWlodzimierz WencelResolve "extend hammer to build aarch64 packages"Closes #3186Closes #3186kea2.5.5Wlodzimierz WencelWlodzimierz Wencelhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2190[#3106] Fix network state for ha-sync-complete-notify2023-12-06T15:32:08ZMarcin Siodelski[#3106] Fix network state for ha-sync-complete-notifyFixes an issue whereby the network stuck in disabled state after receiving the `ha-sync-complete-notify` command in the `partner-down` state.
Closes #3106Fixes an issue whereby the network stuck in disabled state after receiving the `ha-sync-complete-notify` command in the `partner-down` state.
Closes #3106Marcin SiodelskiMarcin Siodelskihttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2188Resolve "Coverity scan issues"2024-01-22T15:58:15ZPiotrek ZadrogaResolve "Coverity scan issues"#huge-sorry
Closes #3119#huge-sorry
Closes #3119kea2.5.5Razvan BecheriuRazvan Becheriuhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2187Draft: Resolve "Avoid copy in range-based for loops"2023-12-04T09:23:35ZFrancis DupontDraft: Resolve "Avoid copy in range-based for loops"Closes #3182Closes #3182https://gitlab.isc.org/isc-projects/kea/-/merge_requests/2186Resolve "update hammer to build kea packages without freeradius dependency"2023-12-05T09:23:27ZWlodzimierz WencelResolve "update hammer to build kea packages without freeradius dependency"Closes #3128Closes #3128kea2.5.5Wlodzimierz WencelWlodzimierz Wencelhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2185Resolve "Run multiple HA relationships in hub-and-spoke configuration"2024-01-08T07:30:14ZMarcin SiodelskiResolve "Run multiple HA relationships in hub-and-spoke configuration"Closes #3178Closes #3178kea2.5.5Marcin SiodelskiMarcin Siodelskihttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2184Draft: Resolve "Get rid of libdhcp_old_radius.so"2023-12-12T08:51:25ZFrancis DupontDraft: Resolve "Get rid of libdhcp_old_radius.so"Part of #3168Part of #3168kea2.5.5Francis DupontFrancis Duponthttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2183Resolve "core ping-check-hook implement ST mode in PingCheckMgr"2023-12-13T15:44:38ZThomas MarkwalderResolve "core ping-check-hook implement ST mode in PingCheckMgr"Closes #3107
Update ARM to reflect single-threaded mode now supported.Closes #3107
Update ARM to reflect single-threaded mode now supported.kea2.5.5Thomas MarkwalderThomas Markwalderhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2182Resolve "bump up version in configure.ac to 2.5.5-git"2023-11-29T16:30:32ZAndrei Pavelandrei@isc.orgResolve "bump up version in configure.ac to 2.5.5-git"Closes #3177.Closes #3177.kea2.5.5Andrei Pavelandrei@isc.orgAndrei Pavelandrei@isc.orghttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2181Resolve "ping-check-hook update HA peer(s) with lease declinations"2023-12-01T16:56:45ZThomas MarkwalderResolve "ping-check-hook update HA peer(s) with lease declinations"Closes #3110Closes #3110kea2.5.5Thomas MarkwalderThomas Markwalderhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2180Changes for Kea 2.5.4 release2023-11-28T10:55:02ZAndrei Pavelandrei@isc.orgChanges for Kea 2.5.4 releaseCloses #3174Closes #3174kea2.5.4Andrei Pavelandrei@isc.orgAndrei Pavelandrei@isc.orghttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2179Resolve "Bump up library versions for 2.5.4"2023-11-28T09:10:54ZRazvan BecheriuResolve "Bump up library versions for 2.5.4"Closes #3173Closes #3173kea2.5.4Razvan BecheriuRazvan Becheriuhttps://gitlab.isc.org/isc-projects/kea/-/merge_requests/2178Resolve "Process: Fix issues reported by dependabot"2024-03-22T13:53:42ZTomek MrugalskiResolve "Process: Fix issues reported by dependabot"Closes #3079Closes #3079kea2.5.7Tomek MrugalskiTomek Mrugalski