[kea-dev] Design for Kea Host Reservation

Mon Oct 6 13:55:31 UTC 2014

Tomek,

Thanks for the review. I addressed most of the things you pointed out in
the review.

For those things that I haven't addressed I provide short responses below.

On 01/10/14 20:52, Tomek Mrugalski wrote:
> On 30.09.2014 13:20, Marcin Siodelski wrote:
>> I have created a new design document on Kea wiki:
>> http://kea.isc.org/wiki/HostReservationDesign for Host Reservation.
> Here are my comments. I like the design a lot. I think it will address
> our needs. We will probably tweak it a little bit over time, but it a
> very solid proposal. Here are my comments:
> 
> Information stored for HR section
> IP address(es) - should mention that list is for IPv6 only, v4 need a
> single address only. There's no need to reserve more than one IPv4
> address. Also, please explicitly say that IP means IPv4 and IPv6.
> 
> Temporary addresses. We do not support temporary addresses at all and
> in a sane environment, there is no requirement to reserve them in any
> way. On the contrary, it should be an explicit non-requirement. We
> should probably state that somewhere in the design (and hope that odd
> users wanting their temporaries to be static will never appear :)
> 
> We should add that currently there is no way to define what options
> can be sent for a given class. This capability will be added in the
> future.
> 
> I'd love to see DUID reservation example for DHCPv4.
> 
> I like the paragraph explaining what the prefixes (dhcp_, dhcp4_ etc.)
> mean. This is sort of intuitive, but it's nice to have it explicitly
> stated.
> 
> Shouldn't host_id and reservation_id be of type primary key? The diagram
> in MySQL database schema suggests it is a simple int. I'm not a
> MySQL expert, so this is an honest question. I think that setting a
> field to primary key type introduces three things: first, it is being
> assigned automatically during inserts. Second, the next value is being
> stored and applied automatically (autoincrement). Third, a unique
> restriction is added.

Yes, they are supposed to be primary key. I don't know why the SQL has
been generated without them being primary keys.

> 
> I like the BEFORE DELETE trigger, but I think we need something
> similar for the other way. Something like BEFORE INSERT that checks
> that the host_id exists when adding entries into ipv6_reserverations.
> Is there a BEFORE UPDATE trigger? It could be useful, too.

No, this is handled by the fact that the ipv6_reservations table has an
"Identifying Relation" with a hosts table. The entry in the
ipv6_reservations table can't exist without an appropriate entry in the
hosts table. So, this is already handled.

> 
> Comment for dhcp_identifier in the table does not mention that it must
> not be null. I think we won't support a case when no identifier is
> given at all.
> 
> Description for ipv4_address is strange: "This value must not be set
> to NOT-NULL if dhcp4_subnet_id is not specified." It would be better
> to simply say "must be NULL if ..."
> 
> I'm thinking that we may specify hostname a bit more precisely. How
> about this: it may contain a single label, e.g. "host-foo" or a FQDN,
> e.g. "host-foo.example.org.". If only hostname is specified, it will
> be appended with domain name specified for a given subnet. I can
> imagine that sysadmins would hate to write down their own domain
> thousands of times.
> 
> Why do we need reservation_id in the ipv6_reservations at all? To
> refer to specific option when updating it manually? That's fine, but
> if that is the rationale for it, we should say in the description that
> the field is not going to be actively used by the server and can be
> used by third party during database maintenance.
> 
> prefix_len description should mention that allowed length is 0..128.
> 
> Thanks for explaining the rationale for splitting v4 and v6 options
> into separate tables. "Why?" was my immediate question when I saw the
> tables layout. Your explanation answers my doubt.
> 
> I'm a total MySQL noob, so forgive my ignorance, but why do we need
> ALLOW_INVALID_DATES? MySQL does not simply insert invalid dates, and
> sort of sanitizes it to zeros. MySQL doc
> (http://dev.mysql.com/doc/refman/5.5/en/sql-mode.html, also applies to
> older versions) says that invalid date of 2014-04-31 will be sanitized
> to 0000-00-00.

I don't have strong opinion here. In fact, the database schema doesn't
use any time values at the moment. So, having this one way or another
doesn't hurt. The reason why it is set to this value is that the MySQL
workbench tends to inject this into the generated SQL. How about we
revisit this if we introduce some time/date columns to the database?

> 
> Space description: It may be useful to add a reference to Kea ARM,
> section 6.2.9 (v4) and 7.2.11 (v6) for people who are unfamiliar with
> the concept of option spaces. It is pretty Kea and ISC DHCP specific.
> 
> The section about how we interpret dhcp_client_class, host_id and
> subnet_id need to be more precise. People may read it, think that
> we'll support something and the be disappointed if we don't. It think
> the following approach will give us the most flexibility:
> 
> SELECT ... FROM dhcp4_options WHERE subnet_id=X AND (host_id=Y OR
> dhcp_client_class in (class1,class2,...,classN))
> 
> I'm not sure if the way how you organized the relation between
> SrvConfig and CfgHost and HostContainer are optimal. I would think
> that since the host reservations are defined per subnet, it should be
> SrvConfig keeps Subnet4 and Subnet6 collections (as it does now) and
> each Subnet{4,6} can contain zero or more CfgHost objects. The
> difference is actually important from performance perspective.
> Imagine 100 subnets with 1000 hosts in each. In the solution you
> propose you'd have to pick the appropriate reservation out of 100.000
> for every request. It will be much slower than picking it out of 1000
> reservations. You don't waste any additional time for picking subnet
> as that is done in the code already and is unavoidable.

Marcin