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