BIND 10 #3207: per-client option values (hook lib extension)

BIND 10 Development do-not-reply at isc.org
Tue Dec 31 22:04:06 UTC 2013


#3207: per-client option values (hook lib extension)
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tomek
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp          |                    Milestone:  DHCP-
            Keywords:                |  Kea1.0-alpha
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * owner:  tmark => tomek


Comment:

 In addition to the things I addressed below, I had to move the helper
 functions
 in pkt_send.cc out of the "extern C" linkage.  I was getting compiler
 errors
 informing me that functions returning C++ objects are not valid under C
 linakge.
 Makes sense.  I also added namespace user_chk to keep the user registry
 classes
 isolated into their own space.  This also groups then nicely in doxygen.


 Replying to [comment:5 tomek]:
 > '''load_unload.cc'''
 > What does the format of IPv4 used in default_user4_id_str mean?
 > Is it IPv4 address converted to hex? That's ok, I'd just like
 > to have a very short comment that explain it. The same
 > applies to default_user6_id_str.

 Added commentary.

 >
 > '''pkt_receive_co.cc'''
 >
 > pkt4_receive() function comment:
 > Shouldn't "query_user_id_label be "query_user_id", not
 > "query_used_id_label"? That's what I understood from the comment
 > for that function, but the value from load_unload.cc has
 > different value (with _label in it, which I think is not
 > necessary).

 Oops, fixed it.

 >
 > '''pkt_send_co.cc'''
 > The comment for pkt4_send is incorrect. The callout does not
 > modify vendor options, it modifies normal, standard options
 > (tftp-server and bootfile server).
 >

 I altered the commentary to simply refer to options.

 > pkt4_send: There's a @todo about determining the list of types. I
 > think the current code (modify every packet, except NAK) is correct.
 > There's no need to revisit it in the future, so the @todo can
 > be safely removed.

 Got it.

 >
 > TODOs in add6Options about option codes: When merging with
 > master, you can update those and remove the @todo tags in line
 > 291.

 Done.

 >
 > getV6AddrStr(): space after function name and parentheses that
 > open arguments list is not needed.

 Oops.

 >
 > getV6AddrStr: Please add a @note comment that this will fetch
 > only the first address or prefix in case there are more addresses
 > and/or prefixes. (No need to update the code, just the comments)

 Got it.

 >
 > '''user_chk.h'''
 > Please swap charcters 17 and 18 in line 45 (should be
 > "char* default_user6_id_str")

 Dang it. Fixed.
 >
 > Please add a comment that the constants are defined in load_unload.cc

 Done.
 >
 > -------------------
 >
 > Please add a .dox page that explains how user_chk is working (from
 admin's
 > perspective). It doesn't have to be extensive. An introductory paragraph
 explaining
 > the concept with a list of steps required to use it will do the trick.
 Make
 > sure it is mentioned on the main page in Hooks framework directory. It
 seems
 > reasonable to call it an example hook library.
 >
 > Please add a !ChangeLog entry.

 How is this? (The library was introduced in ChangeLog entry 697)

 {{{
 7xx.    [func]  tmark

     Updates the "user_check" hooks shared library with callouts for packet
     receive and packet send.  Decision outcome now includes the lease or
 prefix
     assigned.  The user registry now supports a "default" user entry.
     (Trac #3207, git TBD)
 }}}

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


More information about the bind10-tickets mailing list