From: Francis Dupont Date: Thu, 16 Feb 2023 17:08:55 +0000 (+0100) Subject: [#467] Addressed comments X-Git-Tag: Kea-2.3.6~110 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f6a0ea2c0a40e1f614860754a135a544da1c4cbf;p=thirdparty%2Fkea.git [#467] Addressed comments --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 78cef4f97f..1f45b7a6d2 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1859,10 +1859,14 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) { } } - // For each requested option code get the instance of the option + // For each requested option code get the first instance of the option // to be returned to the client. for (auto const& opt : requested_opts) { // Add nothing when it is already there. + // Skip special cases: DHO_VIVSO_SUBOPTIONS. + if (opt == DHO_VIVSO_SUBOPTIONS) { + continue; + } if (!resp->getOption(opt)) { // Iterate on the configured option list for (auto const& copts : co_list) { @@ -1876,8 +1880,11 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) { } } + // Special cases for vendor class and options which are identified + // by the code/type and the vendor/enterprise id vs. the code/type only. if (requested_opts.count(DHO_VIVCO_SUBOPTIONS) > 0) { set vendor_ids; + // Get what already exists in the response. for (auto opt : resp->getOptions(DHO_VIVCO_SUBOPTIONS)) { OptionVendorClassPtr vendor_opts; vendor_opts = boost::dynamic_pointer_cast(opt.second); @@ -1911,6 +1918,7 @@ Dhcpv4Srv::appendRequestedOptions(Dhcpv4Exchange& ex) { if (requested_opts.count(DHO_VIVSO_SUBOPTIONS) > 0) { set vendor_ids; + // Get what already exists in the response. for (auto opt : resp->getOptions(DHO_VIVSO_SUBOPTIONS)) { OptionVendorPtr vendor_opts; vendor_opts = boost::dynamic_pointer_cast(opt.second); diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 74f9004bfe..20189f09cf 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1518,6 +1518,8 @@ Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer, } } + // For each requested option code get the first instance of the option + // to be returned to the client. for (uint16_t opt : requested_opts) { // Add nothing when it is already there. if (!answer->getOption(opt)) { @@ -1534,9 +1536,11 @@ Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer, } } - // Special cases for vendor class and options. + // Special cases for vendor class and options which are identified + // by the code/type and the vendor/enterprise id vs. the code/type only. if (requested_opts.count(D6O_VENDOR_CLASS) > 0) { set vendor_ids; + // Get what already exists in the response. for (auto opt : answer->getOptions(D6O_VENDOR_CLASS)) { OptionVendorClassPtr vendor_class; vendor_class = boost::dynamic_pointer_cast(opt.second); @@ -1572,6 +1576,7 @@ Dhcpv6Srv::appendRequestedOptions(const Pkt6Ptr& question, Pkt6Ptr& answer, if (requested_opts.count(D6O_VENDOR_OPTS) > 0) { set vendor_ids; + // Get what already exists in the response. for (auto opt : answer->getOptions(D6O_VENDOR_OPTS)) { OptionVendorPtr vendor_opts; vendor_opts = boost::dynamic_pointer_cast(opt.second); diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc index 9aa49cbe28..f8ac769ed6 100644 --- a/src/lib/dhcp/libdhcp++.cc +++ b/src/lib/dhcp/libdhcp++.cc @@ -685,14 +685,12 @@ LibDHCP::fuseOptions4(OptionCollection& options) { return (result); } -void -LibDHCP::extendVendorOptions4(OptionCollection& options) { - LibDHCP::extendVivco(options); - LibDHCP::extendVivso(options); -} +namespace { // Anynomous namespace. + +// VIVCO part of extendVendorOptions4. void -LibDHCP::extendVivco(OptionCollection& options) { +extendVivco(OptionCollection& options) { typedef vector TuplesCollection; map vendors_tuples; const auto& range = options.equal_range(DHO_VIVCO_SUBOPTIONS); @@ -718,7 +716,7 @@ LibDHCP::extendVivco(OptionCollection& options) { } catch (const OpaqueDataTupleError&) { // Ignore this kind of error and continue. break; - } catch (const Exception&) { + } catch (const isc::Exception&) { options.erase(DHO_VIVCO_SUBOPTIONS); throw; } @@ -749,8 +747,10 @@ LibDHCP::extendVivco(OptionCollection& options) { } } +// VIVSO part of extendVendorOptions4. + void -LibDHCP::extendVivso(OptionCollection& options) { +extendVivso(OptionCollection& options) { map vendors_data; const auto& range = options.equal_range(DHO_VIVSO_SUBOPTIONS); for (auto it = range.first; it != range.second; ++it) { @@ -773,7 +773,7 @@ LibDHCP::extendVivso(OptionCollection& options) { } catch (const SkipThisOptionError&) { // Ignore this kind of error and continue. break; - } catch (const Exception&) { + } catch (const isc::Exception&) { options.erase(DHO_VIVSO_SUBOPTIONS); throw; } @@ -796,6 +796,14 @@ LibDHCP::extendVivso(OptionCollection& options) { } } +} // end of anonymous namespace. + +void +LibDHCP::extendVendorOptions4(OptionCollection& options) { + extendVivco(options); + extendVivso(options); +} + size_t LibDHCP::unpackVendorOptions6(const uint32_t vendor_id, const OptionBuffer& buf, OptionCollection& options) { diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h index 241b50e2d5..452b2b9830 100644 --- a/src/lib/dhcp/libdhcp++.h +++ b/src/lib/dhcp/libdhcp++.h @@ -300,28 +300,12 @@ public: static bool fuseOptions4(isc::dhcp::OptionCollection& options); /// @brief Extend vendor options from fused options in multiple OptionVendor - /// options and add respective suboptions. + /// or OptionVendorClass options and add respective suboptions or values. /// /// @param options The option container which needs to be updated with /// extended vendor options. static void extendVendorOptions4(isc::dhcp::OptionCollection& options); - /// @brief Extend VIVCO options. - /// - /// VIVCO part of extendVendorOptions4. - /// - /// @param options The option container which needs to be updated with - /// extended vendor options. - static void extendVivco(isc::dhcp::OptionCollection& options); - - /// @brief Extend VIVSO options. - /// - /// VIVSO part of extendVendorOptions4. - /// - /// @param options The option container which needs to be updated with - /// extended vendor options. - static void extendVivso(isc::dhcp::OptionCollection& options); - /// @brief Parses provided buffer as DHCPv4 options and creates /// Option objects. ///