BIND 10 #3232: Kea6: Develop REBIND support

[BIND 10 Development]

#3232: Kea6: Develop REBIND support
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tomek
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  Unclassified  |                    Milestone:  DHCP-
            Keywords:                |  Kea0.9-alpha
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  39            |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  4
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tomek):

 * hours:  28 => 4
 * totalhours:  35 => 39


Comment:

 '''doc/guide/bind10-guide.xml'''
 Your changes are correct, but I noticed that there are some
 mistakes. I mentioned them in #3258 ticket.

 '''src/bin/dhcp6/dhcp6_messages.mes'''
 DHCP6_EXTEND_LEASE_SUBNET_SELECTED should give some information about
 the client. Source address and/or DUID would be useful.

 sleep/recovery => sleep/resume

 This may mean that faulty the mobile client =>
 This may mean that the faulty mobile client

 '''src/bin/dhcp6/dhcpv6_srv.cc'''
 extendIA_NA:
 simpliclty => simplicity

 The comment is valid, but somewhat incomplete. We have a
 LeasMgr::getLease6(type, address) query. This could be used
 to pick up the lease. The issue this comment talks about still
 persists, though. If you change the subnet, currently only the
 subnet configuration changes, but not the leases. In particular,
 their addresses are still in old subnet and their subnet_id remains
 unchanged. This is a problem, but I don't think this can be
 addressed in this ticket. We have #3281, which will address
 some of the issues, but definitely not all. Supporting full
 reconfiguration is a big feature on its own.

 Do you think it would be appropriate to check if subnet, duid, query,
 answer and ia are non-null? The code just dereferences them assuming
 non-null. boost::shared_ptr will throw if there's an attempt to
 dereference NULL, so perhaps its execption will be caught somehwere
 higher.

 the code assumes (line 1661) that every packet that is not RENEW is
 automatically REBIND. I think there should be a check and exception
 thrown if the query of type other than RENEW or REBIND.

 extendIA_PD
 Let's create a IA_PD => Let's create an IA_PD

 Exception being thrown in line 1742 has unclosed parentheses.

 The wrapping in the following code
 {{{
     Lease6Ptr lease =
         LeaseMgrFactory::instance().getLease6(Lease::TYPE_PD,
                                               *duid, ia->getIAID(),
                                               subnet->getID());
 }}}
 looks really ugly. Perhaps it's a good time to make the decision to
 extend the code width to 100 characters?

 Comment in line 1883:
 {{{
     // Execute all callouts registered for packet6_send
 }}}
 is incorrect. its lease6_renew or lease6_rebind hooks are being
 called.

 extendLeases:
 The second @todo in line 1916 mentions that the check can be
 removed. That is not really true. What if hooks remove the client-id
 option?

 Review paused on line 1020 of the diff file. The review will continue
 next week.

-- 
Ticket URL: <http://bind10.isc.org/ticket/3232#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development