Commit 3e09b745 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[3689] Addressed primary review comments

doc/guide/dhcp6-srv.xml
    Updated "Reserving a hostname" section
    (also removed trailing spaces)

src/bin/dhcp6/dhcp6_srv.cc
   corrected test and commentary typo in assignIA_NA
   removed second parameter to renewLeases6() calls

src/bin/dhcp6/dhcp6_srv.h
    updated commentary for createContext()

src/lib/dhcpsrv/alloc_engine.cc
src/lib/dhcpsrv/alloc_engine.h
    AllocEngine::createLease6()
        removed find_host parameter and logic which calls
        findReservation and/or alters context hostname

src/lib/dhcpsrv/tests/alloc_engine_utils.cc
    AllocEngine6Test::renewTest() - added call to
    AllocEngine6Test::findReservation()

src/lib/dhcpsrv/tests/alloc_engine_utils.h
    fixed typo
parent 323fa8b6
......@@ -1178,7 +1178,7 @@ should include options from the isc option space:
"data" is not set the parser will assume that this parameter is not
specified and an error will be reported.
</para>
<para>Note that it is possible to create an option which carries some data
in addition to the sub-options defined in the encapsulated option space.
For example, if the "container" option from the previous example was
......@@ -1728,8 +1728,8 @@ should include options from the isc option space:
The third row in the table above describes the case in which the client
requests that no DNS updates be done. The parameter, "override-no-update",
can be used to instruct the server to disregard the client's wishes. When
this parameter is true, kea-dhcp6 will generate DDNS update requests to
kea-dhcp-ddns even if the client requests no updates be done. The N-S-O
this parameter is true, kea-dhcp6 will generate DDNS update requests to
kea-dhcp-ddns even if the client requests no updates be done. The N-S-O
flags in the server's response to the client will be 0-1-1.
</para>
<para>
......@@ -2031,11 +2031,64 @@ should include options from the isc option space:
<section id="reservation6-hostname">
<title>Reserving a hostname</title>
<!-- @todo: replace this with the actual text once #3689 is implemented -->
<para>Reserving a hostname is currently not supported. It is possible
to specify that information in the configuration file, but that data
is not used by the server engine yet.</para>
</section>
<para>When the reservation for the client includes the <command>hostname
</command>, the server will assign this hostname to the client and send
it back in the Client FQDN, if the client sent the FQDN option to the
the server. The reserved hostname always takes precedence over the hostname
supplied by the client (via the FQDN option) or the autogenerated
(from the IPv6 address) hostname.</para>
<para>The server qualifies the reserved hostname with the value
of the <command>qualifying-suffix</command> parameter. For example, the
following subnet configuration:
<screen>
"subnet6": [
{
"subnet": "2001:db8:1::/48",
"pools": [ { "pool": "2001:db8:1::/80" } ],
"reservations": [
{
"duid": "01:02:03:04:05:0A:0B:0C:0D:0E",
"ip-addresses": [ "2001:db8:1::100" ]
"hostname": "alice-laptop"
}
]
}
],
"dhcp-ddns": {
"enable-updates": true,
"qualifying-suffix": "example.isc.org."
}
</screen>
will result in assigning the "alice-laptop.example.isc.org." hostname to the
client using the DUID "01:02:03:04:05:0A:0B:0C:0D:0E". If the <command>qualifying-suffix
</command> is not specified, the default (empty) value will be used, and
in this case the value specified as a <command>hostname</command> will
be treated as fully qualified name. Thus, by leaving the
<command>qualifying-suffix</command> empty it is possible to qualify
hostnames for the different clients with different domain names:
<screen>
"subnet6": [
{
"subnet": "2001:db8:1::/48",
"pools": [ { "pool": "2001:db8:1::/80" } ],
"reservations": [
{
"duid": "01:02:03:04:05:0A:0B:0C:0D:0E",
"ip-addresses": [ "2001:db8:1::100" ]
"hostname": "mark-desktop.example.org."
}
]
}
],
"dhcp-ddns": {
"enable-updates": true,
}
</screen>
will result in assigning the "mark-desktip.example.org." hostname to the
client using the DUID "01:02:03:04:05:0A:0B:0C:0D:0E".
</para>
</section>
<section id="reservation6-options">
<title>Reserving specific options</title>
......@@ -2110,7 +2163,7 @@ should include options from the isc option space:
]
}
</screen>
</para>
</para>
</section>
<!-- @todo: add support for per IA reservation (that specifies IAID in
......@@ -2270,7 +2323,7 @@ should include options from the isc option space:
"relay": {
"ip-address": "3000::1"
}</userinput>
},
},
{
"subnet": "2001:db8:1::/64",
......
......@@ -224,7 +224,8 @@ Dhcpv6Srv::testUnicast(const Pkt6Ptr& pkt) const {
return (true);
}
DuidPtr Dhcpv6Srv::extractClientId(const Pkt6Ptr& pkt) {
DuidPtr
Dhcpv6Srv::extractClientId(const Pkt6Ptr& pkt) {
// Let's find client's DUID. Client is supposed to include its client-id
// option almost all the time (the only exception is an anonymous inf-request,
// but that is mostly a theoretical case). Our allocation engine needs DUID
......@@ -237,7 +238,8 @@ DuidPtr Dhcpv6Srv::extractClientId(const Pkt6Ptr& pkt) {
}
}
AllocEngine::ClientContext6 Dhcpv6Srv::createContext(const Pkt6Ptr& pkt) {
AllocEngine::ClientContext6
Dhcpv6Srv::createContext(const Pkt6Ptr& pkt) {
AllocEngine::ClientContext6 ctx;
ctx.subnet_ = selectSubnet(pkt);
ctx.duid_ = extractClientId(pkt);
......@@ -1271,8 +1273,8 @@ Dhcpv6Srv::assignIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
// to say that we are sorry, but the user won't get an address. As a convenience, we
// use a different status text to indicate that (compare to the same status code,
// but different wording below)
if (!orig_ctx.subnet_) {
// Creatasse empty IA_NA option with IAID matching the request.
if (!subnet) {
// Create an empty IA_NA option with IAID matching the request.
// Note that we don't use OptionDefinition class to create this option.
// This is because we prefer using a constructor of Option6IA that
// initializes IAID. Otherwise we would have to use setIAID() after
......@@ -1613,7 +1615,7 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
ctx.allow_new_leases_in_renewals_ = true;
}
Lease6Collection leases = alloc_engine_->renewLeases6(ctx, false);
Lease6Collection leases = alloc_engine_->renewLeases6(ctx);
// Ok, now we have the leases extended. We have:
// - what the client tried to renew in ctx.hints_
......@@ -1796,7 +1798,7 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query,
// - old_leases - leases that used to be, but are no longer valid
// - changed_leases - leases that have FQDN changed (not really important
// in PD context)
Lease6Collection leases = alloc_engine_->renewLeases6(ctx, false);
Lease6Collection leases = alloc_engine_->renewLeases6(ctx);
// For all the leases we have now, add the IAPPREFIX with non-zero lifetimes
for (Lease6Collection::const_iterator l = leases.begin(); l != leases.end(); ++l) {
......
......@@ -649,8 +649,14 @@ protected:
/// @brief Creates client context for specified packet
///
/// Creates context that includes subnet, client-id, hw address and
/// possibly other parameters.
/// Instantiates the ClientContext6 and then:
/// - Performs the subnet selection and stores the result in context
/// - Extracts the duid from the packet and saves it to the context
/// - Extracts the hardware address from the packet and saves it to
/// the context
/// - Performs host reservation lookup and stores the result in the
/// context
///
/// @return client context
AllocEngine::ClientContext6 createContext(const Pkt6Ptr& pkt);
......
......@@ -1006,7 +1006,7 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
}
Lease6Collection
AllocEngine::renewLeases6(ClientContext6& ctx, bool find_host) {
AllocEngine::renewLeases6(ClientContext6& ctx) {
try {
if (!ctx.subnet_) {
isc_throw(InvalidOperation, "Subnet is required for allocation");
......@@ -1016,21 +1016,6 @@ AllocEngine::renewLeases6(ClientContext6& ctx, bool find_host) {
isc_throw(InvalidOperation, "DUID is mandatory for allocation");
}
// We can attempt to find appropriate reservation
if (find_host) {
findReservation(ctx);
}
// Let's check whether there's a hostname specified in the reservation
if (ctx.host_) {
std::string hostname = ctx.host_->getHostname();
// If there is, let's use it
if (!hostname.empty()) {
ctx.hostname_ = hostname;
}
}
// Check if there are any leases for this client.
Lease6Collection leases = LeaseMgrFactory::instance()
.getLeases6(ctx.type_, *ctx.duid_, ctx.iaid_, ctx.subnet_->getID());
......
......@@ -498,16 +498,8 @@ public:
/// prefixes the client had sent. @ref ClientContext6::old_leases_ will
/// contain removed leases in this case.
///
/// @param find_resrv specifies whether the code should search for host
/// reservation. true means that the code will consult HostMgr, false means
/// to skip this check. That is easier to use, but is redundant if the
/// ctx.host_ field is already set. We can't use ctx.host_ == NULL as
/// check, because for cases whithout reservations, the reservation
/// search would be repeated.
///
///
/// @return Returns renewed lease.
Lease6Collection renewLeases6(ClientContext6& ctx, bool find_resrv = true);
Lease6Collection renewLeases6(ClientContext6& ctx);
/// @brief Attempts to find appropriate host reservation.
///
......
......@@ -230,6 +230,7 @@ AllocEngine6Test::renewTest(AllocEngine& engine, const Pool6Ptr& pool,
ctx.query_.reset(new Pkt6(DHCPV6_RENEW, 123));
ctx.allow_new_leases_in_renewals_ = allow_new_leases_in_renewal;
findReservation(engine, ctx);
Lease6Collection leases = engine.renewLeases6(ctx);
for (Lease6Collection::iterator it = leases.begin(); it != leases.end(); ++it) {
......
......@@ -113,7 +113,7 @@ public:
/// @brief Wrapper around call to AllocEngine6::findRervation
///
/// If a reservation is found bg the engine, the function sets
/// If a reservation is found by the engine, the function sets
/// ctx.hostname_ accordingly.
///
/// @param engine allocation engine to use
......
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