BIND 10 trac2995, updated. 0d98fbf5f919f2ca5a5134bd6d3527bed6606d10 [2995] Changes after review.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Jul 10 09:57:59 UTC 2013


The branch, trac2995 has been updated
       via  0d98fbf5f919f2ca5a5134bd6d3527bed6606d10 (commit)
      from  682cac0da7b318a81746d93424a638faa867ee7c (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 0d98fbf5f919f2ca5a5134bd6d3527bed6606d10
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Wed Jul 10 11:57:43 2013 +0200

    [2995] Changes after review.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp6/dhcp6_hooks.dox             |    8 +--
 src/bin/dhcp6/dhcp6_srv.cc                |   36 +++++++---
 src/bin/dhcp6/tests/dhcp6_srv_unittest.cc |  105 ++++++++++++++++++++---------
 3 files changed, 105 insertions(+), 44 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_hooks.dox b/src/bin/dhcp6/dhcp6_hooks.dox
index 1b8d16f..3f447c3 100644
--- a/src/bin/dhcp6/dhcp6_hooks.dox
+++ b/src/bin/dhcp6/dhcp6_hooks.dox
@@ -19,7 +19,7 @@
  BIND10 features an API (the "Hooks" API) that allows user-written code to
  be integrated into BIND 10 and called at specific points in its processing.
  An overview of the API and a tutorial for writing generalised hook code can
- API can be found in @ref hookDevelopersGuide.
+ API can be found in @ref hooksDevelopersGuide and @ref hooksComponentDeveloperGuide.
 
  This manual is more specialised and is aimed at developers of hook
  code for the DHCPv6 server. It describes each hook point, the callouts
@@ -51,7 +51,7 @@ packet processing. Hook points that are not specific to packet processing
  @subsection dhcpv6HooksPkt6Receive pkt6_receive
 
  - @b Arguments:
-   - name: @b pkt6, type: isc::dhcp::Pkt6Ptr, direction: <b>in/out</b>
+   - name: @b query6, type: isc::dhcp::Pkt6Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when an incoming DHCPv6
    packet is received and its content is parsed. The sole argument -
@@ -71,7 +71,7 @@ packet processing. Hook points that are not specific to packet processing
 @subsection dhcpv6HooksSubnet6Select subnet6_select
 
  - @b Arguments:
-   - name: @b pkt6, type: isc::dhcp::Pkt6Ptr, direction: <b>in/out</b>
+   - name: @b query6, type: isc::dhcp::Pkt6Ptr, direction: <b>in/out</b>
    - name: @b subnet6, type: isc::dhcp::Subnet6Ptr, direction: <b>in/out</b>
    - name: @b subnet6collection, type: const isc::dhcp::Subnet6Collection&, direction: <b>in</b>
 
@@ -113,7 +113,7 @@ packet processing. Hook points that are not specific to packet processing
 @subsection dhcpv6HooksPkt6Send pkt6_send
 
  - @b Arguments:
-   - name: @b pkt6, type: isc::dhcp::Pkt6Ptr, direction: <b>in/out</b>
+   - name: @b response6, type: isc::dhcp::Pkt6Ptr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when server's response
    is about to be send back to the client. The sole argument - pkt6 -
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 9da17c6..3389837 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -210,8 +210,13 @@ bool Dhcpv6Srv::run() {
             if (HooksManager::getHooksManager().calloutsPresent(hook_index_pkt6_receive_)) {
                 CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-                // This is the first callout, so no need to clear any arguments
-                callout_handle->setArgument("pkt6", query);
+                // Delete previously set arguments
+                callout_handle->deleteAllArguments();
+
+                // Pass incoming packet as argument
+                callout_handle->setArgument("query6", query);
+
+                // Call callouts
                 HooksManager::getHooksManager().callCallouts(hook_index_pkt6_receive_,
                                                 *callout_handle);
 
@@ -223,7 +228,7 @@ bool Dhcpv6Srv::run() {
                     continue;
                 }
 
-                callout_handle->getArgument("pkt6", query);
+                callout_handle->getArgument("query6", query);
             }
 
             try {
@@ -298,7 +303,7 @@ bool Dhcpv6Srv::run() {
 
                 // Execute all callouts registered for packet6_send
                 if (HooksManager::getHooksManager().calloutsPresent(hook_index_pkt6_send_)) {
-                    boost::shared_ptr<CalloutHandle> callout_handle = getCalloutHandle(query);
+                    CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
                     // Delete all previous arguments
                     callout_handle->deleteAllArguments();
@@ -307,7 +312,7 @@ bool Dhcpv6Srv::run() {
                     callout_handle->setSkip(false);
 
                     // Set our response
-                    callout_handle->setArgument("pkt6", rsp);
+                    callout_handle->setArgument("response6", rsp);
 
                     // Call all installed callouts
                     HooksManager::getHooksManager().callCallouts(hook_index_pkt6_send_,
@@ -654,12 +659,17 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) {
 
     // Let's execute all callouts registered for packet_received
     if (HooksManager::getHooksManager().calloutsPresent(hook_index_subnet6_select_)) {
-        boost::shared_ptr<CalloutHandle> callout_handle = getCalloutHandle(question);
+        CalloutHandlePtr callout_handle = getCalloutHandle(question);
+
+        // We're reusing callout_handle from previous calls
+        callout_handle->deleteAllArguments();
 
-        // This is the first callout, so no need to clear any arguments
-        callout_handle->setArgument("pkt6", question);
+        // Set new arguments
+        callout_handle->setArgument("query6", question);
         callout_handle->setArgument("subnet6", subnet);
         callout_handle->setArgument("subnet6collection", CfgMgr::instance().getSubnets6());
+
+        // Call user (and server-side) callouts
         HooksManager::getHooksManager().callCallouts(hook_index_subnet6_select_,
                                         *callout_handle);
 
@@ -1223,8 +1233,16 @@ Dhcpv6Srv::processInfRequest(const Pkt6Ptr& infRequest) {
 }
 
 isc::hooks::CalloutHandlePtr Dhcpv6Srv::getCalloutHandle(const Pkt6Ptr& pkt) {
-    CalloutHandlePtr callout_handle;
+    // This method returns a CalloutHandle for a given packet. It is guaranteed
+    // to return the same callout_handle (so user library contexts are
+    // preserved). This method works well if the server processes one packet
+    // at a time. Once the server architecture is extended to cover parallel
+    // packets processing (e.g. delayed-ack, some form of buffering etc.), this
+    // method has to be extended (e.g. store callouts in a map and use pkt as
+    // a key). Additional code would be required to release the callout handle
+    // once the server finished processing.
 
+    CalloutHandlePtr callout_handle;
     static Pkt6Ptr old_pointer;
 
     if (!callout_handle ||
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 12490c8..5c7f5b6 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -1831,11 +1831,17 @@ TEST_F(Dhcpv6SrvTest, Hooks) {
     NakedDhcpv6Srv srv(0);
 
     // check if appropriate hooks are registered
+    int hook_index_pkt6_received = -1;
+    int hook_index_select_subnet = -1;
+    int hook_index_pkt6_send     = -1;
 
     // check if appropriate indexes are set
-    int hook_index_pkt6_received = ServerHooks::getServerHooks().getIndex("pkt6_receive");
-    int hook_index_select_subnet = ServerHooks::getServerHooks().getIndex("subnet6_select");
-    int hook_index_pkt6_send     = ServerHooks::getServerHooks().getIndex("pkt6_send");
+    EXPECT_NO_THROW(hook_index_pkt6_received = ServerHooks::getServerHooks()
+                    .getIndex("pkt6_receive"));
+    EXPECT_NO_THROW(hook_index_select_subnet = ServerHooks::getServerHooks()
+                    .getIndex("subnet6_select"));
+    EXPECT_NO_THROW(hook_index_pkt6_send     = ServerHooks::getServerHooks()
+                    .getIndex("pkt6_send"));
 
     EXPECT_TRUE(hook_index_pkt6_received > 0);
     EXPECT_TRUE(hook_index_select_subnet > 0);
@@ -1900,6 +1906,8 @@ Pkt6* captureSimpleSolicit() {
 class HooksDhcpv6SrvTest : public Dhcpv6SrvTest {
 
 public:
+
+    /// @brief creates Dhcpv6Srv and prepares buffers for callouts
     HooksDhcpv6SrvTest() {
 
         // Allocate new DHCPv6 Server
@@ -1909,10 +1917,19 @@ public:
         resetCalloutBuffers();
     }
 
+    /// @brief destructor (deletes Dhcpv6Srv)
     ~HooksDhcpv6SrvTest() {
         delete srv_;
     }
 
+    /// @brief creates an option with specified option code
+    ///
+    /// This method is static, because it is used from callouts
+    /// that do not have a pointer to HooksDhcpv6SSrvTest object
+    ///
+    /// @param option_code code of option to be created
+    ///
+    /// @return pointer to create option object
     static OptionPtr createOption(uint16_t option_code) {
 
         char payload[] = {
@@ -1923,23 +1940,27 @@ public:
         return OptionPtr(new Option(Option::V6, option_code, tmp));
     }
 
-    /// callback that stores received callout name and pkt6 value
+    /// test callback that stores received callout name and pkt6 value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     pkt6_receive_callout(CalloutHandle& callout_handle) {
         callback_name_ = string("pkt6_receive");
 
-        callout_handle.getArgument("pkt6", callback_pkt6_);
+        callout_handle.getArgument("query6", callback_pkt6_);
 
         callback_argument_names_ = callout_handle.getArgumentNames();
         return (0);
     }
 
-    // callback that changes client-id value
+    /// test callback that changes client-id value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     pkt6_receive_change_clientid(CalloutHandle& callout_handle) {
 
         Pkt6Ptr pkt;
-        callout_handle.getArgument("pkt6", pkt);
+        callout_handle.getArgument("query6", pkt);
 
         // get rid of the old client-id
         pkt->delOption(D6O_CLIENTID);
@@ -1951,12 +1972,14 @@ public:
         return pkt6_receive_callout(callout_handle);
     }
 
-    // callback that deletes client-id
+    /// test callback that deletes client-id
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     pkt6_receive_delete_clientid(CalloutHandle& callout_handle) {
 
         Pkt6Ptr pkt;
-        callout_handle.getArgument("pkt6", pkt);
+        callout_handle.getArgument("query6", pkt);
 
         // get rid of the old client-id
         pkt->delOption(D6O_CLIENTID);
@@ -1965,12 +1988,14 @@ public:
         return pkt6_receive_callout(callout_handle);
     }
 
-    // callback that sets skip flag
+    /// test callback that sets skip flag
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     pkt6_receive_skip(CalloutHandle& callout_handle) {
 
         Pkt6Ptr pkt;
-        callout_handle.getArgument("pkt6", pkt);
+        callout_handle.getArgument("query6", pkt);
 
         callout_handle.setSkip(true);
 
@@ -1978,23 +2003,27 @@ public:
         return pkt6_receive_callout(callout_handle);
     }
 
-    // Callback that stores received callout name and pkt6 value
+    /// Test callback that stores received callout name and pkt6 value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     pkt6_send_callout(CalloutHandle& callout_handle) {
         callback_name_ = string("pkt6_send");
 
-        callout_handle.getArgument("pkt6", callback_pkt6_);
+        callout_handle.getArgument("response6", callback_pkt6_);
 
         callback_argument_names_ = callout_handle.getArgumentNames();
         return (0);
     }
 
-    // Callback that changes server-id
+    // Test callback that changes server-id
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     pkt6_send_change_serverid(CalloutHandle& callout_handle) {
 
         Pkt6Ptr pkt;
-        callout_handle.getArgument("pkt6", pkt);
+        callout_handle.getArgument("response6", pkt);
 
         // get rid of the old server-id
         pkt->delOption(D6O_SERVERID);
@@ -2006,12 +2035,14 @@ public:
         return pkt6_send_callout(callout_handle);
     }
 
-    // callback that deletes server-id
+    /// test callback that deletes server-id
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     pkt6_send_delete_serverid(CalloutHandle& callout_handle) {
 
         Pkt6Ptr pkt;
-        callout_handle.getArgument("pkt6", pkt);
+        callout_handle.getArgument("response6", pkt);
 
         // get rid of the old client-id
         pkt->delOption(D6O_SERVERID);
@@ -2020,12 +2051,14 @@ public:
         return pkt6_send_callout(callout_handle);
     }
 
-    // Callback that sets skip blag
+    /// Test callback that sets skip flag
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     pkt6_send_skip(CalloutHandle& callout_handle) {
 
         Pkt6Ptr pkt;
-        callout_handle.getArgument("pkt6", pkt);
+        callout_handle.getArgument("response6", pkt);
 
         callout_handle.setSkip(true);
 
@@ -2033,12 +2066,14 @@ public:
         return pkt6_send_callout(callout_handle);
     }
 
-    // Callback that stores received callout name and subnet6 values
+    /// Test callback that stores received callout name and subnet6 values
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     subnet6_select_callout(CalloutHandle& callout_handle) {
         callback_name_ = string("subnet6_select");
 
-        callout_handle.getArgument("pkt6", callback_pkt6_);
+        callout_handle.getArgument("query6", callback_pkt6_);
         callout_handle.getArgument("subnet6", callback_subnet6_);
         callout_handle.getArgument("subnet6collection", callback_subnet6collection_);
 
@@ -2046,7 +2081,9 @@ public:
         return (0);
     }
 
-    // Callback that picks the other subnet if possible.
+    /// Test callback that picks the other subnet if possible.
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
     static int
     subnet6_select_different_subnet_callout(CalloutHandle& callout_handle) {
 
@@ -2067,6 +2104,7 @@ public:
         return (0);
     }
 
+    /// resets buffers used to store data received by callouts
     void resetCalloutBuffers() {
         callback_name_ = string("");
         callback_pkt6_.reset();
@@ -2075,28 +2113,33 @@ public:
         callback_argument_names_.clear();
     }
 
+    /// pointer to Dhcpv6Srv that is used in tests
     NakedDhcpv6Srv* srv_;
 
-    // Used in testing pkt6_receive_callout
-    static string callback_name_; ///< string name of the received callout
+    // The following fields are used in testing pkt6_receive_callout
+
+    /// String name of the received callout
+    static string callback_name_;
 
-    static Pkt6Ptr callback_pkt6_; ///< Pkt6 structure returned in the callout
+    /// Pkt6 structure returned in the callout
+    static Pkt6Ptr callback_pkt6_;
 
+    /// Pointer to a subnet received by callout
     static Subnet6Ptr callback_subnet6_;
 
+    /// A list of all available subnets (received by callout)
     static Subnet6Collection callback_subnet6collection_;
 
+    /// A list of all received arguments
     static vector<string> callback_argument_names_;
 };
 
+// The following fields are used in testing pkt6_receive_callout.
+// See fields description in the class for details
 string HooksDhcpv6SrvTest::callback_name_;
-
 Pkt6Ptr HooksDhcpv6SrvTest::callback_pkt6_;
-
 Subnet6Ptr HooksDhcpv6SrvTest::callback_subnet6_;
-
 Subnet6Collection HooksDhcpv6SrvTest::callback_subnet6collection_;
-
 vector<string> HooksDhcpv6SrvTest::callback_argument_names_;
 
 
@@ -2131,7 +2174,7 @@ TEST_F(HooksDhcpv6SrvTest, simple_pkt6_receive) {
 
     // Check that all expected parameters are there
     vector<string> expected_argument_names;
-    expected_argument_names.push_back(string("pkt6"));
+    expected_argument_names.push_back(string("query6"));
 
     EXPECT_TRUE(expected_argument_names == callback_argument_names_);
 }
@@ -2253,7 +2296,7 @@ TEST_F(HooksDhcpv6SrvTest, simple_pkt6_send) {
 
     // Check that all expected parameters are there
     vector<string> expected_argument_names;
-    expected_argument_names.push_back(string("pkt6"));
+    expected_argument_names.push_back(string("response6"));
     EXPECT_TRUE(expected_argument_names == callback_argument_names_);
 }
 



More information about the bind10-changes mailing list