Commit 91f8ca94 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[#13,!6] Addressed review comments

    Mostly corrections additions to commentary.
parent 9253b4fa
...@@ -3275,7 +3275,7 @@ should include options from the isc option space: ...@@ -3275,7 +3275,7 @@ should include options from the isc option space:
specific subnet. Kea will still match inbound client packets to a specific subnet. Kea will still match inbound client packets to a
subnet as before, but when the subnet's reservation mode is set to subnet as before, but when the subnet's reservation mode is set to
<command>"global"</command>, Kea will look for host reservations only <command>"global"</command>, Kea will look for host reservations only
among the global reservations defined. Typcially, such resrvations would among the global reservations defined. Typcially, such reservations would
be used to reserve hostnames for clients which may move from one subnet be used to reserve hostnames for clients which may move from one subnet
to another. to another.
</para> </para>
...@@ -3709,17 +3709,17 @@ If not specified, the default value is: ...@@ -3709,17 +3709,17 @@ If not specified, the default value is:
<para>This feature can be used to assign certain parameters, such as <para>This feature can be used to assign certain parameters, such as
hostname or other dedicated, host-specific options. It can also be used to hostname or other dedicated, host-specific options. It can also be used to
assign addresses or prefixes. However, global reservations that assign assign addresses or prefixes. However, global reservations that assign
either of these bypass the whole topology determination provided by DHCP either of these bypass the whole topology determination provided by DHCP
logic implemented in Kea. It is very easy to misuse this feature and get logic implemented in Kea. It is very easy to misuse this feature and get
configuration that is inconsistent. To give a specific example, imagine a configuration that is inconsistent. To give a specific example, imagine a
global reservation for an address 2001:db8:1111::1 and two subnets global reservation for an address 2001:db8:1111::1 and two subnets
2001:db8:1111::/64 and 2001:db8:ffff::/48. If global reservations are used 2001:db8:1111::/48 and 2001:db8:ffff::/48. If global reservations are used
in both subnets and a device matching global host reservations visits part in both subnets and a device matching global host reservations visits part
of the network that is covered by 2001:db8:ffff::/48, it will get an IP of the network that is covered by 2001:db8:ffff::/48, it will get an IP
address 2001:db8:ffff::/48, which will be outside of the prefix announced address 2001:db8:ffff::1, which will be outside of the prefix announced
by its local router using Router Advertisements. Such a configuration by its local router using Router Advertisements. Such a configuration
would be unsuable or at the very least ridden with issues, such as the would be unsuable or at the very least ridden with issues, such as the
downlink traffic not reaching the device.</para> downlink traffic not reaching the device.</para>
<para> <para>
...@@ -3741,7 +3741,7 @@ If not specified, the default value is: ...@@ -3741,7 +3741,7 @@ If not specified, the default value is:
"hostname": "hw-host-fixed", "hostname": "hw-host-fixed",
// Use of IP address is global reservation is risky. If used outside of // Use of IP address is global reservation is risky. If used outside of
// matching subnet, such as 2001:db8:1::/64, it will result in a broken // matching subnet, such as 3001::/64, it will result in a broken
// configuration being handled to the client. // configuration being handled to the client.
"ip-address": "2001:db8:ff::77" "ip-address": "2001:db8:ff::77"
}, },
......
...@@ -1180,8 +1180,8 @@ HostTest::sarrTest(Dhcp6Client& client, const std::string& exp_address, ...@@ -1180,8 +1180,8 @@ HostTest::sarrTest(Dhcp6Client& client, const std::string& exp_address,
Lease6 lease_client = client.getLease(0); Lease6 lease_client = client.getLease(0);
EXPECT_EQ(exp_address, lease_client.addr_.toText()); EXPECT_EQ(exp_address, lease_client.addr_.toText());
// Check that the server recorded the lease. // Check that the server recorded the lease
// and that the server lease has NO hostname // and that the server lease has expected hostname.
Lease6Ptr lease_server = checkLease(lease_client); Lease6Ptr lease_server = checkLease(lease_client);
ASSERT_TRUE(lease_server); ASSERT_TRUE(lease_server);
EXPECT_EQ(exp_hostname, lease_server->hostname_); EXPECT_EQ(exp_hostname, lease_server->hostname_);
......
...@@ -1252,7 +1252,7 @@ AllocEngine::allocateGlobalReservedLeases6(ClientContext6& ctx, ...@@ -1252,7 +1252,7 @@ AllocEngine::allocateGlobalReservedLeases6(ClientContext6& ctx,
return (false); return (false);
} }
// We want to avoid allocating new lease for an IA if there is already // We want to avoid allocating a new lease for an IA if there is already
// a valid lease for which client has reservation. So, we first check if // a valid lease for which client has reservation. So, we first check if
// we already have a lease for a reserved address or prefix. // we already have a lease for a reserved address or prefix.
BOOST_FOREACH(const Lease6Ptr& lease, existing_leases) { BOOST_FOREACH(const Lease6Ptr& lease, existing_leases) {
...@@ -1266,6 +1266,7 @@ AllocEngine::allocateGlobalReservedLeases6(ClientContext6& ctx, ...@@ -1266,6 +1266,7 @@ AllocEngine::allocateGlobalReservedLeases6(ClientContext6& ctx,
.arg(lease->typeToText(lease->type_)) .arg(lease->typeToText(lease->type_))
.arg(lease->addr_.toText()); .arg(lease->addr_.toText());
// Besides IP reservations we're also going to return other reserved
// parameters, such as hostname. We want to hand out the hostname value // parameters, such as hostname. We want to hand out the hostname value
// from the same reservation entry as IP addresses. Thus, let's see if // from the same reservation entry as IP addresses. Thus, let's see if
// there is any hostname reservation. // there is any hostname reservation.
......
...@@ -488,7 +488,7 @@ public: ...@@ -488,7 +488,7 @@ public:
/// ///
/// If the current subnet's reservation mode is global and /// If the current subnet's reservation mode is global and
/// there is a global host (i.e. reservation belonging to /// there is a global host (i.e. reservation belonging to
/// the global subnet), return it. Otherwise return an /// the global subnet), return it. Otherwise return an
/// empty pointer. /// empty pointer.
/// ///
/// @return Pointer to the host object. /// @return Pointer to the host object.
...@@ -783,7 +783,7 @@ public: ...@@ -783,7 +783,7 @@ public:
if (lease.type_ == Lease::TYPE_NA) { if (lease.type_ == Lease::TYPE_NA) {
return (IPv6Resrv(IPv6Resrv::TYPE_NA, lease.addr_, return (IPv6Resrv(IPv6Resrv::TYPE_NA, lease.addr_,
(lease.prefixlen_ ? lease.prefixlen_ : 128))); (lease.prefixlen_ ? lease.prefixlen_ : 128)));
} }
return (IPv6Resrv(IPv6Resrv::TYPE_PD, lease.addr_, lease.prefixlen_)); return (IPv6Resrv(IPv6Resrv::TYPE_PD, lease.addr_, lease.prefixlen_));
} }
...@@ -842,16 +842,33 @@ private: ...@@ -842,16 +842,33 @@ private:
/// @brief Creates new leases based on reservations. /// @brief Creates new leases based on reservations.
/// ///
/// This method allocates new leases, based on host reservation. Existing /// This method allcoates new leases, based on host reservations.
/// leases are specified in existing_leases parameter. A new lease is not created, /// Existing leases are specified in the existing_leases parameter.
/// if there is a lease for specified address on existing_leases list or there is /// It first calls @c allocateGlobalReservedLeases6 to accomodate
/// a lease used by someone else. /// subnets using global reservations. If that method allocates
/// addresses, we return, otherwise we continue and check for non-global
/// reservations. A new lease is not created, if there is a lease for
/// specified address on existing_leases list or there is a lease used by
/// someone else.
/// ///
/// @param ctx client context that contains all details (subnet, client-id, etc.) /// @param ctx client context that contains all details (subnet, client-id, etc.)
/// @param existing_leases leases that are already associated with the client /// @param existing_leases leases that are already associated with the client
void void
allocateReservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases); allocateReservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases);
/// @brief Creates new leases based on global reservations.
///
/// This method is used by @allocateReservedLeases6, to allocate new leases based
/// on global reservation if one exists and global reservations are enabled for
/// the selected subnet. It differs from it's caller by looking only at the global
/// reservation and therefore has no need to iterate over the selected subnet or it's
/// siblings looking for host reservations. Like it's caller, existing leases are
/// specified in existing_leases parameter. A new lease is not created, if there is
/// a lease for specified address on existing_leases list or there is a lease used by
/// someone else.
///
/// @param ctx client context that contains all details (subnet, client-id, etc.)
/// @param existing_leases leases that are already associated with the client
bool bool
allocateGlobalReservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases); allocateGlobalReservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment