Opened 7 years ago

Closed 7 years ago

#2140 closed task (fixed)

Create abstract API for lease database access

Reported by: stephen Owned by: tomek
Priority: medium Milestone: Sprint-DHCP-20121018
Component: dhcp Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This is the creation of a fully-documented header file defining the API into the lease database. The actual work of accessing the database will be done by database-specific code.

Subtickets

Change History (9)

comment:1 Changed 7 years ago by tomek

  • Owner set to tomek
  • Status changed from new to accepted

comment:2 Changed 7 years ago by tomek

  • Owner changed from tomek to UnAssigned
  • Status changed from accepted to reviewing

Abstract API for lease database implemented. Please note that:

  • host reservations are not planned for 2012, so they are not implemented (appropriate @todo is added to the code, though)
  • backend versioning is currently simplified. Full featured versioning is not needed in 2012 timeframe, so it was skipped. There's a comment about planned version checks in LeaseMgr::getVersion() method.

LeaseMgr? handles only information that will be kept in the database. Therefore there are no subnet or pool information methods. Those will be implemented in upcoming CfgMgr? (that will hold all configuration options).

The code is available on trac2140 branch. Please review.

comment:3 Changed 7 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:4 Changed 7 years ago by stephen

  • Owner changed from stephen to tomek

Reviewed commit 0b7e9646307a6e58d3e1df60c68cbac5dbcbfecb

src/lib/dhcp/duid.cc
DUID constructor: use "const uint8_t* data" instead of "const uint8_t * data"

ClientId constructor: use "const uint8_t* clientid" instead of "const uint8_t *clientid"

src/lib/dhcp/duid.h
Avoid spaces in definition of operation methods (e.g. "operator==(" instead of "operator == (")

getClientId() returns a copy of the client ID, not a reference to it (as indicated in the "@brief" description.)

operator!=(): could probably write this inline - "return (! operator==(other))".

ClientId::getAddress(): (return statement) should not be spaces after an opening parenthesis or before closing one.

Any reason not to split ClientId and DUID classes (and their tests) into separate files (makes it easier to find the code)?

src/lib/dhcp/lease_mgr.h
lease4.fqdn_fwd_ header - we would update A records for an IPv4 lease.

lease4.options_ header - typo - "databased".

Is the overhead of keeping a shared point to a client ID justified? Why not keep a copy of the ID?

getVersion() is a good idea, but we also have the version of the underlying database product to consider. However, I suggest we keep it simple for now and just check the schema version. (Anyway, I would expect the version of the underlying API (and backend) will be compatible with the rest of the code as they will all be distributed together.) As for schema version, I suggest we go for major/minor number (as per BIND) - see this comment on #963 for some details.

getLeaseX() functions: how often do we expect the lease to be accessed by address/hardware address/client ID? I ask because that affects the indexing - we should probably have an index for each of them, but that will slow down the rate at which leases can be added to the database (because of the need to update indexes).

src/lib/dhcp/tests/duid_unittest.cc
When doing tests, is there a need to create DUIDs though "new"? Creating them on the stack would be equally effective (and would mean that no scoped_ptr is needed to guarantee their removal).

src/lib/dhcp/tests/lease_mgr_unittest.cc
Avoid using constructs opf the form "type * variable" - prefer "type* variable" (in "constructor" test).

comment:5 follow-up: Changed 7 years ago by sar

I have one architectural level question and a set of simpler questions or
comments.

The architectural level issue deal with getting information from the db
via the lease_mgr api. In the current code a given key may map to several
different leases on different subnets. These could either be somewhat fixed,
for example a laptop used for testing that has a set of fixed addresses
and can be carried from subnet to subnet. Or they could by dynamic, for
example if a laptop is moved around a campus and acquires multiple leases.

The functions that find a lease via some key (clientid, hwaddr etc)
don't seem to allow for either finding a lease within a subnet or
walking through all of the leases with that key until one for
the proper subnet is found.

Comments on DUID (dhid.h, dhid.cc)

The return of a pointer to the private vector in the get functions causes me
a certain amount of concern. Howerver I don't have any concrete problems with
it and therefore it's probably fine as is until how the rest of code might
usae the pointer becomes more clear.

I have a larger concern about the typing in the structures. Perhaps it won't
happen but it seems like there is some potential for confusion between the
different types if the structures can be mixed. It also seems that deciding
a 4 byte id is an IPv4 address might not always be true.

I'm not sure about putting an IPv4 address into the same structure - do you
have a use case for that already?

Lastly did you consider adding functions to create the various types of DUIDs?
(LL, LLT etc) It seems to be that such functions would be useful and would
go into this class.

Comments on lease_mgr (lease_mgr.h, lease_mgr.cc)

Is there a description of how the lease structures will be used (in a
general sense)? I'm imagining the lease_mgr structure being used as a
temporary holder to get the data from the backend db, manipulate it
and then push the changes back to the backend. Is that somewhat
correct?

Are you planning on including the type of the hardware addess as the
first element in the hardware address vector?

There are several additional timestamps associated with failover.
I suppose they can be deferred for now and added if/when we do failover.

I'm curious about the choice for valid_lft. Making it seconds since cltt
means that most uses of it will require getting cltt and doing an addition.
Storing it as a time_t in its own right might take more space but might make
the code more efficient.

Does recylce_time need to be in the structure? If the lease structures aren't
permanent I can see the use of this functionality to provide a "grace" period
for a user to return and reclaim an address. However I'm a bit iffy on needing
it on a lease by lease basis vs server wide basis.

I'm not sure we need the DDNS flags here. In the current DDNS requirements it
appears that there will probably be a separate DDNS structure to track DDNS
information - the names in use, if A, AAAA or PTR have been updated etc.
(Such an arrangement is likely required by the desire to be able to remove DDNS
information when a lease expires even if the range has already been removed.
We might be able to keep the lease structures around but that seems like it
might be overly complicated.) In my mind I'm thinking of the DDNS module as
a somewhat separate entity. It would be informed of all updates to the lease
(requests, renews, rebinds, releases, expires etc) and would update its own db
and the DNS server as necessary. In this model the flags woudld go in that
other structure not in the lease. One downside of this is that the lease won't
always have the same name information as the DNS.

The v6 lease structure includes t1 and t2 is there a reason not to include them
in the v4 structure? The trend in the current code is to handle them in a
similar fashion and it also makes serving leasequeries easier.

Comments on the test code:

It would probably be good to add a short description to each of
the tests to indicate what they are trying to test.

While it wouldn't matter all that much to the tests it might be
useful to use something that is closer to the full format
for an LLT or LL.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 7 years ago by tomek

  • Owner changed from tomek to sar

Replying to sar:

I have one architectural level question and a set of simpler questions or
comments.

The architectural level issue deal with getting information from the db
via the lease_mgr api. In the current code a given key may map to several
different leases on different subnets. These could either be somewhat fixed,
for example a laptop used for testing that has a set of fixed addresses
and can be carried from subnet to subnet. Or they could by dynamic, for
example if a laptop is moved around a campus and acquires multiple leases.

Excellent point. I've updated several getLeaseX methods and added several new ones. In general, if we are very specific and want a single lease for a given subnet, we can get at most one lease, so the method returns a single pointer. On the other hand if we don't specify subnet_id, it is possible to get more than one lease, so the function returns a collection (basically a vector of pointers).

There are getLease() methods that pass address as a parameter. As for any given address there could be at most one lease, they return a single pointer.

The functions that find a lease via some key (clientid, hwaddr etc)
don't seem to allow for either finding a lease within a subnet or
walking through all of the leases with that key until one for
the proper subnet is found.

They now do.

Comments on DUID (dhid.h, dhid.cc)

The return of a pointer to the private vector in the get functions causes me
a certain amount of concern. Howerver I don't have any concrete problems with
it and therefore it's probably fine as is until how the rest of code might
usae the pointer becomes more clear.

I've changed them to return values rather than references. That means that they are safer, but much less efficient, so those methods should be used only sporadically. If we find a frequent use for them, we co could toss them altogether, but we will have to replace them with with storeSelf() or similar that will store the actual content somewhere (in a buffer?)
when sending out a packet.

I have a larger concern about the typing in the structures. Perhaps it won't
happen but it seems like there is some potential for confusion between the
different types if the structures can be mixed.

Do you mean DUID and ClientID? No, not really. At first I thought if we could define DHCPv4 client-id as yet another DUID type and then use DUIDs in both v4 and v6. However, I decided that is not that useful and would make the use a bit problematic, e.g. what type should getType() return?

There are two reasons why ClientID was derived from DUID class. First, it couldn't stay as vector<uint8_t>, because that is the same type as hwaddress - compiler was confused by getLease4(hwaddr) and getLease4(client_id). The second reason is that both have exactly the same operations and data stored, so it reuses some of the code in both classes.

It also seems that deciding a 4 byte id is an IPv4 address might not always be true.

Do you prefer to add a flag that specifies that client-id is really an address or get rid of

    ClientId(const isc::asiolink::IOAddress& addr);
    isc::asiolink::IOAddress getAddress() const;
    bool isAddress() const;

altogether? I don't have any preference and will do whatever you recommend.

I'm not sure about putting an IPv4 address into the same structure - do you
have a use case for that already?

No, I haven't. That seems to answer my previous question. Ok, aforementioned methods has been removed. ClientId? is now an opaque value.

Lastly did you consider adding functions to create the various types of DUIDs?
(LL, LLT etc) It seems to be that such functions would be useful and would
go into this class.

Yes, we will do that eventually. See Dhcpv6Srv::setServerID() in src/bin/dhcp6/dhcp6_srv.cc. It was implemented before we had DUID class. I've created ticket #2352 for DUID work.

Comments on lease_mgr (lease_mgr.h, lease_mgr.cc)

Is there a description of how the lease structures will be used (in a
general sense)? I'm imagining the lease_mgr structure being used as a
temporary holder to get the data from the backend db, manipulate it
and then push the changes back to the backend. Is that somewhat
correct?

Yes. It is correct. There is no description as that will be worked out soon. See scary large number of tickets in the current sprint starting with "allocation engine".

There was a discussion that some of the leases may be kept in memory as a form of cache, but that is a performance optimization and it will not be implemented in 2012. Another way lease structures will eventually be used is in memfile backend - basically a C++ version of what we have in ISC-DHCP.

Are you planning on including the type of the hardware addess as the
first element in the hardware address vector?

I haven't really thought about it, but since we are doing this in ISC-DHCP and there are no obvious issues with it, we can do it again.

There are several additional timestamps associated with failover.
I suppose they can be deferred for now and added if/when we do failover.

Yes. I have added a comment that the lease structure will be expanded with the failover specific things.

I'm curious about the choice for valid_lft. Making it seconds since cltt
means that most uses of it will require getting cltt and doing an addition.
Storing it as a time_t in its own right might take more space but might make
the code more efficient.

Making it time_t would also mean that you need to update it every time the lease is refreshed. With the current approach, unless you do something weird (like a first renewal after reconfiguration) the only thing you need to do with renewal is to update cltt. That may have impact on DB performance. Stephen (who is currently working on DB backend) suggested that we should reverse the logic and keep the timestamp of expiration and cltt as a difference to that. That will make his idea for housekeeping process that sweeps through database and removes expired leases a bit simpler to implement.

Also, the argument about avoiding a simple arithmetic is somewhat weak. We will have to use subtraction to calculate the valid-lifetime field. If we decide to keep everything as time_t values, we will get into much worse troubles, as we could get inconsistent values. I can agree to any solution that has one timestamp with the other fields specified as diffs to that. I don't really care about which one that is. Keeping cltt as a timestamp was the simplest approach as then all other fields are expressed as always positive differences. Also, they can then be directly used when creating a message.

If you don't agree with that approach, we (Stephen, you and me) should have a discussion about it and make a compromise.

Does recylce_time need to be in the structure? If the lease structures aren't
permanent I can see the use of this functionality to provide a "grace" period
for a user to return and reclaim an address. However I'm a bit iffy on needing
it on a lease by lease basis vs server wide basis.

Good point. That would be only useful if we wanted to have the grace period changeable on a subnet basis. I have removed recycle_time from both lease structures.

I'm not sure we need the DDNS flags here. In the current DDNS requirements it
appears that there will probably be a separate DDNS structure to track DDNS
information - the names in use, if A, AAAA or PTR have been updated etc.
(Such an arrangement is likely required by the desire to be able to remove DDNS
information when a lease expires even if the range has already been removed.
We might be able to keep the lease structures around but that seems like it
might be overly complicated.) In my mind I'm thinking of the DDNS module as
a somewhat separate entity. It would be informed of all updates to the lease

Fully agree here. However, we need to provide that info to DDNS module somehow. I envisage the module as a relatively simple stateless (sort of his state would be limited to currently ongoing updates) machine.

I do not want it to keep separate table with information that is closely coupled with lease information. First, such a separation would allow inconsistency in the database (some administrator deleted leases, but forgot to delete dns transactions related to it). Second, many implementations do fqdn in renewals. That means that the number of queries required for handling renewal would double. Third, we need to take into consideration that this information must survive restarts. If we keep the lease and the necessary DNS info (fqdn, was A/AAAA updated, was PTR updated) separate, we will need to do costly sanity checks between lease database and some table that hold dns info.

(requests, renews, rebinds, releases, expires etc) and would update its own db
and the DNS server as necessary. In this model the flags woudld go in that
other structure not in the lease. One downside of this is that the lease won't
always have the same name information as the DNS.

I was more imagining DNS module as something very simple, little more than a wrapper around nsupdate (functionally speaking, the actual implementation will be completely different).


The v6 lease structure includes t1 and t2 is there a reason not to include them
in the v4 structure? The trend in the current code is to handle them in a
similar fashion and it also makes serving leasequeries easier.

Good catch. Added.

Comments on the test code:

It would probably be good to add a short description to each of
the tests to indicate what they are trying to test.

Added comments to all tests. In fact, I think such descriptions should be a bit more structured. I have created a ticket #2350 for that purpose.

While it wouldn't matter all that much to the tests it might be
useful to use something that is closer to the full format
for an LLT or LL.

We will get do that once we implement support for LLT and LL. #2352 description was updated to cover that as well.

Thank you for your review. All changes are in git. Please check if they address your comments.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 7 years ago by sar

The changes look fine. I have snipped out anything I don't need to clarify.

Replying to tomek:

Replying to sar:

Comments on DUID (dhid.h, dhid.cc)

The return of a pointer to the private vector in the get functions causes me
a certain amount of concern. Howerver I don't have any concrete problems with
it and therefore it's probably fine as is until how the rest of code might
usae the pointer becomes more clear.

I've changed them to return values rather than references. That means that they are safer, but much less efficient, so those methods should be used only sporadically. If we find a frequent use for them, we co could toss them altogether, but we will have to replace them with with storeSelf() or similar that will store the actual content somewhere (in a buffer?)
when sending out a packet.

That looks fine, if we find that it's a performance issue we can do something else - including bringing back the code you removed. Until we know how it will be used I think it's best to be safe.


I have a larger concern about the typing in the structures. Perhaps it won't
happen but it seems like there is some potential for confusion between the
different types if the structures can be mixed.

Do you mean DUID and ClientID? No, not really. At first I thought if we could define DHCPv4 client-id as yet another DUID type and then use DUIDs in both v4 and v6. However, I decided that is not that useful and would make the use a bit problematic, e.g. what type should getType() return?

Yes I was thinking of DUID vs ClientID. If you haven't already you may wish to add an item to construct a client iD that is based on a DUID (see rfc4361). I think mostly that would be useful for a client built from Kea so there's no hurry for it.

Are you planning on including the type of the hardware addess as the
first element in the hardware address vector?

I haven't really thought about it, but since we are doing this in ISC-DHCP and there are no obvious issues with it, we can do it again.

It seems like a convenient way to keep the type associated with the address. Another item that's not an immediate issue but perhaps should be mentioned is that the hardware address may not be used in some cases. For example when using Infiniband hardware the hardware address is too long to fit into a DHCPv4 header.

There are several additional timestamps associated with failover.
I suppose they can be deferred for now and added if/when we do failover.

Yes. I have added a comment that the lease structure will be expanded with the failover specific things.

I'm curious about the choice for valid_lft. Making it seconds since cltt
means that most uses of it will require getting cltt and doing an addition.
Storing it as a time_t in its own right might take more space but might make
the code more efficient.

Making it time_t would also mean that you need to update it every time the lease is refreshed. With the current approach, unless you do something weird (like a first renewal after reconfiguration) the only thing you need to do with renewal is to update cltt. That may have impact on DB performance. Stephen (who is currently working on DB backend) suggested that we should reverse the logic and keep the timestamp of expiration and cltt as a difference to that. That will make his idea for housekeeping process that sweeps through database and removes expired leases a bit simpler to implement.

Also, the argument about avoiding a simple arithmetic is somewhat weak. We will have to use subtraction to calculate the valid-lifetime field. If we decide to keep everything as time_t values, we will get into much worse troubles, as we could get inconsistent values. I can agree to any solution that has one timestamp with the other fields specified as diffs to that. I don't really care about which one that is. Keeping cltt as a timestamp was the simplest approach as then all other fields are expressed as always positive differences. Also, they can then be directly used when creating a message.

If you don't agree with that approach, we (Stephen, you and me) should have a discussion about it and make a compromise.

I don't have a strong opinion at this time. Your argument is reasonable but I still have some concerns. I think we can start with this and adjust it as necessary as we progress. The two areas I have concerns about are:

1) in failover the two peers exchange time information and converting it to a delta might be difficult or annoying. But we aren't doing failover yet so I don't think that's a reason to delay.

2) I think there may be some optimizations we can use to avoid updating the DB and I'm not sure how they will fit into a scheme of cltt and deltas. For example we may choose to do some form of lazy update where what is written into the DB is longer than what is passed to the client such that the next time the client makes a request the server doesn't need to write to the db before replying. Again we aren't doing this yet so this isn't a reason to delay either.

One side note we may elect to change the lease duration several times during a client's lifetime. This happens in failover as the server switches from MCLT to the actual lease time and we may want to consider some sort of adaptive lease times as well. There was a discussion of this on the dhcp-users mailing list that I found interesting. In any case the number of times we change the lease duration is likely to be a good deal less than the number of times clot gets updated.

I'm not sure we need the DDNS flags here. In the current DDNS requirements it
appears that there will probably be a separate DDNS structure to track DDNS
information - the names in use, if A, AAAA or PTR have been updated etc.
(Such an arrangement is likely required by the desire to be able to remove DDNS
information when a lease expires even if the range has already been removed.
We might be able to keep the lease structures around but that seems like it
might be overly complicated.) In my mind I'm thinking of the DDNS module as
a somewhat separate entity. It would be informed of all updates to the lease

Fully agree here. However, we need to provide that info to DDNS module somehow. I envisage the module as a relatively simple stateless (sort of his state would be limited to currently ongoing updates) machine.

I do not want it to keep separate table with information that is closely coupled with lease information. First, such a separation would allow inconsistency in the database (some administrator deleted leases, but forgot to delete dns transactions related to it). Second, many implementations do fqdn in renewals. That means that the number of queries required for handling renewal would double. Third, we need to take into consideration that this information must survive restarts. If we keep the lease and the necessary DNS info (fqdn, was A/AAAA updated, was PTR updated) separate, we will need to do costly sanity checks between lease database and some table that hold dns info.

(requests, renews, rebinds, releases, expires etc) and would update its own db
and the DNS server as necessary. In this model the flags woudld go in that
other structure not in the lease. One downside of this is that the lease won't
always have the same name information as the DNS.

I was more imagining DNS module as something very simple, little more than a wrapper around nsupdate (functionally speaking, the actual implementation will be completely different).

I think we probably need to have a discussion about this, perhaps in the a Kea conference call. In the DHCP4 code it would probably be better to split the two for several reasons. But the DB is held in memory so doing an extra "lookup" for the ddns bits isn't very costly. In the Kea code the tradeoffs will be different and the utility of a single lookup may outweigh the pain of keeping the various pieces in sync.

comment:8 Changed 7 years ago by sar

  • Owner changed from sar to tomek

comment:9 in reply to: ↑ 7 Changed 7 years ago by tomek

  • Resolution set to fixed
  • Status changed from reviewing to closed

Replying to sar:

Yes I was thinking of DUID vs ClientID. If you haven't already you may wish to add an item to construct a client iD that is based on a DUID (see rfc4361). I think mostly that would be useful for a client built from Kea so there's no hurry for it.

Added new ticket for that work #2354. As it would be useful for v4 clients based on Kea, that's a very low priority for now.

It seems like a convenient way to keep the type associated with the address. Another item that's not an immediate issue but perhaps should be mentioned is that the hardware address may not be used in some cases. For example when using Infiniband hardware the hardware address is too long to fit into a DHCPv4 header.

Ok. We will do that.

There are several additional timestamps associated with failover.
I suppose they can be deferred for now and added if/when we do failover.

Yes. I have added a comment that the lease structure will be expanded with the failover specific things.

I'm curious about the choice for valid_lft. Making it seconds since cltt
means that most uses of it will require getting cltt and doing an addition.
Storing it as a time_t in its own right might take more space but might make
the code more efficient.

Making it time_t would also mean that you need to update it every time the lease is refreshed. With the current approach, unless you do something weird (like a first renewal after reconfiguration) the only thing you need to do with renewal is to update cltt. That may have impact on DB performance. Stephen (who is currently working on DB backend) suggested that we should reverse the logic and keep the timestamp of expiration and cltt as a difference to that. That will make his idea for housekeeping process that sweeps through database and removes expired leases a bit simpler to implement.

Also, the argument about avoiding a simple arithmetic is somewhat weak. We will have to use subtraction to calculate the valid-lifetime field. If we decide to keep everything as time_t values, we will get into much worse troubles, as we could get inconsistent values. I can agree to any solution that has one timestamp with the other fields specified as diffs to that. I don't really care about which one that is. Keeping cltt as a timestamp was the simplest approach as then all other fields are expressed as always positive differences. Also, they can then be directly used when creating a message.

If you don't agree with that approach, we (Stephen, you and me) should have a discussion about it and make a compromise.

I don't have a strong opinion at this time. Your argument is reasonable but I still have some concerns. I think we can start with this and adjust it as necessary as we progress. The two areas I have concerns about are:

1) in failover the two peers exchange time information and converting it to a delta might be difficult or annoying. But we aren't doing failover yet so I don't think that's a reason to delay.

2) I think there may be some optimizations we can use to avoid updating the DB and I'm not sure how they will fit into a scheme of cltt and deltas. For example we may choose to do some form of lazy update where what is written into the DB is longer than what is passed to the client such that the next time the client makes a request the server doesn't need to write to the db before replying. Again we aren't doing this yet so this isn't a reason to delay either.

That's more or less cache-threshold freature from ISC-DHCP. Yes, we will likely to implement it. However, in the 2012 timeframe we chose to implement basic functionality first: everything done by the book, without any optimization. Once we get the code functionally stable, we can then start thinking about making it run faster.

One side note we may elect to change the lease duration several times during a client's lifetime. This happens in failover as the server switches from MCLT to the actual lease time and we may want to consider some sort of adaptive lease times as well. There was a discussion of this on the dhcp-users mailing list that I found interesting. In any case the number of times we change the lease duration is likely to be a good deal less than the number of times clot gets updated.

When I said "unless you do something weird", I really meant anything other than simple assignment and renewal. I think there are case when every field of the lease structure could be modified one way or the other. Regardless of the choice we make, some operations will be awkward or annoying. I think we should try to optimize for the most common cases.

I'm not sure we need the DDNS flags here. In the current DDNS requirements it
appears that there will probably be a separate DDNS structure to track DDNS
information - the names in use, if A, AAAA or PTR have been updated etc.
(Such an arrangement is likely required by the desire to be able to remove DDNS
information when a lease expires even if the range has already been removed.
We might be able to keep the lease structures around but that seems like it
might be overly complicated.) In my mind I'm thinking of the DDNS module as
a somewhat separate entity. It would be informed of all updates to the lease

Fully agree here. However, we need to provide that info to DDNS module somehow. I envisage the module as a relatively simple stateless (sort of his state would be limited to currently ongoing updates) machine.

I do not want it to keep separate table with information that is closely coupled with lease information. First, such a separation would allow inconsistency in the database (some administrator deleted leases, but forgot to delete dns transactions related to it). Second, many implementations do fqdn in renewals. That means that the number of queries required for handling renewal would double. Third, we need to take into consideration that this information must survive restarts. If we keep the lease and the necessary DNS info (fqdn, was A/AAAA updated, was PTR updated) separate, we will need to do costly sanity checks between lease database and some table that hold dns info.

(requests, renews, rebinds, releases, expires etc) and would update its own db
and the DNS server as necessary. In this model the flags woudld go in that
other structure not in the lease. One downside of this is that the lease won't
always have the same name information as the DNS.

I was more imagining DNS module as something very simple, little more than a wrapper around nsupdate (functionally speaking, the actual implementation will be completely different).

I think we probably need to have a discussion about this, perhaps in the a Kea conference call. In the DHCP4 code it would probably be better to split the two for several reasons. But the DB is held in memory so doing an extra "lookup" for the ddns bits isn't very costly. In the Kea code the tradeoffs will be different and the utility of a single lookup may outweigh the pain of keeping the various pieces in sync.

Yes, we can discuss it. As the DDNS code is not planned for 2012, we haven't done any specific designs or architectural decisions yet.

As all those issues are open topics and not really blocking this abstract API, I've merged the code.

Thanks for your review.

Note: See TracTickets for help on using tickets.