Merge branch 'master' into trac2036

ChangeLog

@@ -1,3 +1,8 @@
+446.	[bug]		muks
+	A number of warnings reported by Python about unclosed file and
+	socket objects were fixed. Some related code was also made safer.
+	(Trac #1828, git 464682a2180c672f1ed12d8a56fd0a5ab3eb96ed)
+
 445.	[bug]*		jinmei
 	The pre-install check for older SQLite3 DB now refers to the DB
 	file with the prefix of DESTDIR.  This ensures that 'make install'

Makefile.am

@@ -16,6 +16,8 @@ DISTCHECK_CONFIGURE_FLAGS = --disable-install-configurations
 # Use same --with-gtest flag if set
 DISTCHECK_CONFIGURE_FLAGS += $(DISTCHECK_GTEST_CONFIGURE_FLAG)
 
+dist_doc_DATA = AUTHORS COPYING ChangeLog README
+
 clean-cpp-coverage:
 	@if [ $(USE_LCOV) = yes ] ; then \
 		$(LCOV) --directory . --zerocounters; \

configure.ac

@@ -1320,6 +1320,7 @@ Developer:
   Valgrind Suppressions: $use_valgrind_suppressions
   C++ Code Coverage: $USE_LCOV
   Python Code Coverage: $USE_PYCOVERAGE
+  Logger checks: $enable_logger_checks
   Generate Manuals:  $enable_man
 
 END

doc/Doxyfile

@@ -579,7 +579,7 @@ INPUT                  = ../src/lib/exceptions ../src/lib/cc \
     ../src/lib/testutils ../src/lib/cache ../src/lib/server_common/ \
     ../src/bin/sockcreator/ ../src/lib/util/ ../src/lib/util/io/ \
     ../src/lib/resolve ../src/lib/acl ../src/bin/dhcp6 ../src/lib/dhcp \
-    ../src/bin/dhcp4 devel
+    ../src/bin/dhcp4 ../tests/tools/perfdhcp devel
 
 # This tag can be used to specify the character encoding of the source files
 # that doxygen parses. Internally doxygen uses the UTF-8 encoding, which is

doc/guide/Makefile.am

@@ -1,6 +1,7 @@
-EXTRA_DIST = bind10-guide.css
-EXTRA_DIST += bind10-guide.xml bind10-guide.html bind10-guide.txt
-EXTRA_DIST += bind10-messages.xml bind10-messages.html
+dist_doc_DATA = bind10-guide.txt
+dist_html_DATA = bind10-guide.css bind10-guide.html bind10-messages.html
+
+EXTRA_DIST = bind10-guide.xml bind10-messages.xml
 
 # This is not a "man" manual, but reuse this for now for docbook.
 if ENABLE_MAN

doc/guide/bind10-guide.xml

@@ -131,7 +131,9 @@
         and <command>b10-zonemgr</command> components require the
         libpython3 library and the Python _sqlite3.so module
         (which is included with Python).
-        The Python module needs to be built for the corresponding Python 3.
+        The <command>b10-stats-httpd</command> component uses the
+        Python pyexpat.so module.
+        The Python modules need to be built for the corresponding Python 3.
       </para>
 <!-- TODO: this will change ... -->
 

src/bin/bind10/tests/bind10_test.py.in

@@ -1055,22 +1055,29 @@ class TestPIDFile(unittest.TestCase):
         # dump PID to the file, and confirm the content is correct
         dump_pid(self.pid_file)
         my_pid = os.getpid()
-        self.assertEqual(my_pid, int(open(self.pid_file, "r").read()))
+        with open(self.pid_file, "r") as f:
+            self.assertEqual(my_pid, int(f.read()))
 
     def test_dump_pid(self):
         self.check_pid_file()
 
         # make sure any existing content will be removed
-        open(self.pid_file, "w").write('dummy data\n')
+        with open(self.pid_file, "w") as f:
+            f.write('dummy data\n')
         self.check_pid_file()
 
     def test_unlink_pid_file_notexist(self):
         dummy_data = 'dummy_data\n'
-        open(self.pid_file, "w").write(dummy_data)
+
+        with open(self.pid_file, "w") as f:
+            f.write(dummy_data)
+
         unlink_pid_file("no_such_pid_file")
+
         # the file specified for unlink_pid_file doesn't exist,
         # and the original content of the file should be intact.
-        self.assertEqual(dummy_data, open(self.pid_file, "r").read())
+        with open(self.pid_file, "r") as f:
+            self.assertEqual(dummy_data, f.read())
 
     def test_dump_pid_with_none(self):
         # Check the behavior of dump_pid() and unlink_pid_file() with None.
@@ -1079,9 +1086,14 @@ class TestPIDFile(unittest.TestCase):
         self.assertFalse(os.path.exists(self.pid_file))
 
         dummy_data = 'dummy_data\n'
-        open(self.pid_file, "w").write(dummy_data)
+
+        with open(self.pid_file, "w") as f:
+            f.write(dummy_data)
+
         unlink_pid_file(None)
-        self.assertEqual(dummy_data, open(self.pid_file, "r").read())
+
+        with open(self.pid_file, "r") as f:
+            self.assertEqual(dummy_data, f.read())
 
     def test_dump_pid_failure(self):
         # the attempt to open file will fail, which should result in exception.

src/bin/bindctl/tests/bindctl_test.py

@@ -425,6 +425,12 @@ class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
 
 class TestBindCmdInterpreter(unittest.TestCase):
 
+    def setUp(self):
+        self.old_stdout = sys.stdout
+
+    def tearDown(self):
+        sys.stdout = self.old_stdout
+
     def _create_invalid_csv_file(self, csvfilename):
         import csv
         csvfile = open(csvfilename, 'w')
@@ -447,19 +453,17 @@ class TestBindCmdInterpreter(unittest.TestCase):
         self.assertEqual(new_csv_dir, custom_cmd.csv_file_dir)
 
     def test_get_saved_user_info(self):
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
-        cmd = bindcmd.BindCmdInterpreter()
-        users = cmd._get_saved_user_info('/notexist', 'csv_file.csv')
-        self.assertEqual([], users)
-
-        csvfilename = 'csv_file.csv'
-        self._create_invalid_csv_file(csvfilename)
-        users = cmd._get_saved_user_info('./', csvfilename)
-        self.assertEqual([], users)
-        os.remove(csvfilename)
-        sys.stdout = old_stdout
-
+        with open(os.devnull, 'w') as f:
+            sys.stdout = f
+            cmd = bindcmd.BindCmdInterpreter()
+            users = cmd._get_saved_user_info('/notexist', 'csv_file.csv')
+            self.assertEqual([], users)
+
+            csvfilename = 'csv_file.csv'
+            self._create_invalid_csv_file(csvfilename)
+            users = cmd._get_saved_user_info('./', csvfilename)
+            self.assertEqual([], users)
+            os.remove(csvfilename)
 
 class TestCommandLineOptions(unittest.TestCase):
     def setUp(self):

src/bin/cmdctl/tests/cmdctl_test.py

@@ -84,6 +84,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.rfile = open("check.tmp", 'w+b')
 
     def tearDown(self):
+        sys.stdout.close()
         sys.stdout = self.old_stdout
         self.handler.rfile.close()
         os.remove('check.tmp')
@@ -306,6 +307,7 @@ class TestCommandControl(unittest.TestCase):
         self.cmdctl = MyCommandControl(None, True)
    
     def tearDown(self):
+        sys.stdout.close()
         sys.stdout = self.old_stdout
 
     def _check_config(self, cmdctl):
@@ -427,6 +429,9 @@ class TestSecureHTTPServer(unittest.TestCase):
                                          MyCommandControl, verbose=True)
 
     def tearDown(self):
+        # both sys.stdout and sys.stderr are the same, so closing one is
+        # sufficient
+        sys.stdout.close()
         sys.stdout = self.old_stdout
         sys.stderr = self.old_stderr
 

src/bin/ddns/ddns.py.in

@@ -25,6 +25,7 @@ import isc.ddns.session
 from isc.ddns.zone_config import ZoneConfig
 from isc.ddns.logger import ClientFormatter, ZoneFormatter
 from isc.config.ccsession import *
+from isc.config.module_spec import ModuleSpecError
 from isc.cc import SessionError, SessionTimeout, ProtocolError
 import isc.util.process
 import isc.util.cio.socketsession
@@ -34,6 +35,7 @@ from isc.server_common.dns_tcp import DNSTCPContext
 from isc.datasrc import DataSourceClient
 from isc.server_common.auth_command import auth_loadzone_command
 import select
+import time
 import errno
 
 from isc.log_messages.ddns_messages import *
@@ -67,24 +69,22 @@ else:
     SPECFILE_PATH = SPECFILE_PATH.replace("${prefix}", PREFIX)
 
 if "B10_FROM_BUILD" in os.environ:
-    AUTH_SPECFILE_PATH = os.environ["B10_FROM_BUILD"] + "/src/bin/auth"
     if "B10_FROM_SOURCE_LOCALSTATEDIR" in os.environ:
         SOCKET_FILE_PATH = os.environ["B10_FROM_SOURCE_LOCALSTATEDIR"]
     else:
         SOCKET_FILE_PATH = os.environ["B10_FROM_BUILD"]
 else:
     SOCKET_FILE_PATH = bind10_config.DATA_PATH
-    AUTH_SPECFILE_PATH = SPECFILE_PATH
 
 SPECFILE_LOCATION = SPECFILE_PATH + "/ddns.spec"
 SOCKET_FILE = SOCKET_FILE_PATH + '/ddns_socket'
-AUTH_SPECFILE_LOCATION = AUTH_SPECFILE_PATH + '/auth.spec'
 
-isc.util.process.rename()
-
-# Cooperating modules
-XFROUT_MODULE_NAME = 'Xfrout'
+# Cooperating or dependency modules
 AUTH_MODULE_NAME = 'Auth'
+XFROUT_MODULE_NAME = 'Xfrout'
+ZONEMGR_MODULE_NAME = 'Zonemgr'
+
+isc.util.process.rename()
 
 class DDNSConfigError(Exception):
     '''An exception indicating an error in updating ddns configuration.
@@ -143,15 +143,23 @@ def get_datasrc_client(cc_session):
         file = os.environ["B10_FROM_BUILD"] + "/bind10_zones.sqlite3"
     datasrc_config = '{ "database_file": "' + file + '"}'
     try:
-        return HARDCODED_DATASRC_CLASS, DataSourceClient('sqlite3',
-                                                         datasrc_config)
+        return (HARDCODED_DATASRC_CLASS,
+                DataSourceClient('sqlite3', datasrc_config), file)
     except isc.datasrc.Error as ex:
         class DummyDataSourceClient:
             def __init__(self, ex):
                 self.__ex = ex
             def find_zone(self, zone_name):
                 raise isc.datasrc.Error(self.__ex)
-        return HARDCODED_DATASRC_CLASS, DummyDataSourceClient(ex)
+        return (HARDCODED_DATASRC_CLASS, DummyDataSourceClient(ex), file)
+
+def add_pause(sec):
+    '''Pause a specified period for inter module synchronization.
+
+    This is a trivial wrapper of time.sleep, but defined as a separate function
+    so tests can customize it.
+    '''
+    time.sleep(sec)
 
 class DDNSServer:
     # The number of TCP clients that can be handled by the server at the same
@@ -181,8 +189,23 @@ class DDNSServer:
             self._cc.get_default_value('zones'))
         self._cc.start()
 
+        # Internal attributes derived from other modules.  They will be
+        # initialized via dd_remote_xxx below and will be kept updated
+        # through their callbacks.  They are defined as 'protected' so tests
+        # can examine them; but they are essentially private to the class.
+        #
+        # Datasource client used for handling update requests: when set,
+        # should a tuple of RRClass and DataSourceClient.  Constructed and
+        # maintained based on auth configuration.
+        self._datasrc_info = None
+        # A set of secondary zones, retrieved from zonemgr configuration.
+        self._secondary_zones = None
+
         # Get necessary configurations from remote modules.
-        self._cc.add_remote_config(AUTH_SPECFILE_LOCATION)
+        for mod in [(AUTH_MODULE_NAME, self.__auth_config_handler),
+                    (ZONEMGR_MODULE_NAME, self.__zonemgr_config_handler)]:
+            self.__add_remote_module(mod[0], mod[1])
+        # This should succeed as long as cfgmgr is up.
         isc.server_common.tsig_keyring.init_keyring(self._cc)
 
         self._shutdown = False
@@ -256,6 +279,88 @@ class DDNSServer:
             answer = create_answer(1, "Unknown command: " + str(cmd))
         return answer
 
+    def __add_remote_module(self, mod_name, callback):
+        '''Register interest in other module's config with a callback.'''
+
+        # Due to startup timing, add_remote_config can fail.  We could make it
+        # more sophisticated, but for now we simply retry a few times, each
+        # separated by a short period (3 times and 1 sec, arbitrary chosen,
+        # and hardcoded for now).  In practice this should be more than
+        # sufficient, but if it turns out to be a bigger problem we can
+        # consider more elegant solutions.
+        for n_try in range(0, 3):
+            try:
+                # by_name() version can fail with ModuleSpecError in getting
+                # the module spec because cfgmgr returns a "successful" answer
+                # with empty data if it cannot find the specified module.
+                # This seems to be a deviant behavior (see Trac #2039), but
+                # we need to deal with it.
+                self._cc.add_remote_config_by_name(mod_name, callback)
+                return
+            except (ModuleSpecError, ModuleCCSessionError) as ex:
+                logger.warn(DDNS_GET_REMOTE_CONFIG_FAIL, mod_name, n_try + 1,
+                            ex)
+                last_ex = ex
+                add_pause(1)
+        raise last_ex
+
+    def __auth_config_handler(self, new_config, module_config):
+        logger.info(DDNS_RECEIVED_AUTH_UPDATE)
+
+        # If we've got the config before and the new config doesn't update
+        # the DB file, there's nothing we should do with it.
+        # Note: there seems to be a bug either in bindctl or cfgmgr, and
+        # new_config can contain 'database_file' even if it's not really
+        # updated.  We still perform the check so we can avoid redundant
+        # resetting when the bug is fixed.  The redundant reset itself is not
+        # good, but such configuration update should not happen so often and
+        # it should be acceptable in practice.
+        if self._datasrc_info is not None and \
+                not 'database_file' in new_config:
+            return
+        rrclass, client, db_file = get_datasrc_client(self._cc)
+        self._datasrc_info = (rrclass, client)
+        logger.info(DDNS_AUTH_DBFILE_UPDATE, db_file)
+
+    def __zonemgr_config_handler(self, new_config, module_config):
+        logger.info(DDNS_RECEIVED_ZONEMGR_UPDATE)
+
+        # If we've got the config before and the new config doesn't update
+        # the secondary zone list, there's nothing we should do with it.
+        # (Same note as that for auth's config applies)
+        if self._secondary_zones is not None and \
+                not 'secondary_zones' in new_config:
+            return
+
+        # Get the latest secondary zones.  Use get_remote_config_value() so
+        # it can work for both the initial default case and updates.
+        sec_zones, _ = self._cc.get_remote_config_value(ZONEMGR_MODULE_NAME,
+                                                        'secondary_zones')
+        new_secondary_zones = set()
+        try:
+            # Parse the new config and build a new list of secondary zones.
+            # Unfortunately, in the current implementation, even an observer
+            # module needs to perform full validation.  This should be changed
+            # so that only post-validation (done by the main module) config is
+            # delivered to observer modules, but until it's supported we need
+            # to protect ourselves.
+            for zone_spec in sec_zones:
+                zname = Name(zone_spec['name'])
+                # class has the default value in case it's unspecified.
+                # ideally this should be merged within the config module, but
+                # the current implementation doesn't esnure that, so we need to
+                # subsitute it ourselves.
+                if 'class' in zone_spec:
+                    zclass = RRClass(zone_spec['class'])
+                else:
+                    zclass = RRClass(module_config.get_default_value(
+                            'secondary_zones/class'))
+                new_secondary_zones.add((zname, zclass))
+            self._secondary_zones = new_secondary_zones
+            logger.info(DDNS_SECONDARY_ZONES_UPDATE, len(self._secondary_zones))
+        except Exception as ex:
+            logger.error(DDNS_SECONDARY_ZONES_UPDATE_FAIL, ex)
+
     def trigger_shutdown(self):
         '''Initiate a shutdown sequence.
 
@@ -273,10 +378,13 @@ class DDNSServer:
         Perform any cleanup that is necessary when shutting down the server.
         Do NOT call this to initialize shutdown, use trigger_shutdown().
 
-        Currently, it only causes the ModuleCCSession to send a message that
-        this module is stopping.
         '''
+        # tell the ModuleCCSession to send a message that this module is
+        # stopping.
         self._cc.send_stopping()
+        # make sure any open socket is explicitly closed, per Python
+        # convention.
+        self._listen_socket.close()
 
     def accept(self):
         """
@@ -366,9 +474,8 @@ class DDNSServer:
         # Let an update session object handle the request.  Note: things around
         # ZoneConfig will soon be substantially revised.  For now we don't
         # bother to generalize it.
-        datasrc_class, datasrc_client = get_datasrc_client(self._cc)
-        zone_cfg = ZoneConfig([], datasrc_class, datasrc_client,
-                              self._zone_config)
+        zone_cfg = ZoneConfig(self._secondary_zones, self._datasrc_info[0],
+                              self._datasrc_info[1], self._zone_config)
         update_session = self._UpdateSessionClass(self.__request_msg,
                                                   remote_addr, zone_cfg)
         result, zname, zclass = update_session.handle()
@@ -605,7 +712,7 @@ def main(ddns_server=None):
         logger.info(DDNS_STOPPED_BY_KEYBOARD)
     except SessionError as e:
         logger.error(DDNS_CC_SESSION_ERROR, str(e))
-    except ModuleCCSessionError as e:
+    except (ModuleSpecError, ModuleCCSessionError) as e:
         logger.error(DDNS_MODULECC_SESSION_ERROR, str(e))
     except DDNSConfigError as e:
         logger.error(DDNS_CONFIG_ERROR, str(e))

src/bin/ddns/ddns_messages.mes

@@ -25,6 +25,12 @@ There was a low-level error when we tried to accept an incoming connection
 connections we already have, but this connection is dropped. The reason
 is logged.
 
+% DDNS_AUTH_DBFILE_UPDATE updated auth DB file to %1
+b10-ddns was notified of updates to the SQLite3 DB file that b10-auth
+uses for the underlying data source and on which b10-ddns needs to
+make updates.  b10-ddns then updated its internal setup so further
+updates would be made on the new DB.
+
 % DDNS_CC_SESSION_ERROR error reading from cc channel: %1
 There was a problem reading from the command and control channel. The
 most likely cause is that the msgq process is not running.
@@ -53,6 +59,29 @@ authoritative server shuts down, the connection would get closed. It also
 can mean the system is busy and can't keep up or that the other side got
 confused and sent bad data.
 
+% DDNS_GET_REMOTE_CONFIG_FAIL failed to get %1 module configuration %2 times: %3
+b10-ddns tried to get configuration of some remote modules for its
+operation, but it failed.  The most likely cause of this is that the
+remote module has not fully started up and b10-ddns couldn't get the
+configuration in a timely fashion.  b10-ddns attempts to retry it a
+few times, imposing a short delay, hoping it eventually succeeds if
+it's just a timing issue.  The number of total failed attempts is also
+logged.  If it reaches an internal threshold b10-ddns considers it a
+fatal error and terminates.  Even in that case, if b10-ddns is
+configured as a "dispensable" component (which is the default), the
+parent bind10 process will restart it, and there will be another
+chance of getting the remote configuration successfully.  These are
+not the optimal behavior, but it's believed to be sufficient in
+practice (there would normally be no failure in the first place).  If
+it really causes an operational trouble other than having a few of
+these log messages, please submit a bug report; there can be several
+ways to make it more sophisticated.  Another, less likely reason for
+having this error is because the remote modules are not actually
+configured to run.  If that's the case fixing the configuration should
+solve the problem - either by making sure the remote module will run
+or by not running b10-ddns (without these remote modules b10-ddns is
+not functional, so there's no point in running it in this case).
+
 % DDNS_MODULECC_SESSION_ERROR error encountered by configuration/command module: %1
 There was a problem in the lower level module handling configuration and
 control commands.  This could happen for various reasons, but the most likely
@@ -66,10 +95,21 @@ requests from it. The file descriptor number and the address where the request
 comes from is logged. The connection is over a unix domain socket and is likely
 coming from a b10-auth process.
 
+% DDNS_RECEIVED_AUTH_UPDATE received configuration updates from auth server
+b10-ddns is notified of updates to b10-auth configuration
+(including a report of the initial configuration) that b10-ddns might
+be interested in.
+
 % DDNS_RECEIVED_SHUTDOWN_COMMAND shutdown command received
 The ddns process received a shutdown command from the command channel
 and will now shut down.
 
+% DDNS_RECEIVED_ZONEMGR_UPDATE received configuration updates from zonemgr
+b10-ddns is notified of updates to b10-zonemgr's configuration
+(including a report of the initial configuration).  It may possibly
+contain changes to the secondary zones, in which case b10-ddns will
+update its internal copy of that configuration.
+
 % DDNS_REQUEST_PARSE_FAIL failed to parse update request: %1
 b10-ddns received an update request via b10-auth, but the received
 data failed to pass minimum validation: it was either broken wire
@@ -117,6 +157,31 @@ with the client address.
 The ddns process has successfully started and is now ready to receive commands
 and updates.
 
+% DDNS_SECONDARY_ZONES_UPDATE updated secondary zone list (%1 zones are listed)
+b10-ddns has successfully updated the internal copy of secondary zones
+obtained from b10-zonemgr, based on a latest update to zonemgr's
+configuration.  The number of newly configured (unique) secondary
+zones is logged.
+
+% DDNS_SECONDARY_ZONES_UPDATE_FAIL failed to update secondary zone list: %1
+An error message.  b10-ddns was notified of updates to a list of
+secondary zones from b10-zonemgr and tried to update its own internal
+copy of the list, but it failed.  This can happen if the configuration
+contains an error, and b10-zonemgr should also reject that update.
+Unfortunately, in the current implementation there is no way to ensure
+that both zonemgr and ddns have consistent information when an update
+contains an error; further, as of this writing zonemgr has a bug that
+it could partially update the list of secondary zones if part of the
+list has an error (see Trac ticket #2038).  b10-ddns still keeps
+running with the previous configuration, but it's strongly advisable
+to check log messages from zonemgr, and if it indicates there can be
+inconsistent state, it's better to restart the entire BIND 10 system
+(just restarting b10-ddns wouldn't be enough, because zonemgr can have
+partially updated configuration due to bug #2038).  The log message
+contains an error description, but it's intentionally kept simple as
+it's primarily a matter of zonemgr.  To know the details of the error,
+log messages of zonemgr should be consulted.
+
 % DDNS_SESSION session arrived on file descriptor %1
 A debug message, informing there's some activity on the given file descriptor.
 It will be either a request or the file descriptor will be closed. See

src/bin/ddns/tests/ddns_test.py

@@ -21,7 +21,10 @@ from isc.acl.acl import ACCEPT
 import isc.util.cio.socketsession
 from isc.cc.session import SessionTimeout, SessionError, ProtocolError
 from isc.datasrc import DataSourceClient
-from isc.config.ccsession import create_answer
+from isc.config import module_spec_from_file
+from isc.config.config_data import ConfigData
+from isc.config.ccsession import create_answer, ModuleCCSessionError
+from isc.config.module_spec import ModuleSpecError
 from isc.server_common.dns_tcp import DNSTCPContext
 import ddns
 import errno
@@ -56,6 +59,11 @@ TEST_TSIG_KEYRING.add(TEST_TSIG_KEY)
 # Another TSIG key not in the keyring, making verification fail
 BAD_TSIG_KEY = TSIGKey("example.com:SFuWd/q99SzF8Yzd1QbB9g==")
 
+# Incorporate it so we can use the real default values of zonemgr config
+# in the tests.
+ZONEMGR_MODULE_SPEC = module_spec_from_file(
+    os.environ["B10_FROM_BUILD"] + "/src/bin/zonemgr/zonemgr.spec")
+
 class FakeSocket:
     """
     A fake socket. It only provides a file number, peer name and accept method.
@@ -208,6 +216,13 @@ class MyCCSession(isc.config.ConfigData):
         self._sendmsg_exception = None # will be raised from sendmsg if !None
         self._recvmsg_exception = None # will be raised from recvmsg if !None
 
+        # Attributes to handle (faked) remote configurations
+        self.__callbacks = {}   # record callbacks for updates to remote confs
+        self._raise_mods = {}  # map of module to exceptions to be triggered
+                               # on add_remote.  settable by tests.
+        self._auth_config = {}  # faked auth cfg, settable by tests
+        self._zonemgr_config = {} # faked zonemgr cfg, settable by tests
+
     def start(self):
         '''Called by DDNSServer initialization, but not used in tests'''
         self._started = True
@@ -222,8 +237,27 @@ class MyCCSession(isc.config.ConfigData):
         """
         return FakeSocket(1)
 
-    def add_remote_config(self, spec_file_name):
-        pass
+    def add_remote_config_by_name(self, module_name, update_callback=None):
+        # If a list of exceptions is given for the module, raise the front one,
+        # removing that exception from the list (so the list length controls
+        # how many (and which) exceptions should be raised on add_remote).
+        if module_name in self._raise_mods.keys() and \
+                len(self._raise_mods[module_name]) != 0:
+            ex = self._raise_mods[module_name][0]
+            self._raise_mods[module_name] = self._raise_mods[module_name][1:]
+            raise ex('Failure requesting remote config data')
+
+        if update_callback is not None:
+            self.__callbacks[module_name] = update_callback
+        if module_name is 'Auth':
+            if module_name in self.__callbacks:
+                # ddns implementation doesn't use the 2nd element, so just
+                # setting it to None
+                self.__callbacks[module_name](self._auth_config, None)
+        if module_name is 'Zonemgr':
+            if module_name in self.__callbacks:
+                self.__callbacks[module_name](self._zonemgr_config,
+                                              ConfigData(ZONEMGR_MODULE_SPEC))
 
     def get_remote_config_value(self, module_name, item):
         if module_name == "Auth" and item == "database_file":
@@ -233,6 +267,14 @@ class MyCCSession(isc.config.ConfigData):
                 return [], True # default
             else:
                 return self.auth_datasources, False
+        if module_name == 'Zonemgr' and item == 'secondary_zones':
+            if item in self._zonemgr_config:
+                return self._zonemgr_config[item], False
+            else:
+                seczone_default = \
+                    ConfigData(ZONEMGR_MODULE_SPEC).get_default_value(
+                    'secondary_zones')
+                return seczone_default, True
 
     def group_sendmsg(self, msg, group):
         # remember the passed parameter, and return dummy sequence
@@ -299,6 +341,10 @@ class TestDDNSServer(unittest.TestCase):
         self.__select_answer = None
         self.__select_exception = None
         self.__hook_called = False
+        # Because we overwrite the _listen_socket, close any existing
+        # socket object.
+        if self.ddns_server._listen_socket is not None:
+            self.ddns_server._listen_socket.close()
         self.ddns_server._listen_socket = FakeSocket(2)
         ddns.select.select = self.__select
 
@@ -306,12 +352,15 @@ class TestDDNSServer(unittest.TestCase):
         self.__tcp_sock = FakeSocket(10, socket.IPPROTO_TCP)
         self.__tcp_ctx = DNSTCPContext(self.__tcp_sock)
         self.__tcp_data = b'A' * 12 # dummy, just the same size as DNS header
+        # some tests will override this, which will be restored in tearDown:
+        self.__orig_add_pause = ddns.add_pause
 
     def tearDown(self):
         ddns.select.select = select.select
         ddns.isc.util.cio.socketsession.SocketSessionReceiver = \
             isc.util.cio.socketsession.SocketSessionReceiver
         isc.server_common.tsig_keyring = self.orig_tsig_keyring
+        ddns.add_pause = self.__orig_add_pause
 
     def test_listen(self):
         '''
@@ -334,6 +383,9 @@ class TestDDNSServer(unittest.TestCase):
         # Now make sure the clear_socket really works
         ddns.clear_socket()
         self.assertFalse(os.path.exists(ddns.SOCKET_FILE))
+        # Let ddns object complete any necessary cleanup (not part of the test,
+        # but for suppressing any warnings from the Python interpreter)
+        ddnss.shutdown_cleanup()
 
     def test_initial_config(self):
         # right now, the only configuration is the zone configuration, whose
@@ -422,6 +474,112 @@ class TestDDNSServer(unittest.TestCase):
         acl = self.ddns_server._zone_config[(TEST_ZONE_NAME, TEST_RRCLASS)]
         self.assertEqual(ACCEPT, acl.execute(TEST_ACL_CONTEXT))
 
+    def test_datasrc_config(self):
+        # By default (in our faked config) it should be derived from the
+        # test data source
+        rrclass, datasrc_client = self.ddns_server._datasrc_info
+        self.assertEqual(RRClass.IN(), rrclass)
+        self.assertEqual(DataSourceClient.SUCCESS,
+                         datasrc_client.find_zone(Name('example.org'))[0])
+
+        # emulating an update.  calling add_remote_config_by_name is a
+        # convenient faked way to invoke the callback.  We set the db file
+        # to a bogus one; the current implementation will create an unusable
+        # data source client.
+        self.__cc_session.auth_db_file = './notexistentdir/somedb.sqlite3'
+        self.__cc_session._auth_config = \
+            {'database_file': './notexistentdir/somedb.sqlite3'}
+        self.__cc_session.add_remote_config_by_name('Auth')
+        rrclass, datasrc_client = self.ddns_server._datasrc_info
+        self.assertEqual(RRClass.IN(), rrclass)
+        self.assertRaises(isc.datasrc.Error,
+                          datasrc_client.find_zone, Name('example.org'))
+
+        # Check the current info isn't changed if the new config doesn't
+        # update it.
+        info_orig = self.ddns_server._datasrc_info
+        self.ddns_server._datasrc_info = 42 # dummy value, should be kept.
+        self.__cc_session._auth_config = {'other_config': 'value'}
+        self.__cc_session.add_remote_config_by_name('Auth')
+        self.assertEqual(42, self.ddns_server._datasrc_info)
+        self.ddns_server._datasrc_info = info_orig
+
+    def test_secondary_zones_config(self):
+        # By default it should be an empty list
+        self.assertEqual(set(), self.ddns_server._secondary_zones)
+
+        # emulating an update.
+        self.__cc_session._zonemgr_config = {'secondary_zones': [
+                {'name': TEST_ZONE_NAME_STR, 'class': TEST_RRCLASS_STR}]}
+        self.__cc_session.add_remote_config_by_name('Zonemgr')
+
+        # The new set of secondary zones should be stored.
+        self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
+                         self.ddns_server._secondary_zones)
+
+        # Similar to the above, but 'class' is unspecified.  The default value
+        # should be used.
+        self.__cc_session._zonemgr_config = {'secondary_zones': [
+                {'name': TEST_ZONE_NAME_STR}]}
+        self.__cc_session.add_remote_config_by_name('Zonemgr')
+        self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
+                         self.ddns_server._secondary_zones)
+
+        # The given list has a duplicate.  The resulting set should unify them.
+        self.__cc_session._zonemgr_config = {'secondary_zones': [
+                {'name': TEST_ZONE_NAME_STR, 'class': TEST_RRCLASS_STR},
+                {'name': TEST_ZONE_NAME_STR, 'class': TEST_RRCLASS_STR}]}
+        self.__cc_session.add_remote_config_by_name('Zonemgr')
+        self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
+                         self.ddns_server._secondary_zones)
+
+        # Check the 2ndary zones aren't changed if the new config doesn't
+        # update it.
+        seczones_orig = self.ddns_server._secondary_zones
+        self.ddns_server._secondary_zones = 42 # dummy value, should be kept.
+        self.__cc_session._zonemgr_config = {}
+        self.__cc_session.add_remote_config_by_name('Zonemgr')
+        self.assertEqual(42, self.ddns_server._secondary_zones)
+        self.ddns_server._secondary_zones = seczones_orig
+
+        # If the update config is broken, the existing set should be intact.
+        self.__cc_session._zonemgr_config = {'secondary_zones': [
+                {'name': 'good.example', 'class': TEST_RRCLASS_STR},
+                {'name': 'badd..example', 'class': TEST_RRCLASS_STR}]}
+        self.__cc_session.add_remote_config_by_name('Zonemgr')
+        self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
+                         self.ddns_server._secondary_zones)
+
+    def __check_remote_config_fail(self, mod_name, num_ex, expected_ex):
+        '''Subroutine for remote_config_fail test.'''
+
+        # fake pause function for inspection and to avoid having timeouts
+        added_pause = []
+        ddns.add_pause = lambda sec: added_pause.append(sec)
+
+        # In our current implementation, there will be up to 3 tries of
+        # adding the module, each separated by a 1-sec pause.  If all attempts
+        # fail the exception will be propagated.
+        exceptions = [expected_ex for i in range(0, num_ex)]
+        self.__cc_session._raise_mods = {mod_name: exceptions}
+        if num_ex >= 3:
+            self.assertRaises(expected_ex, ddns.DDNSServer, self.__cc_session)
+        else:
+            ddns.DDNSServer(self.__cc_session)
+        self.assertEqual([1 for i in range(0, num_ex)], added_pause)
+
+    def test_remote_config_fail(self):
+        # If getting config of Auth or Zonemgr fails on construction of
+        # DDNServer, it should result in an exception and a few times
+        # of retries.  We test all possible cases, changing the number of
+        # raised exceptions and the type of exceptions that can happen,
+        # which should also cover the fatal error case.
+        for i in range(0, 4):
+            self.__check_remote_config_fail('Auth', i, ModuleCCSessionError)
+            self.__check_remote_config_fail('Auth', i, ModuleSpecError)
+            self.__check_remote_config_fail('Zonemgr', i, ModuleCCSessionError)
+            self.__check_remote_config_fail('Zonemgr', i, ModuleSpecError)
+
     def test_shutdown_command(self):
         '''Test whether the shutdown command works'''
         self.assertFalse(self.ddns_server._shutdown)
@@ -1178,26 +1336,6 @@ class TestConfig(unittest.TestCase):
                          ddns.SOCKET_FILE)
         self.assertEqual(os.environ["B10_FROM_SOURCE"] +
                          "/src/bin/ddns/ddns.spec", ddns.SPECFILE_LOCATION)
-        self.assertEqual(os.environ["B10_FROM_BUILD"] +
-                         "/src/bin/auth/auth.spec",
-                         ddns.AUTH_SPECFILE_LOCATION)
-
-    def test_get_datasrc_client(self):
-        # The test sqlite DB should contain the example.org zone.
-        rrclass, datasrc_client = ddns.get_datasrc_client(self.__ccsession)
-        self.assertEqual(RRClass.IN(), rrclass)
-        self.assertEqual(DataSourceClient.SUCCESS,
-                         datasrc_client.find_zone(Name('example.org'))[0])
-
-    def test_get_datasrc_client_fail(self):
-        # DB file is in a non existent directory, and creatng the client
-        # will fail.  get_datasrc_client will return a dummy client, which
-        # will subsequently make find_zone() fail.
-        self.__ccsession.auth_db_file = './notexistentdir/somedb.sqlite3'
-        rrclass, datasrc_client = ddns.get_datasrc_client(self.__ccsession)
-        self.assertEqual(RRClass.IN(), rrclass)
-        self.assertRaises(isc.datasrc.Error,
-                          datasrc_client.find_zone, Name('example.org'))
 
 if __name__== "__main__":
     isc.log.resetUnitTestRootLogger()

src/bin/dhcp4/Makefile.am

@@ -32,6 +32,12 @@ pkglibexec_PROGRAMS = b10-dhcp4
 
 b10_dhcp4_SOURCES = main.cc dhcp4_srv.cc dhcp4_srv.h
 
+if USE_CLANGPP
+# Disable unused parameter warning caused by some of the
+# Boost headers when compiling with clang.
+b10_dhcp4_CXXFLAGS = -Wno-unused-parameter
+endif
+
 b10_dhcp4_LDADD = $(top_builddir)/src/lib/dhcp/libdhcp++.la
 b10_dhcp4_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 b10_dhcp4_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la

src/bin/dhcp4/tests/Makefile.am

@@ -50,6 +50,12 @@ dhcp4_unittests_SOURCES = ../dhcp4_srv.h ../dhcp4_srv.cc
 dhcp4_unittests_SOURCES += dhcp4_unittests.cc
 dhcp4_unittests_SOURCES += dhcp4_srv_unittest.cc
 
+if USE_CLANGPP
+# Disable unused parameter warning caused by some of the
+# Boost headers when compiling with clang.
+dhcp4_unittests_CXXFLAGS = -Wno-unused-parameter
+endif
+
 dhcp4_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 dhcp4_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 dhcp4_unittests_LDADD = $(GTEST_LDADD)

src/bin/dhcp6/Makefile.am

@@ -34,6 +34,12 @@ pkglibexec_PROGRAMS = b10-dhcp6
 
 b10_dhcp6_SOURCES = main.cc dhcp6_srv.cc dhcp6_srv.h
 
+if USE_CLANGPP
+# Disable unused parameter warning caused by some of the
+# Boost headers when compiling with clang.
+b10_dhcp6_CXXFLAGS = -Wno-unused-parameter
+endif
+
 b10_dhcp6_LDADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
 b10_dhcp6_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 b10_dhcp6_LDADD += $(top_builddir)/src/lib/log/liblog.la

src/bin/dhcp6/tests/Makefile.am

@@ -46,6 +46,12 @@ dhcp6_unittests_SOURCES = ../dhcp6_srv.h ../dhcp6_srv.cc
 dhcp6_unittests_SOURCES += dhcp6_unittests.cc
 dhcp6_unittests_SOURCES += dhcp6_srv_unittest.cc
 
+if USE_CLANGPP
+# Disable unused parameter warning caused by some of the
+# Boost headers when compiling with clang.
+dhcp6_unittests_CXXFLAGS = -Wno-unused-parameter
+endif
+
 dhcp6_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 dhcp6_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 dhcp6_unittests_LDADD = $(GTEST_LDADD)
@@ -53,6 +59,7 @@ dhcp6_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 dhcp6_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libdhcp++.la
 dhcp6_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 dhcp6_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+
 endif
 
 noinst_PROGRAMS = $(TESTS)

src/bin/msgq/msgq.py.in

@@ -177,6 +177,7 @@ class MsgQ:
             # (note this is a catch-all, but we reraise it)
             if os.path.exists(self.socket_file):
                 os.remove(self.socket_file)
+            self.listen_socket.close()
             raise e
 
         if self.poller:

src/bin/msgq/tests/msgq_test.py

@@ -156,6 +156,12 @@ class SendNonblock(unittest.TestCase):
         except socket.error:
             pass
 
+        # Explicitly close temporary socket pair as the Python
+        # interpreter expects it.  It may not be 100% exception safe,
+        # but since this is only for tests we prefer brevity.
+        read.close()
+        write.close()
+
     def test_infinite_sendmsg(self):
         """
         Tries sending messages (and not reading them) until it either times
@@ -218,6 +224,12 @@ class SendNonblock(unittest.TestCase):
                     os.kill(queue_pid, signal.SIGTERM)
         self.terminate_check(run)
 
+        # Explicitly close temporary socket pair as the Python
+        # interpreter expects it.  It may not be 100% exception safe,
+        # but since this is only for tests we prefer brevity.
+        queue.close()
+        out.close()
+
     def test_small_sends(self):
         """
         Tests sending small data many times.

src/bin/tests/process_rename_test.py.in

@@ -25,7 +25,8 @@ class TestRename(unittest.TestCase):
     def __scan(self, directory, script, fun):
         # Scan one script if it contains call to the renaming function
         filename = os.path.join(directory, script)
-        data = ''.join(open(filename).readlines())
+        with open(filename) as f:
+            data = ''.join(f.readlines())
         prettyname = 'src' + filename[filename.rfind('../') + 2:]
         self.assertTrue(fun.search(data),
             "Didn't find a call to isc.util.process.rename in " + prettyname)
@@ -53,8 +54,8 @@ class TestRename(unittest.TestCase):
         # Find all Makefile and extract names of scripts
         for (d, _, fs) in os.walk('@top_builddir@'):
             if 'Makefile' in fs:
-                makefile = ''.join(open(os.path.join(d,
-                    "Makefile")).readlines())
+                with open(os.path.join(d, "Makefile")) as f:
+                    makefile = ''.join(f.readlines())
                 for (var, _) in lines.findall(re.sub(excluded_lines, '',
                                                      makefile)):
                     for (script, _) in scripts.findall(var):

src/bin/xfrin/tests/xfrin_test.py

@@ -2127,7 +2127,8 @@ class TestXfrin(unittest.TestCase):
         self.assertFalse(self.xfr._module_cc.stopped);
         self.xfr.shutdown()
         self.assertTrue(self.xfr._module_cc.stopped);
-        sys.stderr= self.stderr_backup
+        sys.stderr.close()
+        sys.stderr = self.stderr_backup
 
     def _do_parse_zone_name_class(self):
         return self.xfr._parse_zone_name_and_class(self.args)

src/bin/xfrout/tests/xfrout_test.py.in

@@ -1196,26 +1196,28 @@ class TestUnixSockServer(unittest.TestCase):
         # We test with UDP, as it can be "connected" without other
         # endpoint.  Note that in the current implementation _guess_remote()
         # unconditionally returns SOCK_STREAM.
-        sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
-        sock.connect(('127.0.0.1', 12345))
-        self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
-                          ('127.0.0.1', 12345)),
-                         self.unix._guess_remote(sock.fileno()))
+        with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock:
+            sock.connect(('127.0.0.1', 12345))
+            self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
+                              ('127.0.0.1', 12345)),
+                             self.unix._guess_remote(sock.fileno()))
+
         if socket.has_ipv6:
             # Don't check IPv6 address on hosts not supporting them
-            sock = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM)
-            sock.connect(('::1', 12345))
-            self.assertEqual((socket.AF_INET6, socket.SOCK_STREAM,
-                              ('::1', 12345, 0, 0)),
-                             self.unix._guess_remote(sock.fileno()))
+            with socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) as sock:
+                sock.connect(('::1', 12345))
+                self.assertEqual((socket.AF_INET6, socket.SOCK_STREAM,
+                                  ('::1', 12345, 0, 0)),
+                                 self.unix._guess_remote(sock.fileno()))
+
             # Try when pretending there's no IPv6 support
             # (No need to pretend when there's really no IPv6)
             xfrout.socket.has_ipv6 = False
-            sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
-            sock.connect(('127.0.0.1', 12345))
-            self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
-                              ('127.0.0.1', 12345)),
-                             self.unix._guess_remote(sock.fileno()))
+            with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock:
+                sock.connect(('127.0.0.1', 12345))
+                self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
+                                  ('127.0.0.1', 12345)),
+                                 self.unix._guess_remote(sock.fileno()))
             # Return it back
             xfrout.socket.has_ipv6 = True
 
@@ -1375,19 +1377,13 @@ class TestUnixSockServer(unittest.TestCase):
         self._remove_file(sock_file)
         self.assertFalse(self.unix._sock_file_in_use(sock_file))
         self._start_unix_sock_server(sock_file)
-
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
         self.assertTrue(self.unix._sock_file_in_use(sock_file))
-        sys.stdout = old_stdout
 
     def test_remove_unused_sock_file_in_use(self):
         sock_file = 'temp.sock.file'
         self._remove_file(sock_file)
         self.assertFalse(self.unix._sock_file_in_use(sock_file))
         self._start_unix_sock_server(sock_file)
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
         try:
             self.unix._remove_unused_sock_file(sock_file)
         except SystemExit:
@@ -1396,8 +1392,6 @@ class TestUnixSockServer(unittest.TestCase):
             # This should never happen
             self.assertTrue(False)
 
-        sys.stdout = old_stdout
-
     def test_remove_unused_sock_file_dir(self):
         import tempfile
         dir_name = tempfile.mkdtemp()
@@ -1411,6 +1405,7 @@ class TestUnixSockServer(unittest.TestCase):
             # This should never happen
             self.assertTrue(False)
 
+        sys.stdout.close()
         sys.stdout = old_stdout
         os.rmdir(dir_name)
 

src/bin/xfrout/xfrout.py.in

@@ -747,12 +747,14 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
         # to care about the SOCK_STREAM parameter at all (which it really is,
         # except for testing)
         if socket.has_ipv6:
-            sock = socket.fromfd(sock_fd, socket.AF_INET6, socket.SOCK_STREAM)
+            sock_domain = socket.AF_INET6
         else:
             # To make it work even on hosts without IPv6 support
             # (Any idea how to simulate this in test?)
-            sock = socket.fromfd(sock_fd, socket.AF_INET, socket.SOCK_STREAM)
-        peer = sock.getpeername()
+            sock_domain = socket.AF_INET
+
+        with socket.fromfd(sock_fd, sock_domain, socket.SOCK_STREAM) as sock:
+            peer = sock.getpeername()
 
         # Identify the correct socket family.  Due to the above "trick",
         # we cannot simply use sock.family.
@@ -761,6 +763,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
             socket.inet_pton(socket.AF_INET6, peer[0])
         except socket.error:
             family = socket.AF_INET
+
         return (family, socket.SOCK_STREAM, peer)
 
     def finish_request(self, sock_fd, request_data):
@@ -802,8 +805,8 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
         is being used by one running xfrout process. If it is,
         return True, or else return False. '''
         try:
-            sock = socket.socket(socket.AF_UNIX)
-            sock.connect(sock_file)
+            with socket.socket(socket.AF_UNIX) as sock:
+                sock.connect(sock_file)
         except socket.error as err:
             return False
         else:

src/bin/zonemgr/tests/zonemgr_test.py

@@ -111,6 +111,7 @@ class TestZonemgrRefresh(unittest.TestCase):
     def tearDown(self):
         if os.path.exists(TEST_SQLITE3_DBFILE):
             os.unlink(TEST_SQLITE3_DBFILE)
+        sys.stderr.close()
         sys.stderr = self.stderr_backup
 
     def test_random_jitter(self):

src/bin/zonemgr/zonemgr.py.in

@@ -428,6 +428,8 @@ class ZonemgrRefresh:
         self._thread.join()
         # Wipe out what we do not need
         self._thread = None
+        self._read_sock.close()
+        self._write_sock.close()
         self._read_sock = None
         self._write_sock = None
 

src/lib/datasrc/Makefile.am

@@ -35,6 +35,7 @@ libdatasrc_la_SOURCES += logger.h logger.cc
 libdatasrc_la_SOURCES += client.h iterator.h
 libdatasrc_la_SOURCES += database.h database.cc
 libdatasrc_la_SOURCES += factory.h factory.cc
+libdatasrc_la_SOURCES += client_list.h client_list.cc
 nodist_libdatasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 libdatasrc_la_LDFLAGS = -no-undefined -version-info 1:0:1
 

src/lib/datasrc/client_list.cc

@@ -0,0 +1,162 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "client_list.h"
+#include "client.h"
+#include "factory.h"
+
+#include <memory>
+#include <boost/foreach.hpp>
+
+using namespace isc::data;
+using namespace std;
+
+namespace isc {
+namespace datasrc {
+
+void
+ConfigurableClientList::configure(const Element& config, bool) {
+    // TODO: Implement the cache
+    // TODO: Implement recycling from the old configuration.
+    size_t i(0); // Outside of the try to be able to access it in the catch
+    try {
+        vector<DataSourceInfo> new_data_sources;
+        for (; i < config.size(); ++i) {
+            // Extract the parameters
+            const ConstElementPtr dconf(config.get(i));
+            const ConstElementPtr typeElem(dconf->get("type"));
+            if (typeElem == ConstElementPtr()) {
+                isc_throw(ConfigurationError, "Missing the type option in "
+                          "data source no " << i);
+            }
+            const string type(typeElem->stringValue());
+            ConstElementPtr paramConf(dconf->get("params"));
+            if (paramConf == ConstElementPtr()) {
+                paramConf.reset(new NullElement());
+            }
+            // TODO: Special-case the master files type.
+            // Ask the factory to create the data source for us
+            const DataSourcePair ds(this->getDataSourceClient(type,
+                                                              paramConf));
+            // And put it into the vector
+            new_data_sources.push_back(DataSourceInfo(ds.first, ds.second));
+        }
+        // If everything is OK up until now, we have the new configuration
+        // ready. So just put it there and let the old one die when we exit
+        // the scope.
+        data_sources_.swap(new_data_sources);
+    } catch (const TypeError& te) {
+        isc_throw(ConfigurationError, "Malformed configuration at data source "
+                  "no. " << i << ": " << te.what());
+    }
+}
+
+ClientList::FindResult
+ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
+                            bool) const
+{
+    // Nothing found yet.
+    //
+    // We have this class as a temporary storage, as the FindResult can't be
+    // assigned.
+    struct MutableResult {
+        MutableResult() :
+            datasrc_client(NULL),
+            matched_labels(0),
+            matched(false)
+        {}
+        DataSourceClient* datasrc_client;
+        ZoneFinderPtr finder;
+        uint8_t matched_labels;
+        bool matched;
+        operator FindResult() const {
+            // Conversion to the right result. If we return this, there was
+            // a partial match at best.
+            return (FindResult(datasrc_client, finder, false));
+        }
+    } candidate;
+
+    BOOST_FOREACH(const DataSourceInfo& info, data_sources_) {
+        // TODO: Once we have support for the caches, consider them too here
+        // somehow. This would probably get replaced by a function, that
+        // checks if there's a cache available, if it is, checks the loaded
+        // zones and zones expected to be in the real data source. If it is
+        // the cached one, provide the cached one. If it is in the external
+        // data source, use the datasource and don't provide the finder yet.
+        const DataSourceClient::FindResult result(
+            info.data_src_client_->findZone(name));
+        switch (result.code) {
+            case result::SUCCESS:
+                // If we found an exact match, we have no hope to getting
+                // a better one. Stop right here.
+
+                // TODO: In case we have only the datasource and not the finder
+                // and the need_updater parameter is true, get the zone there.
+                return (FindResult(info.data_src_client_, result.zone_finder,
+                                   true));
+            case result::PARTIALMATCH:
+                if (!want_exact_match) {
+                    // In case we have a partial match, check if it is better
+                    // than what we have. If so, replace it.
+                    //
+                    // We don't need the labels at the first partial match,
+                    // we have nothing to compare with. So we don't get it
+                    // (as a performance) and hope we will not need it at all.
+                    const uint8_t labels(candidate.matched ?
+                        result.zone_finder->getOrigin().getLabelCount() : 0);
+                    if (candidate.matched && candidate.matched_labels == 0) {
+                        // But if the hope turns out to be false, we need to
+                        // compute it for the first match anyway.
+                        candidate.matched_labels = candidate.finder->
+                            getOrigin().getLabelCount();
+                    }
+                    if (labels > candidate.matched_labels ||
+                        !candidate.matched) {
+                        // This one is strictly better. Replace it.
+                        candidate.datasrc_client = info.data_src_client_;
+                        candidate.finder = result.zone_finder;
+                        candidate.matched_labels = labels;
+                        candidate.matched = true;
+                    }
+                }
+                break;
+            default:
+                // Nothing found, nothing to do.
+                break;
+        }
+    }
+
+    // TODO: In case we have only the datasource and not the finder
+    // and the need_updater parameter is true, get the zone there.
+
+    // Return the partial match we have. In case we didn't want a partial
+    // match, this surely contains the original empty result.
+    return (candidate);
+}
+
+// NOTE: This function is not tested, it would be complicated. However, the
+// purpose of the function is to provide a very thin wrapper to be able to
+// replace the call to DataSourceClientContainer constructor in tests.
+ConfigurableClientList::DataSourcePair
+ConfigurableClientList::getDataSourceClient(const string& type,
+                                            const ConstElementPtr&
+                                            configuration)
+{
+    DataSourceClientContainerPtr
+        container(new DataSourceClientContainer(type, configuration));
+    return (DataSourcePair(&container->getInstance(), container));
+}
+
+}
+}

src/lib/datasrc/client_list.h

@@ -0,0 +1,289 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef DATASRC_CONTAINER_H
+#define DATASRC_CONTAINER_H
+
+#include <dns/name.h>
+#include <cc/data.h>
+#include <exceptions/exceptions.h>
+
+#include <vector>
+#include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
+
+namespace isc {
+namespace datasrc {
+
+class ZoneFinder;
+typedef boost::shared_ptr<ZoneFinder> ZoneFinderPtr;
+class DataSourceClient;
+typedef boost::shared_ptr<DataSourceClient> DataSourceClientPtr;
+class DataSourceClientContainer;
+typedef boost::shared_ptr<DataSourceClientContainer>
+    DataSourceClientContainerPtr;
+
+/// \brief The list of data source clients.
+///
+/// The purpose of this class is to hold several data source clients and search
+/// through them to find one containing a zone best matching a request.
+///
+/// All the data source clients should be for the same class. If you need
+/// to handle multiple classes, you need to create multiple separate lists.
+///
+/// This is an abstract base class. It is not expected we would use multiple
+/// implementation inside the servers (but it is not forbidden either), we
+/// have it to allow easy testing. It is possible to create a mock-up class
+/// instead of creating a full-blown configuration. The real implementation
+/// is the ConfigurableClientList.
+class ClientList : public boost::noncopyable {
+protected:
+    /// \brief Constructor.
+    ///
+    /// It is protected to prevent accidental creation of the abstract base
+    /// class.
+    ClientList() {}
+public:
+    /// \brief Virtual destructor
+    virtual ~ClientList() {}
+    /// \brief Structure holding the (compound) result of find.
+    ///
+    /// As this is read-only structure, we don't bother to create accessors.
+    /// Instead, all the member variables are defined as const and can be
+    /// accessed directly.
+    struct FindResult {
+        /// \brief Constructor.
+        ///
+        /// It simply fills in the member variables according to the
+        /// parameters. See the member descriptions for their meaning.
+        FindResult(DataSourceClient* dsrc_client, const ZoneFinderPtr& finder,
+                   bool exact_match) :
+            dsrc_client_(dsrc_client),
+            finder_(finder),
+            exact_match_(exact_match)
+        {}
+
+        /// \brief Negative answer constructor.
+        ///
+        /// This conscructs a result for negative answer. Both pointers are
+        /// NULL, and exact_match_ is false.
+        FindResult() :
+            dsrc_client_(NULL),
+            exact_match_(false)
+        {}
+
+        /// \brief Comparison operator.
+        ///
+        /// It is needed for tests and it might be of some use elsewhere
+        /// too.
+        bool operator ==(const FindResult& other) const {
+        return (dsrc_client_ == other.dsrc_client_ &&
+                finder_ == other.finder_ &&
+                exact_match_ == other.exact_match_);
+        }
+
+        /// \brief The found data source client.
+        ///
+        /// The client of the data source containing the best matching zone.
+        /// If no such data source exists, this is NULL pointer.
+        ///
+        /// Note that the pointer is valid only as long the ClientList which
+        /// returned the pointer is alive and was not reconfigured. The
+        /// ownership is preserved within the ClientList.
+        DataSourceClient* const dsrc_client_;
+
+        /// \brief The finder for the requested zone.
+        ///
+        /// This is the finder corresponding to the best matching zone.
+        /// This may be NULL even in case the datasrc_ is something
+        /// else, depending on the find options.
+        ///
+        /// \see find
+        const ZoneFinderPtr finder_;
+
+        /// \brief If the result is an exact match.
+        const bool exact_match_;
+    };
+
+    /// \brief Search for a zone through the data sources.
+    ///
+    /// This searches the contained data source clients for a one that best
+    /// matches the zone name.
+    ///
+    /// There are two expected usage scenarios. One is answering queries. In
+    /// this case, the zone finder is needed and the best matching superzone
+    /// of the searched name is needed. Therefore, the call would look like:
+    ///
+    /// \code FindResult result(list->find(queried_name));
+    ///   FindResult result(list->find(queried_name));
+    ///   if (result.datasrc_) {
+    ///       createTheAnswer(result.finder_);
+    ///   } else {
+    ///       createNotAuthAnswer();
+    /// } \endcode
+    ///
+    /// The other scenario is manipulating zone data (XfrOut, XfrIn, DDNS,
+    /// ...). In this case, the finder itself is not so important. However,
+    /// we need an exact match (if we want to manipulate zone data, we must
+    /// know exactly, which zone we are about to manipulate). Then the call
+    ///
+    /// \code FindResult result(list->find(zone_name, true, false));
+    ///   FindResult result(list->find(zone_name, true, false));
+    ///   if (result.datasrc_) {
+    ///       ZoneUpdaterPtr updater(result.datasrc_->getUpdater(zone_name);
+    ///       ...
+    /// } \endcode
+    ///
+    /// \param zone The name of the zone to look for.
+    /// \param want_exact_match If it is true, it returns only exact matches.
+    ///     If the best possible match is partial, a negative result is
+    ///     returned instead. It is possible the caller could check it and
+    ///     act accordingly if the result would be partial match, but with this
+    ///     set to true, the find might be actually faster under some
+    ///     circumstances.
+    /// \param want_finder If this is false, the finder_ member of FindResult
+    ///     might be NULL even if the corresponding data source is found. This
+    ///     is because of performance, in some cases the finder is a side
+    ///     result of the searching algorithm (therefore asking for it again
+    ///     would be a waste), but under other circumstances it is not, so
+    ///     providing it when it is not needed would also be wasteful.
+    ///
+    ///     Other things are never the side effect of searching, therefore the
+    ///     caller can get them explicitly (the updater, journal reader and
+    ///     iterator).
+    /// \return A FindResult describing the data source and zone with the
+    ///     longest match against the zone parameter.
+    virtual FindResult find(const dns::Name& zone,
+                            bool want_exact_match = false,
+                            bool want_finder = true) const = 0;
+};
+
+/// \brief Shared pointer to the list.
+typedef boost::shared_ptr<ClientList> ClientListPtr;
+/// \brief Shared const pointer to the list.
+typedef boost::shared_ptr<const ClientList> ConstClientListPtr;
+
+/// \Concrete implementation of the ClientList, which is constructed based on
+///     configuration.
+///
+/// This is the implementation which is expected to be used in the servers.
+/// However, it is expected most of the code will use it as the ClientList,
+/// only the creation is expected to be direct.
+///
+/// While it is possible to inherit this class, it is not expected to be
+/// inherited except for tests.
+class ConfigurableClientList : public ClientList {
+public:
+    /// \brief Exception thrown when there's an error in configuration.
+    class ConfigurationError : public Exception {
+    public:
+        ConfigurationError(const char* file, size_t line, const char* what) :
+            Exception(file, line, what)
+        {}
+    };
+
+    /// \brief Sets the configuration.
+    ///
+    /// This fills the ClientList with data source clients corresponding to the
+    /// configuration. The data source clients are newly created or recycled
+    /// from previous configuration.
+    ///
+    /// If any error is detected, an exception is thrown and the current
+    /// configuration is preserved.
+    ///
+    /// \param configuration The JSON element describing the configuration to
+    ///     use.
+    /// \param allow_cache If it is true, the 'cache' option of the
+    ///     configuration is used and some zones are cached into an In-Memory
+    ///     data source according to it. If it is false, it is ignored and
+    ///     no In-Memory data sources are created.
+    /// \throw DataSourceError if there's a problem creating a data source
+    ///     client.
+    /// \throw ConfigurationError if the configuration is invalid in some
+    ///     sense.
+    void configure(const data::Element& configuration, bool allow_cache);
+
+    /// \brief Implementation of the ClientList::find.
+    virtual FindResult find(const dns::Name& zone,
+                            bool want_exact_match = false,
+                            bool want_finder = true) const;
+
+    /// \brief This holds one data source client and corresponding information.
+    ///
+    /// \todo The content yet to be defined.
+    struct DataSourceInfo {
+        /// \brief Default constructor.
+        ///
+        /// Don't use directly. It is here so the structure can live in
+        /// a vector.
+        DataSourceInfo() :
+            data_src_client_(NULL)
+        {}
+        DataSourceInfo(DataSourceClient* data_src_client,
+                       const DataSourceClientContainerPtr& container) :
+            data_src_client_(data_src_client),
+            container_(container)
+        {}
+        DataSourceClient* data_src_client_;
+        DataSourceClientContainerPtr container_;
+    };
+
+    /// \brief The collection of data sources.
+    typedef std::vector<DataSourceInfo> DataSources;
+protected:
+    /// \brief The data sources held here.
+    ///
+    /// All our data sources are stored here. It is protected to let the
+    /// tests in. You should consider it private if you ever want to
+    /// derive this class (which is not really recommended anyway).
+    DataSources data_sources_;
+
+    /// \brief Convenience type alias.
+    ///
+    /// \see getDataSource
+    typedef std::pair<DataSourceClient*, DataSourceClientContainerPtr>
+        DataSourcePair;
+
+    /// \brief Create a data source client of given type and configuration.
+    ///
+    /// This is a thin wrapper around the DataSourceClientContainer
+    /// constructor. The function is here to make it possible for tests
+    /// to replace the DataSourceClientContainer with something else.
+    /// Also, derived classes could want to create the data source clients
+    /// in a different way, though inheriting this class is not recommended.
+    ///
+    /// The parameters are the same as of the constructor.
+    /// \return Pair containing both the data source client and the container.
+    ///     The container might be NULL in the derived class, it is
+    ///     only stored so the data source client is properly destroyed when
+    ///     not needed. However, in such case, it is the caller's
+    ///     responsibility to ensure the data source client is deleted when
+    ///     needed.
+    virtual DataSourcePair getDataSourceClient(const std::string& type,
+                                               const data::ConstElementPtr&
+                                               configuration);
+public:
+    /// \brief Access to the data source clients.
+    ///
+    /// It can be used to examine the loaded list of data sources clients
+    /// directly. It is not known if it is of any use other than testing, but
+    /// it might be, so it is just made public (there's no real reason to
+    /// hide it).
+    const DataSources& getDataSources() const { return (data_sources_); }
+};
+
+} // namespace datasrc
+} // namespace isc
+
+#endif // DATASRC_CONTAINER_H

src/lib/datasrc/tests/Makefile.am

@@ -62,6 +62,7 @@ run_unittests_SOURCES += memory_datasrc_unittest.cc
 run_unittests_SOURCES += rbnode_rrset_unittest.cc
 run_unittests_SOURCES += zone_finder_context_unittest.cc
 run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
+run_unittests_SOURCES += client_list_unittest.cc
 
 # We need the actual module implementation in the tests (they are not part
 # of libdatasrc)

src/lib/datasrc/tests/client_list_unittest.cc

@@ -0,0 +1,475 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/client_list.h>
+#include <datasrc/client.h>
+#include <datasrc/data_source.h>
+
+#include <dns/rrclass.h>
+
+#include <gtest/gtest.h>
+
+#include <set>
+
+using namespace isc::datasrc;
+using namespace isc::data;
+using namespace isc::dns;
+using namespace boost;
+using namespace std;
+
+namespace {
+
+// A test data source. It pretends it has some zones.
+class MockDataSourceClient : public DataSourceClient {
+public:
+    class Finder : public ZoneFinder {
+    public:
+        Finder(const Name& origin) :
+            origin_(origin)
+        {}
+        Name getOrigin() const { return (origin_); }
+        // The rest is not to be called, so just have them
+        RRClass getClass() const {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        shared_ptr<Context> find(const Name&, const RRType&,
+                                 const FindOptions)
+        {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        shared_ptr<Context> findAll(const Name&,
+                                    vector<ConstRRsetPtr>&,
+                                    const FindOptions)
+        {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        FindNSEC3Result findNSEC3(const Name&, bool) {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        Name findPreviousName(const Name&) const {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+    private:
+        Name origin_;
+    };
+    // Constructor from a list of zones.
+    MockDataSourceClient(const char* zone_names[]) {
+        for (const char** zone(zone_names); *zone; ++zone) {
+            zones.insert(Name(*zone));
+        }
+    }
+    // Constructor from configuration. The list of zones will be empty, but
+    // it will keep the configuration inside for further inspection.
+    MockDataSourceClient(const string& type,
+                         const ConstElementPtr& configuration) :
+        type_(type),
+        configuration_(configuration)
+    {}
+    virtual FindResult findZone(const Name& name) const {
+        if (zones.empty()) {
+            return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        }
+        set<Name>::const_iterator it(zones.upper_bound(name));
+        if (it == zones.begin()) {
+            return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        }
+        --it;
+        NameComparisonResult compar(it->compare(name));
+        const ZoneFinderPtr finder(new Finder(*it));
+        switch (compar.getRelation()) {
+            case NameComparisonResult::EQUAL:
+                return (FindResult(result::SUCCESS, finder));
+            case NameComparisonResult::SUPERDOMAIN:
+                return (FindResult(result::PARTIALMATCH, finder));
+            default:
+                return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        }
+    }
+    // These methods are not used. They just need to be there to have
+    // complete vtable.
+    virtual ZoneUpdaterPtr getUpdater(const Name&, bool, bool) const {
+        isc_throw(isc::NotImplemented, "Not implemented");
+    }
+    virtual pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+        getJournalReader(const Name&, uint32_t, uint32_t) const
+    {
+        isc_throw(isc::NotImplemented, "Not implemented");
+    }
+    const string type_;
+    const ConstElementPtr configuration_;
+private:
+    set<Name> zones;
+};
+
+
+// The test version is the same as the normal version. We, however, add
+// some methods to dig directly in the internals, for the tests.
+class TestedList : public ConfigurableClientList {
+public:
+    DataSources& getDataSources() { return (data_sources_); }
+    // Overwrite the list's method to get a data source with given type
+    // and configuration. We mock the data source and don't create the
+    // container. This is just to avoid some complexity in the tests.
+    virtual DataSourcePair getDataSourceClient(const string& type,
+                                               const ConstElementPtr&
+                                               configuration)
+    {
+        if (type == "error") {
+            isc_throw(DataSourceError, "The error data source type");
+        }
+        shared_ptr<MockDataSourceClient>
+            ds(new MockDataSourceClient(type, configuration));
+        // Make sure it is deleted when the test list is deleted.
+        to_delete_.push_back(ds);
+        return (DataSourcePair(ds.get(), DataSourceClientContainerPtr()));
+    }
+private:
+    // Hold list of data sources created internally, so they are preserved
+    // until the end of the test and then deleted.
+    vector<shared_ptr<MockDataSourceClient> > to_delete_;
+};
+
+const char* ds_zones[][3] = {
+    {
+        "example.org.",
+        "example.com.",
+        NULL
+    },
+    {
+        "sub.example.org.",
+        NULL, NULL
+    },
+    {
+        NULL, NULL, NULL
+    },
+    {
+        "sub.example.org.",
+        NULL, NULL
+    }
+};
+
+const size_t ds_count = (sizeof(ds_zones) / sizeof(*ds_zones));
+
+class ListTest : public ::testing::Test {
+public:
+    ListTest() :
+        // The empty list corresponds to a list with no elements inside
+        list_(new TestedList()),
+        config_elem_(Element::fromJSON("["
+            "{"
+            "   \"type\": \"test_type\","
+            "   \"cache\": \"off\","
+            "   \"params\": {}"
+            "}]"))
+    {
+        for (size_t i(0); i < ds_count; ++ i) {
+            shared_ptr<MockDataSourceClient>
+                ds(new MockDataSourceClient(ds_zones[i]));
+            ds_.push_back(ds);
+            ds_info_.push_back(ConfigurableClientList::DataSourceInfo(ds.get(),
+                DataSourceClientContainerPtr()));
+        }
+    }
+    // Check the positive result is as we expect it.
+    void positiveResult(const ClientList::FindResult& result,
+                        const shared_ptr<MockDataSourceClient>& dsrc,
+                        const Name& name, bool exact,
+                        const char* test)
+    {
+        SCOPED_TRACE(test);
+        EXPECT_EQ(dsrc.get(), result.dsrc_client_);
+        ASSERT_NE(ZoneFinderPtr(), result.finder_);
+        EXPECT_EQ(name, result.finder_->getOrigin());
+        EXPECT_EQ(exact, result.exact_match_);
+    }
+    // Configure the list with multiple data sources, according to
+    // some configuration. It uses the index as parameter, to be able to
+    // loop through the configurations.
+    void multiConfiguration(size_t index) {
+        list_->getDataSources().clear();
+        switch (index) {
+            case 2:
+                list_->getDataSources().push_back(ds_info_[2]);
+                // The ds_[2] is empty. We just check that it doesn't confuse
+                // us. Fall through to the case 0.
+            case 0:
+                list_->getDataSources().push_back(ds_info_[0]);
+                list_->getDataSources().push_back(ds_info_[1]);
+                break;
+            case 1:
+                // The other order
+                list_->getDataSources().push_back(ds_info_[1]);
+                list_->getDataSources().push_back(ds_info_[0]);
+                break;
+            case 3:
+                list_->getDataSources().push_back(ds_info_[1]);
+                list_->getDataSources().push_back(ds_info_[0]);
+                // It is the same as ds_[1], but we take from the first one.
+                // The first one to match is the correct one.
+                list_->getDataSources().push_back(ds_info_[3]);
+                break;
+            default:
+                FAIL() << "Unknown configuration index " << index;
+        }
+    }
+    void checkDS(size_t index, const string& type, const string& params) const
+    {
+        ASSERT_GT(list_->getDataSources().size(), index);
+        MockDataSourceClient* ds(dynamic_cast<MockDataSourceClient*>(
+            list_->getDataSources()[index].data_src_client_));
+
+        // Comparing with NULL does not work
+        ASSERT_NE(ds, static_cast<const MockDataSourceClient*>(NULL));
+        EXPECT_EQ(type, ds->type_);
+        EXPECT_TRUE(Element::fromJSON(params)->equals(*ds->configuration_));
+    }
+    shared_ptr<TestedList> list_;
+    const ClientList::FindResult negativeResult_;
+    vector<shared_ptr<MockDataSourceClient> > ds_;
+    vector<ConfigurableClientList::DataSourceInfo> ds_info_;
+    const ConstElementPtr config_elem_;
+};
+
+// Test the test itself
+TEST_F(ListTest, selfTest) {
+    EXPECT_EQ(result::SUCCESS, ds_[0]->findZone(Name("example.org")).code);
+    EXPECT_EQ(result::PARTIALMATCH,
+              ds_[0]->findZone(Name("sub.example.org")).code);
+    EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("org")).code);
+    EXPECT_EQ(result::NOTFOUND, ds_[1]->findZone(Name("example.org")).code);
+    EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("aaa")).code);
+    EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("zzz")).code);
+}
+
+// Test the list we create with empty configuration is, in fact, empty
+TEST_F(ListTest, emptyList) {
+    EXPECT_TRUE(list_->getDataSources().empty());
+}
+
+// Check the values returned by a find on an empty list. It should be
+// a negative answer (nothing found) no matter if we want an exact or inexact
+// match.
+TEST_F(ListTest, emptySearch) {
+    // No matter what we try, we don't get an answer.
+
+    // Note: we don't have operator<< for the result class, so we cannot use
+    // EXPECT_EQ.  Same for other similar cases.
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org"), false,
+                                               false));
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org"), false,
+                                               true));
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org"), true,
+                                               false));
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org"), true,
+                                               true));
+}
+
+// Put a single data source inside the list and check it can find an
+// exact match if there's one.
+TEST_F(ListTest, singleDSExactMatch) {
+    list_->getDataSources().push_back(ds_info_[0]);
+    // This zone is not there
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("org."), true));
+    // But this one is, so check it.
+    positiveResult(list_->find(Name("example.org"), true), ds_[0],
+                   Name("example.org"), true, "Exact match");
+    // When asking for a sub zone of a zone there, we get nothing
+    // (we want exact match, this would be partial one)
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("sub.example.org."),
+                                               true));
+}
+
+// When asking for a partial match, we get all that the exact one, but more.
+TEST_F(ListTest, singleDSBestMatch) {
+    list_->getDataSources().push_back(ds_info_[0]);
+    // This zone is not there
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("org.")));
+    // But this one is, so check it.
+    positiveResult(list_->find(Name("example.org")), ds_[0],
+                   Name("example.org"), true, "Exact match");
+    // When asking for a sub zone of a zone there, we get the parent
+    // one.
+    positiveResult(list_->find(Name("sub.example.org.")), ds_[0],
+                   Name("example.org"), false, "Subdomain match");
+}
+
+const char* const test_names[] = {
+    "Sub second",
+    "Sub first",
+    "With empty",
+    "With a duplicity"
+};
+
+TEST_F(ListTest, multiExactMatch) {
+    // Run through all the multi-configurations
+    for (size_t i(0); i < sizeof(test_names) / sizeof(*test_names); ++i) {
+        SCOPED_TRACE(test_names[i]);
+        multiConfiguration(i);
+        // Something that is nowhere there
+        EXPECT_TRUE(negativeResult_ == list_->find(Name("org."), true));
+        // This one is there exactly.
+        positiveResult(list_->find(Name("example.org"), true), ds_[0],
+                       Name("example.org"), true, "Exact match");
+        // This one too, but in a different data source.
+        positiveResult(list_->find(Name("sub.example.org."), true), ds_[1],
+                       Name("sub.example.org"), true, "Subdomain match");
+        // But this one is in neither data source.
+        EXPECT_TRUE(negativeResult_ ==
+                    list_->find(Name("sub.example.com."), true));
+    }
+}
+
+TEST_F(ListTest, multiBestMatch) {
+    // Run through all the multi-configurations
+    for (size_t i(0); i < 4; ++ i) {
+        SCOPED_TRACE(test_names[i]);
+        multiConfiguration(i);
+        // Something that is nowhere there
+        EXPECT_TRUE(negativeResult_ == list_->find(Name("org.")));
+        // This one is there exactly.
+        positiveResult(list_->find(Name("example.org")), ds_[0],
+                       Name("example.org"), true, "Exact match");
+        // This one too, but in a different data source.
+        positiveResult(list_->find(Name("sub.example.org.")), ds_[1],
+                       Name("sub.example.org"), true, "Subdomain match");
+        // But this one is in neither data source. But it is a subdomain
+        // of one of the zones in the first data source.
+        positiveResult(list_->find(Name("sub.example.com.")), ds_[0],
+                       Name("example.com."), false, "Subdomain in com");
+    }
+}
+
+// Check the configuration is empty when the list is empty
+TEST_F(ListTest, configureEmpty) {
+    ConstElementPtr elem(new ListElement);
+    list_->configure(*elem, true);
+    EXPECT_TRUE(list_->getDataSources().empty());
+}
+
+// Check we can get multiple data sources and they are in the right order.
+TEST_F(ListTest, configureMulti) {
+    ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"type1\","
+        "   \"cache\": \"off\","
+        "   \"params\": {}"
+        "},"
+        "{"
+        "   \"type\": \"type2\","
+        "   \"cache\": \"off\","
+        "   \"params\": {}"
+        "}]"
+    ));
+    list_->configure(*elem, true);
+    EXPECT_EQ(2, list_->getDataSources().size());
+    checkDS(0, "type1", "{}");
+    checkDS(1, "type2", "{}");
+}
+
+// Check we can pass whatever we want to the params
+TEST_F(ListTest, configureParams) {
+    const char* params[] = {
+        "true",
+        "false",
+        "null",
+        "\"hello\"",
+        "42",
+        "[]",
+        "{}",
+        NULL
+    };
+    for (const char** param(params); *param; ++param) {
+        SCOPED_TRACE(*param);
+        ConstElementPtr elem(Element::fromJSON(string("["
+            "{"
+            "   \"type\": \"t\","
+            "   \"cache\": \"off\","
+            "   \"params\": ") + *param +
+            "}]"));
+        list_->configure(*elem, true);
+        EXPECT_EQ(1, list_->getDataSources().size());
+        checkDS(0, "t", *param);
+    }
+}
+
+TEST_F(ListTest, wrongConfig) {
+    const char* configs[] = {
+        // A lot of stuff missing from there
+        "[{\"type\": \"test_type\", \"params\": 13}, {}]",
+        // Some bad types completely
+        "{}",
+        "true",
+        "42",
+        "null",
+        "[{\"type\": \"test_type\", \"params\": 13}, true]",
+        "[{\"type\": \"test_type\", \"params\": 13}, []]",
+        "[{\"type\": \"test_type\", \"params\": 13}, 42]",
+        // Bad type of type
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": 42}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": true}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": null}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": []}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": {}}]",
+        // TODO: Once cache is supported, add some invalid cache values
+        NULL
+    };
+    // Put something inside to see it survives the exception
+    list_->configure(*config_elem_, true);
+    checkDS(0, "test_type", "{}");
+    for (const char** config(configs); *config; ++config) {
+        SCOPED_TRACE(*config);
+        ConstElementPtr elem(Element::fromJSON(*config));
+        EXPECT_THROW(list_->configure(*elem, true),
+                     ConfigurableClientList::ConfigurationError);
+        // Still untouched
+        checkDS(0, "test_type", "{}");
+        EXPECT_EQ(1, list_->getDataSources().size());
+    }
+}
+
+// The param thing defaults to null. Cache is not used yet.
+TEST_F(ListTest, defaults) {
+    ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"type1\""
+        "}]"));
+    list_->configure(*elem, true);
+    EXPECT_EQ(1, list_->getDataSources().size());
+    checkDS(0, "type1", "null");
+}
+
+// Check we can call the configure multiple times, to change the configuration
+TEST_F(ListTest, reconfigure) {
+    ConstElementPtr empty(new ListElement);
+    list_->configure(*config_elem_, true);
+    checkDS(0, "test_type", "{}");
+    list_->configure(*empty, true);
+    EXPECT_TRUE(list_->getDataSources().empty());
+    list_->configure(*config_elem_, true);
+    checkDS(0, "test_type", "{}");
+}
+
+// Make sure the data source error exception from the factory is propagated
+TEST_F(ListTest, dataSrcError) {
+    ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"error\""
+        "}]"));
+    list_->configure(*config_elem_, true);
+    checkDS(0, "test_type", "{}");
+    EXPECT_THROW(list_->configure(*elem, true), DataSourceError);
+    checkDS(0, "test_type", "{}");
+}
+
+}

src/lib/dhcp/Makefile.am

@@ -5,6 +5,12 @@ AM_CPPFLAGS += $(BOOST_INCLUDES)
 
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
+# Some versions of GCC warn about some versions of Boost regarding
+# missing initializer for members in its posix_time.
+# https://svn.boost.org/trac/boost/ticket/3477
+# But older GCC compilers don't have the flag.
+AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
+
 CLEANFILES = *.gcno *.gcda
 
 lib_LTLIBRARIES = libdhcp++.la
@@ -31,3 +37,9 @@ libdhcp___la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
 libdhcp___la_LIBADD   = $(top_builddir)/src/lib/asiolink/libasiolink.la
 libdhcp___la_LIBADD  += $(top_builddir)/src/lib/util/libutil.la
 libdhcp___la_LDFLAGS  = -no-undefined -version-info 1:0:0
+
+if USE_CLANGPP
+# Disable unused parameter warning caused by some of the
+# Boost headers when compiling with clang.
+libdhcp___la_CXXFLAGS += -Wno-unused-parameter
+endif

src/lib/dhcp/iface_mgr.cc

@@ -606,6 +606,8 @@ IfaceMgr::send(const Pkt6Ptr& pkt) {
     pktinfo->ipi6_ifindex = pkt->getIndex();
     m.msg_controllen = cmsg->cmsg_len;
 
+    pkt->updateTimestamp();
+
     result = sendmsg(getSocket(*pkt), &m, 0);
     if (result < 0) {
         isc_throw(Unexpected, "Pkt6 send failed: sendmsg() returned " << result);
@@ -665,6 +667,8 @@ IfaceMgr::send(const Pkt4Ptr& pkt)
          << " over socket " << getSocket(*pkt) << " on interface "
          << getIface(pkt->getIface())->getFullName() << endl;
 
+    pkt->updateTimestamp();
+
     int result = sendmsg(getSocket(*pkt), &m, 0);
     if (result < 0) {
         isc_throw(Unexpected, "Pkt4 send failed.");
@@ -755,6 +759,8 @@ IfaceMgr::receive4() {
     // We have all data let's create Pkt4 object.
     Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(buf, result));
 
+    pkt->updateTimestamp();
+
     unsigned int ifindex = iface->getIndex();
 
     IOAddress from(htonl(from_addr.sin_addr.s_addr));
@@ -899,6 +905,8 @@ Pkt6Ptr IfaceMgr::receive6() {
         return (Pkt6Ptr()); // NULL
     }
 
+    pkt->updateTimestamp();
+
     pkt->setLocalAddr(IOAddress::from_bytes(AF_INET6,
                       reinterpret_cast<const uint8_t*>(&to_addr)));
     pkt->setRemoteAddr(IOAddress::from_bytes(AF_INET6,

src/lib/dhcp/option.cc

@@ -270,6 +270,14 @@ void Option::setUint32(uint32_t value) {
   writeUint32(value, &data_[0]);
 }
 
+void Option::setData(const OptionBufferConstIter first,
+                     const OptionBufferConstIter last) {
+    // We will copy entire option buffer, so we have to resize data_.
+    data_.resize(std::distance(first, last));
+    std::copy(first, last, data_.begin());
+}
+
+
 Option::~Option() {
 
 }

src/lib/dhcp/option.h

@@ -244,6 +244,15 @@ public:
     /// @param value value to be set
     void setUint32(uint32_t value);
 
+    /// @brief Sets content of this option from buffer.
+    ///
+    /// Option will be resized to length of buffer.
+    ///
+    /// @param first iterator pointing begining of buffer to copy.
+    /// @param last iterator pointing to end of buffer to copy.
+    void setData(const OptionBufferConstIter first,
+                 const OptionBufferConstIter last);
+
     /// just to force that every option has virtual dtor
     virtual ~Option();
 

src/lib/dhcp/pkt4.cc

@@ -305,6 +305,11 @@ Pkt4::getOption(uint8_t type) {
     return boost::shared_ptr<isc::dhcp::Option>(); // NULL
 }
 
+void
+Pkt4::updateTimestamp() {
+    timestamp_ = boost::posix_time::microsec_clock::universal_time();
+}
+
 } // end of namespace isc::dhcp
 
 } // end of namespace isc

src/lib/dhcp/pkt4.h

@@ -16,8 +16,10 @@
 #define PKT4_H
 
 #include <iostream>
+#include <time.h>
 #include <vector>
 #include <boost/shared_ptr.hpp>
+#include <boost/date_time/posix_time/posix_time.hpp>
 #include "asiolink/io_address.h"
 #include "util/buffer.h"
 #include "dhcp/option.h"
@@ -202,6 +204,11 @@ public:
     void
     setGiaddr(const isc::asiolink::IOAddress& giaddr) { giaddr_ = giaddr; };
 
+    /// @brief Sets transaction-id value
+    ///
+    /// @param transid transaction-id to be set.
+    void setTransid(uint32_t transid) { transid_ = transid; }
+
     /// @brief Returns value of transaction-id field.
     ///
     /// @return transaction-id
@@ -321,6 +328,14 @@ public:
     /// @return interface name
     std::string getIface() const { return iface_; };
 
+    /// @brief Returns packet timestamp.
+    ///
+    /// Returns packet timestamp value updated when
+    /// packet is received or send.
+    ///
+    /// @return packet timestamp.
+    const boost::posix_time::ptime& getTimestamp() const { return timestamp_; }
+
     /// @brief Sets interface name.
     ///
     /// Sets interface name over which packet was received or is
@@ -387,6 +402,14 @@ public:
     /// @return remote port
     uint16_t getRemotePort() { return (remote_port_); }
 
+    /// @brief Update packet timestamp.
+    ///
+    /// Updates packet timestamp. This method is invoked
+    /// by interface manager just before sending or
+    /// just after receiving it.
+    /// @throw isc::Unexpected if timestamp update failed
+    void updateTimestamp();
+
 protected:
 
     /// converts DHCP message type to BOOTP op type
@@ -470,12 +493,26 @@ protected:
     // end of real DHCPv4 fields
 
     /// output buffer (used during message transmission)
+    ///
+    /// @warning This protected member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
     isc::util::OutputBuffer bufferOut_;
 
     /// that's the data of input buffer used in RX packet. Note that
     /// InputBuffer does not store the data itself, but just expects that
     /// data will be valid for the whole life of InputBuffer. Therefore we
     /// need to keep the data around.
+    ///
+    /// @warning This protected member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
     std::vector<uint8_t> data_;
 
     /// message type (e.g. 1=DHCPDISCOVER)
@@ -484,7 +521,17 @@ protected:
     uint8_t msg_type_;
 
     /// collection of options present in this message
+    ///
+    /// @warnig This protected member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
     isc::dhcp::Option::OptionCollection options_;
+
+    /// packet timestamp
+    boost::posix_time::ptime timestamp_;
 }; // Pkt4 class
 
 typedef boost::shared_ptr<Pkt4> Pkt4Ptr;

src/lib/dhcp/pkt6.cc

@@ -202,5 +202,11 @@ void Pkt6::repack() {
     bufferOut_.writeData(&data_[0], data_.size());
 }
 
+void
+Pkt6::updateTimestamp() {
+    timestamp_ = boost::posix_time::microsec_clock::universal_time();
+}
+
+
 } // end of isc::dhcp namespace
 } // end of isc namespace

src/lib/dhcp/pkt6.h

@@ -16,8 +16,10 @@
 #define PKT6_H
 
 #include <iostream>
+#include <time.h>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_array.hpp>
+#include <boost/date_time/posix_time/posix_time.hpp>
 #include "asiolink/io_address.h"
 #include "dhcp/option.h"
 
@@ -129,6 +131,11 @@ public:
     /// @param type message type to be set
     void setType(uint8_t type) { msg_type_=type; };
 
+    /// @brief Sets transaction-id value
+    ///
+    /// @param transid transaction-id to be set.
+    void setTransid(uint32_t transid) { transid_ = transid; }
+
     /// Returns value of transaction-id field
     ///
     /// @return transaction-id
@@ -220,6 +227,14 @@ public:
     /// @return interface name
     std::string getIface() const { return iface_; };
 
+    /// @brief Returns packet timestamp.
+    ///
+    /// Returns packet timestamp value updated when
+    /// packet is received or send.
+    ///
+    /// @return packet timestamp.
+    const boost::posix_time::ptime& getTimestamp() const { return timestamp_; }
+
     /// @brief Sets interface name.
     ///
     /// Sets interface name over which packet was received or is
@@ -231,8 +246,23 @@ public:
     /// TODO Need to implement getOptions() as well
 
     /// collection of options present in this message
+    ///
+    /// @warning This protected member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
     isc::dhcp::Option::OptionCollection options_;
 
+    /// @brief Update packet timestamp.
+    ///
+    /// Updates packet timestamp. This method is invoked
+    /// by interface manager just before sending or
+    /// just after receiving it.
+    /// @throw isc::Unexpected if timestamp update failed
+    void updateTimestamp();
+
 protected:
     /// Builds on wire packet for TCP transmission.
     ///
@@ -278,6 +308,13 @@ protected:
     uint32_t transid_;
 
     /// unparsed data (in received packets)
+    ///
+    /// @warning This protected member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
     OptionBuffer data_;
 
     /// name of the network interface the packet was received/to be sent over
@@ -304,7 +341,17 @@ protected:
     uint16_t remote_port_;
 
     /// output buffer (used during message transmission)
+    ///
+    /// @warning This protected member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
     isc::util::OutputBuffer bufferOut_;
+
+    /// packet timestamp
+    boost::posix_time::ptime timestamp_;
 }; // Pkt6 class
 
 typedef boost::shared_ptr<Pkt6> Pkt6Ptr;

src/lib/dhcp/tests/Makefile.am

@@ -7,6 +7,12 @@ AM_CPPFLAGS += -DINSTALL_PROG=\"$(abs_top_srcdir)/install-sh\"
 
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
+# Some versions of GCC warn about some versions of Boost regarding
+# missing initializer for members in its posix_time.
+# https://svn.boost.org/trac/boost/ticket/3477
+# But older GCC compilers don't have the flag.
+AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
+
 if USE_STATIC_LINK
 AM_LDFLAGS = -static
 endif
@@ -41,8 +47,9 @@ libdhcp___unittests_CXXFLAGS = $(AM_CXXFLAGS)
 
 if USE_CLANGPP
 # This is to workaround unused variables tcout and tcerr in
-# log4cplus's streams.h.
-libdhcp___unittests_CXXFLAGS += -Wno-unused-variable
+# log4cplus's streams.h and unused parameters from some of the
+# Boost headers.
+libdhcp___unittests_CXXFLAGS += -Wno-unused-variable -Wno-unused-parameter
 endif
 libdhcp___unittests_LDADD  = $(GTEST_LDADD)
 libdhcp___unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la

src/lib/dhcp/tests/option_unittest.cc

@@ -485,7 +485,7 @@ TEST_F(OptionTest, setUintX) {
     uint8_t exp2[] = {125, 2, 12345/256, 12345%256};
     EXPECT_TRUE(0 == memcmp(exp2, outBuf_.getData(), 4));
 
-    // verity getUint32
+    // verify getUint32
     outBuf_.clear();
     opt4->setUint32(0x12345678);
     opt4->pack4(outBuf_);
@@ -495,4 +495,31 @@ TEST_F(OptionTest, setUintX) {
     uint8_t exp4[] = {125, 4, 0x12, 0x34, 0x56, 0x78};
     EXPECT_TRUE(0 == memcmp(exp4, outBuf_.getData(), 6));
 }
+
+TEST_F(OptionTest, setData) {
+    // verify data override with new buffer larger than
+    // initial option buffer size
+    OptionPtr opt1(new Option(Option::V4, 125,
+                              buf_.begin(), buf_.begin() + 10));
+    buf_.resize(20, 1);
+    opt1->setData(buf_.begin(), buf_.end());
+    opt1->pack4(outBuf_);
+    ASSERT_EQ(outBuf_.getLength() - opt1->getHeaderLen(), buf_.size());
+    const uint8_t* test_data = static_cast<const uint8_t*>(outBuf_.getData());
+    EXPECT_TRUE(0 == memcmp(&buf_[0], test_data + opt1->getHeaderLen(),
+                            buf_.size()));
+
+    // verify data override with new buffer shorter than
+    // initial option buffer size
+    OptionPtr opt2(new Option(Option::V4, 125,
+                              buf_.begin(), buf_.begin() + 10));
+    outBuf_.clear();
+    buf_.resize(5, 1);
+    opt2->setData(buf_.begin(), buf_.end());
+    opt2->pack4(outBuf_);
+    ASSERT_EQ(outBuf_.getLength() - opt1->getHeaderLen(), buf_.size());
+    test_data = static_cast<const uint8_t*>(outBuf_.getData());
+    EXPECT_TRUE(0 == memcmp(&buf_[0], test_data + opt1->getHeaderLen(),
+                            buf_.size()));
+}
 }

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -31,7 +31,9 @@ using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::util;
-using namespace boost;
+// don't import the entire boost namespace.  It will unexpectedly hide uint8_t
+// for some systems.
+using boost::scoped_ptr;
 
 namespace {
 
@@ -598,4 +600,32 @@ TEST(Pkt4Test, metaFields) {
     delete pkt;
 }
 
+TEST(Pkt4Test, Timestamp) {
+    scoped_ptr<Pkt4> pkt(new Pkt4(DHCPOFFER, 1234));
+
+    // Just after construction timestamp is invalid
+    ASSERT_TRUE(pkt->getTimestamp().is_not_a_date_time());
+
+    // Update packet time.
+    pkt->updateTimestamp();
+
+    // Get updated packet time.
+    boost::posix_time::ptime ts_packet = pkt->getTimestamp();
+
+    // After timestamp is updated it should be date-time.
+    ASSERT_FALSE(ts_packet.is_not_a_date_time());
+
+    // Check current time.
+    boost::posix_time::ptime ts_now =
+        boost::posix_time::microsec_clock::universal_time();
+
+    // Calculate period between packet time and now.
+    boost::posix_time::time_period ts_period(ts_packet, ts_now);
+
+    // Duration should be positive or zero.
+    EXPECT_TRUE(ts_period.length().total_microseconds() >= 0);
+}
+
+
+
 } // end of anonymous namespace

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -16,6 +16,7 @@
 #include <iostream>
 #include <sstream>
 #include <arpa/inet.h>
+#include <boost/date_time/posix_time/posix_time.hpp>
 #include <gtest/gtest.h>
 
 #include <asiolink/io_address.h>
@@ -204,4 +205,30 @@ TEST_F(Pkt6Test, addGetDelOptions) {
     delete parent;
 }
 
+TEST_F(Pkt6Test, Timestamp) {
+    boost::scoped_ptr<Pkt6> pkt(new Pkt6(DHCPV6_SOLICIT, 0x020304));
+
+    // Just after construction timestamp is invalid
+    ASSERT_TRUE(pkt->getTimestamp().is_not_a_date_time());
+
+    // Update packet time.
+    pkt->updateTimestamp();
+
+    // Get updated packet time.
+    boost::posix_time::ptime ts_packet = pkt->getTimestamp();
+
+    // After timestamp is updated it should be date-time.
+    ASSERT_FALSE(ts_packet.is_not_a_date_time());
+
+    // Check current time.
+    boost::posix_time::ptime ts_now =
+        boost::posix_time::microsec_clock::universal_time();
+
+    // Calculate period between packet time and now.
+    boost::posix_time::time_period ts_period(ts_packet, ts_now);
+
+    // Duration should be positive or zero.
+    EXPECT_TRUE(ts_period.length().total_microseconds() >= 0);
+}
+
 }

src/lib/dns/python/tests/testutil.py

@@ -28,14 +28,14 @@ def read_wire_data(filename):
     data = bytes()
     for path in testdata_path.split(":"):
         try:
-            file = open(path + os.sep + filename, "r")
-            for line in file:
-                line = line.strip()
-                if line == "" or line.startswith("#"):
-                    pass
-                else:
-                    cur_data = bytes.fromhex(line)
-                    data += cur_data
+            with open(path + os.sep + filename, "r") as f:
+                for line in f:
+                    line = line.strip()
+                    if line == "" or line.startswith("#"):
+                        pass
+                    else:
+                        cur_data = bytes.fromhex(line)
+                        data += cur_data
 
             return data
         except IOError:

src/lib/log/tests/message_dictionary_unittest.cc

@@ -28,16 +28,17 @@ using namespace std;
 // global dictionary is loaded, the former should be marked as a duplicate
 // and the latter should be present.
 
-static const char* values[] = {
-    "LOG_DUPLICATE_NAMESPACE", "duplicate $NAMESPACE directive found",
+namespace {
+const char* values[] = {
+    // This message for DUPLICATE_NAMESPACE must be copied from
+    // ../log_messages.mes; otherwise logger check might fail.
+    "LOG_DUPLICATE_NAMESPACE", "line %1: duplicate $NAMESPACE directive found",
     "NEWSYM", "new symbol added",
     NULL
 };
 
 MessageInitializer init(values);
-
-
-
+}
 
 class MessageDictionaryTest : public ::testing::Test {
 protected:

src/lib/python/isc/bind10/tests/sockcreator_test.py

@@ -303,12 +303,16 @@ class WrapTests(unittest.TestCase):
 
         # Transfer the descriptor
         send_fd(t1.fileno(), p1.fileno())
-        p1 = socket.fromfd(t2.read_fd(), socket.AF_UNIX, socket.SOCK_STREAM)
+        p1.close()
 
-        # Now, pass some data trough the socket
-        p1.send(b'A')
-        data = p2.recv(1)
-        self.assertEqual(b'A', data)
+        with socket.fromfd(t2.read_fd(), socket.AF_UNIX,
+                           socket.SOCK_STREAM) as p1:
+            # Now, pass some data trough the socket
+            p1.send(b'A')
+            data = p2.recv(1)
+            self.assertEqual(b'A', data)
+
+        p2.close()
 
         # Test the wrapping didn't hurt the socket's usual methods
         t1.send(b'B')
@@ -318,6 +322,9 @@ class WrapTests(unittest.TestCase):
         data = t1.recv(1)
         self.assertEqual(b'C', data)
 
+        t1.close()
+        t2.close()
+
 if __name__ == '__main__':
     isc.log.init("bind10") # FIXME Should this be needed?
     isc.log.resetUnitTestRootLogger()

src/lib/python/isc/config/tests/module_spec_test.py

@@ -46,8 +46,8 @@ class TestModuleSpec(unittest.TestCase):
         self.spec1(dd)
 
     def test_open_file_obj(self):
-        file1 = open(self.spec_file("spec1.spec"))
-        dd = isc.config.module_spec_from_file(file1)
+        with open(self.spec_file("spec1.spec")) as file1:
+            dd = isc.config.module_spec_from_file(file1)
         self.spec1(dd)
 
     def test_open_bad_file_obj(self):
@@ -89,8 +89,8 @@ class TestModuleSpec(unittest.TestCase):
 
     def validate_data(self, specfile_name, datafile_name):
         dd = self.read_spec_file(specfile_name);
-        data_file = open(self.spec_file(datafile_name))
-        data_str = data_file.read()
+        with open(self.spec_file(datafile_name)) as data_file:
+            data_str = data_file.read()
         data = isc.cc.data.parse_value_str(data_str)
         return dd.validate_config(True, data)
         
@@ -109,8 +109,8 @@ class TestModuleSpec(unittest.TestCase):
 
     def validate_command_params(self, specfile_name, datafile_name, cmd_name):
         dd = self.read_spec_file(specfile_name);
-        data_file = open(self.spec_file(datafile_name))
-        data_str = data_file.read()
+        with open(self.spec_file(datafile_name)) as data_file:
+            data_str = data_file.read()
         params = isc.cc.data.parse_value_str(data_str)
         return dd.validate_command(cmd_name, params)
 
@@ -131,8 +131,8 @@ class TestModuleSpec(unittest.TestCase):
     def test_statistics_validation(self):
         def _validate_stat(specfile_name, datafile_name):
             dd = self.read_spec_file(specfile_name);
-            data_file = open(self.spec_file(datafile_name))
-            data_str = data_file.read()
+            with open(self.spec_file(datafile_name)) as data_file:
+                data_str = data_file.read()
             data = isc.cc.data.parse_value_str(data_str)
             return dd.validate_statistics(True, data, [])
         self.assertFalse(self.read_spec_file("spec1.spec").validate_statistics(True, None, None));

src/lib/python/isc/ddns/session.py

@@ -242,12 +242,17 @@ class UpdateSession:
         '''
         try:
             self._get_update_zone()
+            # Contrary to what RFC2136 specifies, we do ACL checks before
+            # prerequisites. It's now generally considered to be a bad
+            # idea, and actually does harm such as information
+            # leak. It should make more sense to prevent any security issues
+            # by performing ACL check as early as possible.
+            self.__check_update_acl(self.__zname, self.__zclass)
             self._create_diff()
             prereq_result = self.__check_prerequisites()
             if prereq_result != Rcode.NOERROR():
                 self.__make_response(prereq_result)
                 return UPDATE_ERROR, self.__zname, self.__zclass
-            self.__check_update_acl(self.__zname, self.__zclass)
             update_result = self.__do_update()
             if update_result != Rcode.NOERROR():
                 self.__make_response(update_result)
@@ -688,8 +693,9 @@ class UpdateSession:
            Special cases: if the delete statement is for the
            zone's apex, and the type is either SOA or NS, it
            is ignored.'''
-        result, to_delete, _ = self.__diff.find(rrset.get_name(),
-                                                rrset.get_type())
+        # find the rrset with local updates
+        result, to_delete, _ = self.__diff.find_updated(rrset.get_name(),
+                                                        rrset.get_type())
         if result == ZoneFinder.SUCCESS:
             if to_delete.get_name() == self.__zname and\
                (to_delete.get_type() == RRType.SOA() or\
@@ -705,14 +711,10 @@ class UpdateSession:
            may never be removed (and any action that would do so
            should be ignored).
         '''
-        # NOTE: This method is currently bad: it WILL delete all
-        # NS rrsets in a number of cases.
-        # We need an extension to our diff.py to handle this correctly
-        # (see ticket #2016)
-        # The related test is currently disabled. When this is fixed,
-        # enable that test again.
-        result, orig_rrset, _ = self.__diff.find(rrset.get_name(),
-                                                 rrset.get_type())
+        # Find the current NS rrset, including local additions and deletions
+        result, orig_rrset, _ = self.__diff.find_updated(rrset.get_name(),
+                                                         rrset.get_type())
+
         # Even a real rrset comparison wouldn't help here...
         # The goal is to make sure that after deletion of the
         # given rrset, at least 1 NS record is left (at the apex).
@@ -742,7 +744,8 @@ class UpdateSession:
            Special case: if the name is the zone's apex, SOA and
            NS records are kept.
         '''
-        result, rrsets, flags = self.__diff.find_all(rrset.get_name())
+        # Find everything with the name, including local additions
+        result, rrsets, flags = self.__diff.find_all_updated(rrset.get_name())
         if result == ZoneFinder.SUCCESS and\
            (flags & ZoneFinder.RESULT_WILDCARD == 0):
             for to_delete in rrsets:

src/lib/python/isc/ddns/tests/session_tests.py

@@ -18,7 +18,7 @@ import shutil
 import isc.log
 import unittest
 from isc.dns import *
-from isc.datasrc import DataSourceClient
+from isc.datasrc import DataSourceClient, ZoneFinder
 from isc.ddns.session import *
 from isc.ddns.zone_config import *
 
@@ -200,7 +200,7 @@ class SessionTestBase(unittest.TestCase):
         self._acl_map = {(TEST_ZONE_NAME, TEST_RRCLASS):
                              REQUEST_LOADER.load([{"action": "ACCEPT"}])}
         self._session = UpdateSession(self._update_msg, TEST_CLIENT4,
-                                      ZoneConfig([], TEST_RRCLASS,
+                                      ZoneConfig(set(), TEST_RRCLASS,
                                                  self._datasrc_client,
                                                  self._acl_map))
         self._session._get_update_zone()
@@ -327,7 +327,7 @@ class SessionTest(SessionTestBase):
         msg = create_update_msg(zones=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
                                                 RRType.SOA())])
         session = UpdateSession(msg, TEST_CLIENT4,
-                                ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+                                ZoneConfig({(TEST_ZONE_NAME, TEST_RRCLASS)},
                                            TEST_RRCLASS, self._datasrc_client))
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
         self.check_response(session.get_message(), Rcode.NOTIMP())
@@ -336,7 +336,7 @@ class SessionTest(SessionTestBase):
         '''Common test sequence for the 'notauth' test'''
         msg = create_update_msg(zones=[Question(zname, zclass, RRType.SOA())])
         session = UpdateSession(msg, TEST_CLIENT4,
-                                ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+                                ZoneConfig({(TEST_ZONE_NAME, TEST_RRCLASS)},
                                            TEST_RRCLASS, self._datasrc_client))
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
         self.check_response(session.get_message(), Rcode.NOTAUTH())
@@ -360,7 +360,7 @@ class SessionTest(SessionTestBase):
         msg = create_update_msg(zones=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
                                                 RRType.SOA())])
         session = UpdateSession(msg, TEST_CLIENT4,
-                                ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+                                ZoneConfig({(TEST_ZONE_NAME, TEST_RRCLASS)},
                                            TEST_RRCLASS,
                                            BadDataSourceClient()))
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
@@ -617,7 +617,7 @@ class SessionTest(SessionTestBase):
            from 'prerequisites'. Then checks if __check_prerequisites()
            returns the Rcode specified in 'expected'.'''
         msg = create_update_msg([TEST_ZONE_RECORD], prerequisites)
-        zconfig = ZoneConfig([], TEST_RRCLASS, self._datasrc_client,
+        zconfig = ZoneConfig(set(), TEST_RRCLASS, self._datasrc_client,
                              self._acl_map)
         session = UpdateSession(msg, TEST_CLIENT4, zconfig)
         session._get_update_zone()
@@ -643,7 +643,7 @@ class SessionTest(SessionTestBase):
            from 'updates'. Then checks if __do_prescan()
            returns the Rcode specified in 'expected'.'''
         msg = create_update_msg([TEST_ZONE_RECORD], [], updates)
-        zconfig = ZoneConfig([], TEST_RRCLASS, self._datasrc_client,
+        zconfig = ZoneConfig(set(), TEST_RRCLASS, self._datasrc_client,
                              self._acl_map)
         session = UpdateSession(msg, TEST_CLIENT4, zconfig)
         session._get_update_zone()
@@ -657,13 +657,13 @@ class SessionTest(SessionTestBase):
         self.assertEqual(str(expected_soa),
                          str(session._UpdateSession__added_soa))
 
-    def check_full_handle_result(self, expected, updates):
+    def check_full_handle_result(self, expected, updates, prerequisites=[]):
         '''Helper method for checking the result of a full handle;
            creates an update session, and fills it with the list of rrsets
            from 'updates'. Then checks if __handle()
            results in a response with rcode 'expected'.'''
-        msg = create_update_msg([TEST_ZONE_RECORD], [], updates)
-        zconfig = ZoneConfig([], TEST_RRCLASS, self._datasrc_client,
+        msg = create_update_msg([TEST_ZONE_RECORD], prerequisites, updates)
+        zconfig = ZoneConfig(set(), TEST_RRCLASS, self._datasrc_client,
                              self._acl_map)
         session = UpdateSession(msg, TEST_CLIENT4, zconfig)
 
@@ -677,7 +677,6 @@ class SessionTest(SessionTestBase):
         else:
             self.assertEqual(UPDATE_ERROR, result)
 
-
     def test_check_prerequisites(self):
         # This test checks if the actual prerequisite-type-specific
         # methods are called.
@@ -853,7 +852,6 @@ class SessionTest(SessionTestBase):
                                               "1233 3600 1800 2419200 7200" ])
         self.rrset_update_soa_del = rrset_update_soa_del
 
-
         rrset_update_soa2 = create_rrset("example.org", TEST_RRCLASS,
                                          RRType.SOA(), 3600,
                                          [ "ns1.example.org. " +
@@ -904,6 +902,21 @@ class SessionTest(SessionTestBase):
                                 [ b'\x00\x0a\x04mail\x07example\x03org\x00' ])
         self.rrset_update_del_rrset_mx = rrset_update_del_rrset_mx
 
+    def test_acl_before_prereq(self):
+        name_in_use_no = create_rrset("foo.example.org", RRClass.ANY(),
+                                      RRType.ANY(), 0)
+
+        # Test a prerequisite that would fail
+        self.check_full_handle_result(Rcode.NXDOMAIN(), [], [ name_in_use_no ])
+
+        # Change ACL so that it would be denied
+        self._acl_map = {(TEST_ZONE_NAME, TEST_RRCLASS):
+                             REQUEST_LOADER.load([{"action": "REJECT"}])}
+
+        # The prerequisite should now not be reached; it should fail on the
+        # ACL
+        self.check_full_handle_result(Rcode.REFUSED(), [], [ name_in_use_no ])
+
     def test_prescan(self):
         '''Test whether the prescan succeeds on data that is ok, and whether
            if notices the SOA if present'''
@@ -974,7 +987,6 @@ class SessionTest(SessionTestBase):
         rrset = create_rrset("different.zone", RRClass.ANY(), RRType.TXT(), 0)
         self.check_prescan_result(Rcode.NOTZONE(), [ rrset ])
 
-
         # forbidden type, zone class
         rrset = create_rrset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.ANY(), 0,
                              [ b'\x00' ])
@@ -1207,6 +1219,9 @@ class SessionTest(SessionTestBase):
                                  rrset2)
 
     def test_update_delete_name(self):
+        '''
+        Tests whether deletion of every RR for a name works
+        '''
         self.__initialize_update_rrsets()
 
         # First check it is there
@@ -1268,7 +1283,7 @@ class SessionTest(SessionTestBase):
                                  isc.dns.Name("example.org"),
                                  RRType.MX())
 
-        # Check that we cannot delete the SOA record by direction deletion
+        # Check that we cannot delete the SOA record by direct deletion
         # both by name+type and by full rrset
         self.check_full_handle_result(Rcode.NOERROR(),
                                       [ self.rrset_update_del_soa_apex,
@@ -1304,7 +1319,7 @@ class SessionTest(SessionTestBase):
                                  RRType.NS(),
                                  orig_ns_rrset)
 
-    def DISABLED_test_update_apex_special_case_ns_rrset(self):
+    def test_update_apex_special_case_ns_rrset(self):
         # If we delete the NS at the apex specifically, it should still
         # keep one record
         self.__initialize_update_rrsets()
@@ -1319,6 +1334,21 @@ class SessionTest(SessionTestBase):
                                  RRType.NS(),
                                  short_ns_rrset)
 
+    def test_update_apex_special_case_ns_rrset2(self):
+        # If we add new NS records, then delete all existing ones, it
+        # should not keep any
+        self.__initialize_update_rrsets()
+        new_ns = create_rrset("example.org", TEST_RRCLASS, RRType.NS(), 3600,
+                              [ "newns1.example.org", "newns2.example.org" ])
+
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ new_ns,
+                                        self.rrset_update_del_rrset_ns ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("example.org"),
+                                 RRType.NS(),
+                                 new_ns)
+
     def test_update_delete_normal_rrset_at_apex(self):
         '''
         Tests a number of 'normal rrset' deletes at the apex
@@ -1335,6 +1365,64 @@ class SessionTest(SessionTestBase):
                                  isc.dns.Name("example.org"),
                                  RRType.MX())
 
+    def test_update_add_then_delete_rrset(self):
+        # If we add data, then delete the whole rrset, added data should
+        # be gone as well
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ self.rrset_update_a,
+                                        self.rrset_update_del_rrset ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+
+    def test_update_add_then_delete_name(self):
+        # If we add data, then delete the entire name, added data should
+        # be gone as well
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ self.rrset_update_a,
+                                        self.rrset_update_del_name ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.NXDOMAIN,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+
+    def test_update_delete_then_add_rrset(self):
+        # If we delete an entire rrset, then add something there again,
+        # the addition should be done
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ self.rrset_update_del_rrset,
+                                        self.rrset_update_a ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.rrset_update_a)
+
+    def test_update_delete_then_add_rrset(self):
+        # If we delete an entire name, then add something there again,
+        # the addition should be done
+        self.__initialize_update_rrsets()
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A())
+        self.check_full_handle_result(Rcode.NOERROR(),
+                                      [ self.rrset_update_del_name,
+                                        self.rrset_update_a ])
+        self.__check_inzone_data(isc.datasrc.ZoneFinder.SUCCESS,
+                                 isc.dns.Name("www.example.org"),
+                                 RRType.A(),
+                                 self.rrset_update_a)
+
     def test_update_cname_special_cases(self):
         self.__initialize_update_rrsets()
 
@@ -1406,7 +1494,7 @@ class SessionACLTest(SessionTestBase):
         '''
         # create a separate session, with default (empty) ACL map.
         session = UpdateSession(self._update_msg,
-                                TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
+                                TEST_CLIENT4, ZoneConfig(set(), TEST_RRCLASS,
                                                          self._datasrc_client))
         # then the request should be rejected.
         self.assertEqual((UPDATE_ERROR, None, None), session.handle())
@@ -1435,7 +1523,7 @@ class SessionACLTest(SessionTestBase):
         # If the message doesn't contain TSIG, it doesn't match the ACCEPT
         # ACL entry, and the request should be rejected.
         session = UpdateSession(self._update_msg,
-                                TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
+                                TEST_CLIENT4, ZoneConfig(set(), TEST_RRCLASS,
                                                          self._datasrc_client,
                                                          acl_map))
         self.assertEqual((UPDATE_ERROR, None, None), session.handle())
@@ -1444,7 +1532,7 @@ class SessionACLTest(SessionTestBase):
         # If the message contains TSIG, it should match the ACCEPT
         # ACL entry, and the request should be granted.
         session = UpdateSession(create_update_msg(tsig_key=TEST_TSIG_KEY),
-                                TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
+                                TEST_CLIENT4, ZoneConfig(set(), TEST_RRCLASS,
                                                          self._datasrc_client,
                                                          acl_map))
         self.assertEqual((UPDATE_SUCCESS, TEST_ZONE_NAME, TEST_RRCLASS),

src/lib/python/isc/ddns/tests/zone_config_tests.py

@@ -55,7 +55,7 @@ class ZoneConfigTest(unittest.TestCase):
     '''Some basic tests for the ZoneConfig class.'''
     def setUp(self):
         self.__datasrc_client = FakeDataSourceClient()
-        self.zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)],
+        self.zconfig = ZoneConfig({(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)},
                                   TEST_RRCLASS, self.__datasrc_client)
 
     def test_find_zone(self):
@@ -87,34 +87,27 @@ class ZoneConfigTest(unittest.TestCase):
                                                  TEST_RRCLASS)))
         # zone class doesn't match (but zone name matches)
         self.__datasrc_client.set_find_result(DataSourceClient.SUCCESS)
-        zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)],
+        zconfig = ZoneConfig({(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)},
                              RRClass.CH(), self.__datasrc_client)
         self.assertEqual((ZONE_NOTFOUND, None),
                          (zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS)))
         # similar to the previous case, but also in the secondary list
-        zconfig = ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+        zconfig = ZoneConfig({(TEST_ZONE_NAME, TEST_RRCLASS)},
                              RRClass.CH(), self.__datasrc_client)
         self.assertEqual((ZONE_NOTFOUND, None),
                          (zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS)))
 
         # check some basic tests varying the secondary list.
         # empty secondary list doesn't cause any disruption.
-        zconfig = ZoneConfig([], TEST_RRCLASS, self.__datasrc_client)
+        zconfig = ZoneConfig(set(), TEST_RRCLASS, self.__datasrc_client)
         self.assertEqual((ZONE_PRIMARY, self.__datasrc_client),
                          self.zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS))
-        # adding some mulitle tuples, including subdomainof the test zone name,
-        # and the same zone name but a different class
-        zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS),
+        # adding some mulitle tuples, including subdomain of the test zone
+        # name, and the same zone name but a different class
+        zconfig = ZoneConfig({(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS),
                               (Name('example'), TEST_RRCLASS),
                               (Name('sub.example.org'), TEST_RRCLASS),
-                              (TEST_ZONE_NAME, RRClass.CH())],
-                             TEST_RRCLASS, self.__datasrc_client)
-        self.assertEqual((ZONE_PRIMARY, self.__datasrc_client),
-                         self.zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS))
-        # secondary zone list has a duplicate entry, which is just
-        # (effecitivey) ignored
-        zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS),
-                              (TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)],
+                              (TEST_ZONE_NAME, RRClass.CH())},
                              TEST_RRCLASS, self.__datasrc_client)
         self.assertEqual((ZONE_PRIMARY, self.__datasrc_client),
                          self.zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS))
@@ -122,7 +115,7 @@ class ZoneConfigTest(unittest.TestCase):
 class ACLConfigTest(unittest.TestCase):
     def setUp(self):
         self.__datasrc_client = FakeDataSourceClient()
-        self.__zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)],
+        self.__zconfig = ZoneConfig({(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)},
                                     TEST_RRCLASS, self.__datasrc_client)
 
     def test_get_update_acl(self):

src/lib/python/isc/ddns/zone_config.py

@@ -22,6 +22,9 @@ ZONE_NOTFOUND = -1              # Zone isn't found in find_zone()
 ZONE_PRIMARY = 0                # Primary zone
 ZONE_SECONDARY = 1              # Secondary zone
 
+# The default ACL if unspecifed on construction of ZoneConfig.
+DEFAULT_ACL = REQUEST_LOADER.load([{"action": "REJECT"}])
+
 class ZoneConfig:
     '''A temporary helper class to encapsulate zone related configuration.
 
@@ -38,7 +41,7 @@ class ZoneConfig:
         '''Constructor.
 
         Parameters:
-        - secondaries: a list of 2-element tuples.  Each element is a pair
+        - secondaries: a set of 2-element tuples.  Each element is a pair
           of isc.dns.Name and isc.dns.RRClass, and identifies a single
           secondary zone.
         - datasrc_class: isc.dns.RRClass object.  Specifies the RR class
@@ -53,12 +56,10 @@ class ZoneConfig:
           ACL will be applied to all zones, which is to reject any requests.
 
         '''
-        self.__secondaries = set()
-        for (zname, zclass) in secondaries:
-            self.__secondaries.add((zname, zclass))
+        self.__secondaries = secondaries
         self.__datasrc_class = datasrc_class
         self.__datasrc_client = datasrc_client
-        self.__default_acl = REQUEST_LOADER.load([{"action": "REJECT"}])
+        self.__default_acl = DEFAULT_ACL
         self.__acl_map = acl_map
 
     def find_zone(self, zone_name, zone_class):

src/lib/python/isc/util/cio/tests/socketsession_test.py

@@ -22,6 +22,8 @@ TESTDATA_OBJDIR = os.getenv("TESTDATAOBJDIR")
 TEST_UNIX_FILE = TESTDATA_OBJDIR + '/ssessiontest.unix'
 TEST_DATA = b'BIND10 test'
 TEST_PORT = 53535
+TEST_PORT2 = 53536
+TEST_PORT3 = 53537
 
 class TestForwarder(unittest.TestCase):
     '''In general, this is a straightforward port of the C++ counterpart.
@@ -31,12 +33,15 @@ class TestForwarder(unittest.TestCase):
     '''
 
     def setUp(self):
+        self.listen_sock = None
         self.forwarder = SocketSessionForwarder(TEST_UNIX_FILE)
         if os.path.exists(TEST_UNIX_FILE):
             os.unlink(TEST_UNIX_FILE)
         self.large_text = b'a' * 65535
 
     def tearDown(self):
+        if self.listen_sock is not None:
+            self.listen_sock.close()
         if os.path.exists(TEST_UNIX_FILE):
             os.unlink(TEST_UNIX_FILE)
 
@@ -123,64 +128,70 @@ class TestForwarder(unittest.TestCase):
 
     def check_push_and_pop(self, family, type, protocol, local, remote,
                            data, new_connection):
-        sock = self.create_socket(family, type, protocol, local, True)
-        fwd_fd = sock.fileno()
-        if protocol == IPPROTO_TCP:
-            client_addr = ('::1', 0, 0, 0) if family == AF_INET6 \
-                else ('127.0.0.1', 0)
-            client_sock = self.create_socket(family, type, protocol,
-                                             client_addr, False)
-            client_sock.setblocking(False)
-            try:
-                client_sock.connect(local)
-            except socket.error:
-                pass
-            server_sock, _ = sock.accept()
-            fwd_fd = server_sock.fileno()
-
-        # If a new connection is required, start the "server", have the
-        # internal forwarder connect to it, and then internally accept it.
-        if new_connection:
-            self.start_listen()
-            self.forwarder.connect_to_receiver()
-            self.accept_sock = self.accept_forwarder()
-
-        # Then push one socket session via the forwarder.
-        self.forwarder.push(fwd_fd, family, type, protocol, local, remote,
-                            data)
-
-        # Pop the socket session we just pushed from a local receiver, and
-        # check the content.
-        receiver = SocketSessionReceiver(self.accept_sock)
-        signal.alarm(1)
-        sock_session = receiver.pop()
-        signal.alarm(0)
-        passed_sock = sock_session[0]
-        self.assertNotEqual(fwd_fd, passed_sock.fileno())
-        self.assertEqual(family, passed_sock.family)
-        self.assertEqual(type, passed_sock.type)
-        self.assertEqual(protocol, passed_sock.proto)
-        self.assertEqual(local, sock_session[1])
-        self.assertEqual(remote, sock_session[2])
-        self.assertEqual(data, sock_session[3])
-
-        # Check if the passed FD is usable by sending some data from it.
-        passed_sock.setblocking(True)
-        if protocol == IPPROTO_UDP:
-            self.assertEqual(len(TEST_DATA), passed_sock.sendto(TEST_DATA,
-                                                                local))
-            sock.settimeout(10)
-            self.assertEqual(TEST_DATA, sock.recvfrom(len(TEST_DATA))[0])
-        else:
-            server_sock.close()
-            self.assertEqual(len(TEST_DATA), passed_sock.send(TEST_DATA))
-            client_sock.setblocking(True)
-            client_sock.settimeout(10)
-            self.assertEqual(TEST_DATA, client_sock.recv(len(TEST_DATA)))
+        with self.create_socket(family, type, protocol, local, True) as sock:
+            fwd_fd = sock.fileno()
+            if protocol == IPPROTO_TCP:
+                client_addr = ('::1', 0, 0, 0) if family == AF_INET6 \
+                    else ('127.0.0.1', 0)
+                client_sock = self.create_socket(family, type, protocol,
+                                                 client_addr, False)
+                client_sock.setblocking(False)
+                try:
+                    client_sock.connect(local)
+                except socket.error:
+                    pass
+                server_sock, _ = sock.accept()
+                fwd_fd = server_sock.fileno()
+
+            # If a new connection is required, start the "server", have the
+            # internal forwarder connect to it, and then internally accept it.
+            if new_connection:
+                self.start_listen()
+                self.forwarder.connect_to_receiver()
+                self.accept_sock = self.accept_forwarder()
+
+            # Then push one socket session via the forwarder.
+            self.forwarder.push(fwd_fd, family, type, protocol, local, remote,
+                                data)
+
+            # Pop the socket session we just pushed from a local receiver, and
+            # check the content.
+            receiver = SocketSessionReceiver(self.accept_sock)
+            signal.alarm(1)
+            sock_session = receiver.pop()
+            signal.alarm(0)
+            passed_sock = sock_session[0]
+            self.assertNotEqual(fwd_fd, passed_sock.fileno())
+            self.assertEqual(family, passed_sock.family)
+            self.assertEqual(type, passed_sock.type)
+            self.assertEqual(protocol, passed_sock.proto)
+            self.assertEqual(local, sock_session[1])
+            self.assertEqual(remote, sock_session[2])
+            self.assertEqual(data, sock_session[3])
+
+            # Check if the passed FD is usable by sending some data from it.
+            passed_sock.setblocking(True)
+            if protocol == IPPROTO_UDP:
+                self.assertEqual(len(TEST_DATA), passed_sock.sendto(TEST_DATA,
+                                                                    local))
+                sock.settimeout(10)
+                self.assertEqual(TEST_DATA, sock.recvfrom(len(TEST_DATA))[0])
+            else:
+                self.assertEqual(len(TEST_DATA), passed_sock.send(TEST_DATA))
+                client_sock.setblocking(True)
+                client_sock.settimeout(10)
+                self.assertEqual(TEST_DATA, client_sock.recv(len(TEST_DATA)))
+                server_sock.close()
+                client_sock.close()
+
+            passed_sock.close()
 
     def test_push_and_pop(self):
-        # This is a straightforward port of C++ pushAndPop test.
+        # This is a straightforward port of C++ pushAndPop test.  See the
+        # C++ version why we use multiple ports for "local".
         local6 = ('::1', TEST_PORT, 0, 0)
+        local6_alt = ('::1', TEST_PORT2, 0, 0)
+        local6_alt2 = ('::1', TEST_PORT3, 0, 0)
         remote6 = ('2001:db8::1', 5300, 0, 0)
         self.check_push_and_pop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP,
                                 local6, remote6, TEST_DATA, True)
@@ -188,6 +199,7 @@ class TestForwarder(unittest.TestCase):
                                 local6, remote6, TEST_DATA, False)
 
         local4 = ('127.0.0.1', TEST_PORT)
+        local4_alt = ('127.0.0.1', TEST_PORT2)
         remote4 = ('192.0.2.2', 5300)
         self.check_push_and_pop(AF_INET, SOCK_DGRAM, IPPROTO_UDP,
                                 local4, remote4, TEST_DATA, False)
@@ -195,11 +207,11 @@ class TestForwarder(unittest.TestCase):
                                 local4, remote4, TEST_DATA, False)
 
         self.check_push_and_pop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP,
-                                local6, remote6, self.large_text, False)
+                                local6_alt, remote6, self.large_text, False)
         self.check_push_and_pop(AF_INET6, SOCK_STREAM, IPPROTO_TCP,
                                 local6, remote6, self.large_text, False)
         self.check_push_and_pop(AF_INET, SOCK_DGRAM, IPPROTO_UDP,
-                                local4, remote4, self.large_text, False)
+                                local4_alt, remote4, self.large_text, False)
         self.check_push_and_pop(AF_INET, SOCK_STREAM, IPPROTO_TCP,
                                 local4, remote4, self.large_text, False)
 
@@ -207,7 +219,7 @@ class TestForwarder(unittest.TestCase):
         # scope (zone) ID
         scope6 = ('fe80::1', TEST_PORT, 0, 1)
         self.check_push_and_pop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP,
-                                local6, scope6, TEST_DATA, False)
+                                local6_alt2, scope6, TEST_DATA, False)
 
     def test_push_too_fast(self):
         # A straightforward port of C++ pushTooFast test.
@@ -231,10 +243,10 @@ class TestForwarder(unittest.TestCase):
         s = socket.socket(socket.AF_UNIX, SOCK_STREAM, 0)
         s.setblocking(False)
         s.connect(TEST_UNIX_FILE)
-        accept_sock = self.accept_forwarder()
-        receiver = SocketSessionReceiver(accept_sock)
-        s.close()
-        self.assertRaises(SocketSessionError, receiver.pop)
+        with self.accept_forwarder() as accept_sock:
+            receiver = SocketSessionReceiver(accept_sock)
+            s.close()
+            self.assertRaises(SocketSessionError, receiver.pop)
 
 class TestReceiver(unittest.TestCase):
     # We only check a couple of failure cases on construction.  Valid cases

src/lib/python/isc/xfrin/diff.py

@@ -24,6 +24,7 @@ But for now, it lives here.
 """
 
 import isc.dns
+from isc.datasrc import ZoneFinder
 import isc.log
 from isc.datasrc import ZoneFinder
 from isc.log_messages.libxfrin_messages import *
@@ -180,9 +181,13 @@ class Diff:
                              str(self.__updater.get_class()))
         if self.__single_update_mode:
             if operation == 'add':
-                self.__append_with_soa_check(self.__additions, operation, rr)
+                if not self._remove_rr_from_deletions(rr):
+                    self.__append_with_soa_check(self.__additions, operation,
+                                                 rr)
             elif operation == 'delete':
-                self.__append_with_soa_check(self.__deletions, operation, rr)
+                if not self._remove_rr_from_additions(rr):
+                    self.__append_with_soa_check(self.__deletions, operation,
+                                                 rr)
         else:
             self.__buffer.append((operation, rr))
             if len(self.__buffer) >= DIFF_APPLY_TRESHOLD:
@@ -407,3 +412,175 @@ class Diff:
         """
         self.__check_committed()
         return self.__updater.find_all(name, options)
+
+    def __remove_rr_from_buffer(self, buf, rr):
+        '''Helper for common code in remove_rr_from_deletions() and
+           remove_rr_from_additions();
+           returns the result of the removal operation on the given buffer
+        '''
+        def same_rr(a, b):
+            # Consider two rr's the same if name, type, and rdata match
+            # Note that at this point it should have been checked that
+            # the rr in the buffer and the given rr have exactly one rdata
+            return a.get_name() == b.get_name() and\
+                   a.get_type() == b.get_type() and\
+                   a.get_rdata()[0] == b.get_rdata()[0]
+        if rr.get_type() == isc.dns.RRType.SOA():
+            return buf
+        else:
+            return [ op for op in buf if not same_rr(op[1], rr)]
+
+    def _remove_rr_from_deletions(self, rr):
+        '''
+        Removes the given rr from the currently buffered deletions;
+        returns True if anything is removed, False if the RR was not present.
+        This method is protected; it is not meant to be called from anywhere
+        but the add_data() method. It is not private for easier testing.
+        '''
+        orig_size = len(self.__deletions)
+        self.__deletions = self.__remove_rr_from_buffer(self.__deletions, rr)
+        return len(self.__deletions) != orig_size
+
+    def _remove_rr_from_additions(self, rr):
+        '''
+        Removes the given rr from the currently buffered additions;
+        returns True if anything is removed, False if the RR was not present.
+        This method is protected; it is not meant to be called from anywhere
+        but the delete_data() method. It is not private for easier testing.
+        '''
+        orig_size = len(self.__additions)
+        self.__additions = self.__remove_rr_from_buffer(self.__additions, rr)
+        return len(self.__additions) != orig_size
+
+    def __get_name_from_additions(self, name):
+        '''
+        Returns a list of all rrs in the additions queue that have the given
+        Name.
+        This method is protected; it is not meant to be called from anywhere
+        but the find_all_updated() method. It is not private for easier
+        testing.
+        '''
+        return [ rr for (_, rr) in self.__additions if rr.get_name() == name ]
+
+    def __get_name_from_deletions(self, name):
+        '''
+        Returns a list of all rrs in the deletions queue that have the given
+        Name
+        This method is protected; it is not meant to be called from anywhere
+        but the find_all_updated() method. It is not private for easier
+        testing.
+        '''
+        return [ rr for (_, rr) in self.__deletions if rr.get_name() == name ]
+
+    def __get_name_type_from_additions(self, name, rrtype):
+        '''
+        Returns a list of the rdatas of the rrs in the additions queue that
+        have the given name and type
+        This method is protected; it is not meant to be called from anywhere
+        but the find_updated() method. It is not private for easier testing.
+        '''
+        return [ rr for (_, rr) in self.__additions\
+                    if rr.get_name() == name and rr.get_type() == rrtype ]
+
+    def __get_name_type_from_deletions(self, name, rrtype):
+        '''
+        Returns a list of the rdatas of the rrs in the deletions queue that
+        have the given name and type
+        This method is protected; it is not meant to be called from anywhere
+        but the find_updated() method. It is not private for easier testing.
+        '''
+        return [ rr.get_rdata()[0] for (_, rr) in self.__deletions\
+                    if rr.get_name() == name and rr.get_type() == rrtype ]
+
+    def find_updated(self, name, rrtype):
+        '''
+        Returns the result of find(), but with current updates applied, i.e.
+        as if this diff has been committed. Only performs additional
+        processing in the case find() returns SUCCESS, NXDOMAIN, or NXRRSET;
+        in all other cases, the results are returned directly.
+        Any RRs in the current deletions buffer are removed from the result,
+        and RRs in the current additions buffer are added to the result.
+        If the result was SUCCESS, but every RR in it is removed due to
+        deletions, and there is nothing in the additions, the rcode is changed
+        to NXRRSET.
+        If the result was NXDOMAIN or NXRRSET, and there are rrs in the
+        additions buffer, the result is changed to SUCCESS.
+        '''
+        if not self.__single_update_mode:
+            raise ValueError("find_updated() can only be used in " +
+                             "single-update mode")
+        result, rrset, flags = self.find(name, rrtype)
+
+        added_rrs = self.__get_name_type_from_additions(name, rrtype)
+        deleted_rrs = self.__get_name_type_from_deletions(name, rrtype)
+
+        if result == ZoneFinder.SUCCESS:
+            new_rrset = isc.dns.RRset(name, self.__updater.get_class(),
+                                      rrtype, rrset.get_ttl())
+            for rdata in rrset.get_rdata():
+                if rdata not in deleted_rrs:
+                    new_rrset.add_rdata(rdata)
+            # If all data has been deleted, and there is nothing to add
+            # we cannot really know whether it is NXDOMAIN or NXRRSET,
+            # NXRRSET seems safest (we could find out, but it would require
+            # another search on the name which is probably not worth the
+            # trouble
+            if new_rrset.get_rdata_count() == 0 and len(added_rrs) == 0:
+                result = ZoneFinder.NXRRSET
+                new_rrset = None
+        elif (result == ZoneFinder.NXDOMAIN or result == ZoneFinder.NXRRSET)\
+             and len(added_rrs) > 0:
+            new_rrset = isc.dns.RRset(name, self.__updater.get_class(),
+                                      rrtype, added_rrs[0].get_ttl())
+            # There was no data in the zone, but there is data now
+            result = ZoneFinder.SUCCESS
+        else:
+            # Can't reliably handle other cases, just return the original
+            # data
+            return result, rrset, flags
+
+        for rr in added_rrs:
+            # Can only be 1-rr RRsets at this point
+            new_rrset.add_rdata(rr.get_rdata()[0])
+
+        return result, new_rrset, flags
+
+    def find_all_updated(self, name):
+        '''
+        Returns the result of find_all(), but with current updates applied,
+        i.e. as if this diff has been committed. Only performs additional
+        processing in the case find() returns SUCCESS or NXDOMAIN;
+        in all other cases, the results are returned directly.
+        Any RRs in the current deletions buffer are removed from the result,
+        and RRs in the current additions buffer are added to the result.
+        If the result was SUCCESS, but every RR in it is removed due to
+        deletions, and there is nothing in the additions, the rcode is changed
+        to NXDOMAIN.
+        If the result was NXDOMAIN, and there are rrs in the additions buffer,
+        the result is changed to SUCCESS.
+        '''
+        if not self.__single_update_mode:
+            raise ValueError("find_all_updated can only be used in " +
+                             "single-update mode")
+        result, rrsets, flags = self.find_all(name)
+        new_rrsets = []
+        added_rrs = self.__get_name_from_additions(name)
+        if result == ZoneFinder.SUCCESS and\
+           (flags & ZoneFinder.RESULT_WILDCARD == 0):
+            deleted_rrs = self.__get_name_from_deletions(name)
+            for rr in rrsets:
+                if rr not in deleted_rrs:
+                    new_rrsets.append(rr)
+            if len(new_rrsets) == 0 and len(added_rrs) == 0:
+                result = ZoneFinder.NXDOMAIN
+        elif result == ZoneFinder.NXDOMAIN and\
+            len(added_rrs) > 0:
+            result = ZoneFinder.SUCCESS
+        else:
+            # Can't reliably handle other cases, just return the original
+            # data
+            return result, rrsets, flags
+        for rr in added_rrs:
+            if rr.get_name() == name:
+                new_rrsets.append(rr)
+        return result, new_rrsets, flags

src/lib/python/isc/xfrin/tests/diff_tests.py

@@ -77,6 +77,31 @@ class DiffTest(unittest.TestCase):
         self.__rrset_multi.add_rdata(Rdata(self.__type, self.__rrclass,
                                            '192.0.2.2'))
 
+        # Also create a few other (valid) rrsets
+        # A SOA record
+        self.__rrset_soa = RRset(Name('example.org.'), self.__rrclass,
+                                 RRType.SOA(), RRTTL(3600))
+        self.__rrset_soa.add_rdata(Rdata(RRType.SOA(), self.__rrclass,
+                                         "ns1.example.org. " +
+                                         "admin.example.org. " +
+                                         "1233 3600 1800 2419200 7200"))
+        # A few single-rr rrsets that together would for a multi-rr rrset
+        self.__rrset3 = RRset(Name('c.example.org.'), self.__rrclass,
+                              RRType.TXT(), self.__ttl)
+        self.__rrset3.add_rdata(Rdata(RRType.TXT(), self.__rrclass, "one"))
+        self.__rrset4 = RRset(Name('c.example.org.'), self.__rrclass,
+                              RRType.TXT(), self.__ttl)
+        self.__rrset4.add_rdata(Rdata(RRType.TXT(), self.__rrclass, "two"))
+        self.__rrset5 = RRset(Name('c.example.org.'), self.__rrclass,
+                              RRType.TXT(), self.__ttl)
+        self.__rrset5.add_rdata(Rdata(RRType.TXT(), self.__rrclass, "three"))
+        self.__rrset6 = RRset(Name('d.example.org.'), self.__rrclass,
+                              RRType.A(), self.__ttl)
+        self.__rrset6.add_rdata(Rdata(RRType.A(), self.__rrclass, "192.0.2.1"))
+        self.__rrset7 = RRset(Name('d.example.org.'), self.__rrclass,
+                              RRType.A(), self.__ttl)
+        self.__rrset7.add_rdata(Rdata(RRType.A(), self.__rrclass, "192.0.2.2"))
+
     def __mock_compact(self):
         """
         This can be put into the diff to hook into its compact method and see
@@ -525,6 +550,17 @@ class DiffTest(unittest.TestCase):
         self.assertRaises(ValueError, diff_single.get_buffer)
         self.assertEqual(([], []), diff_single.get_single_update_buffers())
 
+    def test_finds_single(self):
+        '''
+        Test that find_updated() and find_all_updated() can only be used
+        in single-update-mode.
+        '''
+        diff_multi = Diff(self, Name('example.org.'), single_update_mode=False)
+        self.assertRaises(ValueError, diff_multi.find_updated,
+                          Name('example.org.'), RRType.A())
+        self.assertRaises(ValueError, diff_multi.find_all_updated,
+                          Name('example.org.'))
+
     def test_single_update_mode(self):
         '''
         Test single-update mode. In this mode, updates and deletes can
@@ -533,55 +569,47 @@ class DiffTest(unittest.TestCase):
         and it must be the first change.
         '''
 
-        # First create some RRsets to play with
-        soa = RRset(Name('example.org.'), self.__rrclass, RRType.SOA(),
-                    RRTTL(3600))
-        soa.add_rdata(Rdata(soa.get_type(), soa.get_class(),
-                      "ns.example.org. foo.example.org. 1234 28800 "+
-                      "7200 604800 3600"))
-
-        a = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(),
-                      RRTTL(3600))
-        a.add_rdata(Rdata(a.get_type(), a.get_class(),
-                        "192.0.2.1"))
-
-        a2 = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(),
-                      RRTTL(3600))
-        a2.add_rdata(Rdata(a2.get_type(), a2.get_class(),
-                        "192.0.2.2"))
-
         # full rrset for A (to check compact())
-        a_1_2 = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(),
-                      RRTTL(3600))
-        a_1_2.add_rdata(Rdata(a_1_2.get_type(), a_1_2.get_class(),
-                        "192.0.2.1"))
-        a_1_2.add_rdata(Rdata(a_1_2.get_type(), a_1_2.get_class(),
-                        "192.0.2.2"))
+        txt = RRset(Name('c.example.org.'), self.__rrclass, RRType.TXT(),
+                    RRTTL(3600))
+        txt.add_rdata(Rdata(txt.get_type(), txt.get_class(), "one"))
+        txt.add_rdata(Rdata(txt.get_type(), txt.get_class(), "two"))
+        txt.add_rdata(Rdata(txt.get_type(), txt.get_class(), "three"))
+        a = RRset(Name('d.example.org.'), self.__rrclass, RRType.A(),
+                  RRTTL(3600))
+        a.add_rdata(Rdata(a.get_type(), a.get_class(), "192.0.2.1"))
+        a.add_rdata(Rdata(a.get_type(), a.get_class(), "192.0.2.2"))
 
         diff = Diff(self, Name('example.org.'), single_update_mode=True)
 
         # adding a first should fail
         self.assertRaises(ValueError, diff.add_data, a)
         # But soa should work
-        diff.add_data(soa)
+        diff.add_data(self.__rrset_soa)
         # And then A should as well
-        diff.add_data(a)
-        diff.add_data(a2)
+        diff.add_data(self.__rrset3)
+        diff.add_data(self.__rrset4)
+        diff.add_data(self.__rrset5)
         # But another SOA should fail again
-        self.assertRaises(ValueError, diff.add_data, soa)
+        self.assertRaises(ValueError, diff.add_data, self.__rrset_soa)
 
         # Same for delete
-        self.assertRaises(ValueError, diff.delete_data, a)
-        diff.delete_data(soa)
-        diff.delete_data(a)
-        diff.delete_data(a2)
-        self.assertRaises(ValueError, diff.delete_data, soa)
+        self.assertRaises(ValueError, diff.delete_data, self.__rrset6)
+        diff.delete_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset6)
+        diff.delete_data(self.__rrset7)
+        self.assertRaises(ValueError, diff.delete_data, self.__rrset_soa)
 
         # Not compacted yet, so the buffers should be as we
         # filled them
         (delbuf, addbuf) = diff.get_single_update_buffers()
-        self.assertEqual([('delete', soa), ('delete', a), ('delete', a2)], delbuf)
-        self.assertEqual([('add', soa), ('add', a), ('add', a2)], addbuf)
+        self.assertEqual([('delete', self.__rrset_soa),
+                          ('delete', self.__rrset6),
+                          ('delete', self.__rrset7)], delbuf)
+        self.assertEqual([('add', self.__rrset_soa),
+                          ('add', self.__rrset3),
+                          ('add', self.__rrset4),
+                          ('add', self.__rrset5)], addbuf)
 
         # Compact should compact the A records in both buffers
         diff.compact()
@@ -590,18 +618,18 @@ class DiffTest(unittest.TestCase):
         self.assertEqual(2, len(delbuf))
         self.assertEqual(2, len(delbuf[0]))
         self.assertEqual('delete', delbuf[0][0])
-        self.assertEqual(soa.to_text(), delbuf[0][1].to_text())
+        self.assertEqual(self.__rrset_soa.to_text(), delbuf[0][1].to_text())
         self.assertEqual(2, len(delbuf[1]))
         self.assertEqual('delete', delbuf[1][0])
-        self.assertEqual(a_1_2.to_text(), delbuf[1][1].to_text())
+        self.assertEqual(a.to_text(), delbuf[1][1].to_text())
 
         self.assertEqual(2, len(addbuf))
         self.assertEqual(2, len(addbuf[0]))
         self.assertEqual('add', addbuf[0][0])
-        self.assertEqual(soa.to_text(), addbuf[0][1].to_text())
+        self.assertEqual(self.__rrset_soa.to_text(), addbuf[0][1].to_text())
         self.assertEqual(2, len(addbuf[1]))
         self.assertEqual('add', addbuf[1][0])
-        self.assertEqual(a_1_2.to_text(), addbuf[1][1].to_text())
+        self.assertEqual(txt.to_text(), addbuf[1][1].to_text())
 
         # Apply should reset the buffers
         diff.apply()
@@ -614,6 +642,40 @@ class DiffTest(unittest.TestCase):
         self.assertRaises(ValueError, diff.add_data, a)
         self.assertRaises(ValueError, diff.delete_data, a)
 
+    def test_add_delete_same(self):
+        '''
+        Test that if a record is added, then deleted, it is not added to
+        both buffers, but remove from the addition, and vice versa
+        '''
+        diff = Diff(self, Name('example.org.'), single_update_mode=True)
+        # Need SOA records first
+        diff.delete_data(self.__rrset_soa)
+        diff.add_data(self.__rrset_soa)
+
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(1, len(deletions))
+        self.assertEqual(1, len(additions))
+
+        diff.add_data(self.__rrset1)
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(1, len(deletions))
+        self.assertEqual(2, len(additions))
+
+        diff.delete_data(self.__rrset1)
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(1, len(deletions))
+        self.assertEqual(1, len(additions))
+
+        diff.delete_data(self.__rrset2)
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(2, len(deletions))
+        self.assertEqual(1, len(additions))
+
+        diff.add_data(self.__rrset2)
+        deletions, additions = diff.get_single_update_buffers()
+        self.assertEqual(1, len(deletions))
+        self.assertEqual(1, len(additions))
+
     def test_find(self):
         diff = Diff(self, Name('example.org.'))
         name = Name('www.example.org.')
@@ -675,6 +737,356 @@ class DiffTest(unittest.TestCase):
         self.assertEqual(name, self.__find_all_name)
         self.assertEqual(options, self.__find_all_options)
 
+    def __common_remove_rr_from_buffer(self, diff, add_method, remove_method,
+                                       op_str, buf_nr):
+        add_method(self.__rrset_soa)
+        add_method(self.__rrset2)
+        add_method(self.__rrset3)
+        add_method(self.__rrset4)
+
+        # sanity check
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset2,
+                                                   self.__rrset3,
+                                                   self.__rrset4 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+        # remove one
+        self.assertTrue(remove_method(self.__rrset2))
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset3,
+                                                   self.__rrset4 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+        # SOA should not be removed
+        self.assertFalse(remove_method(self.__rrset_soa))
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset3,
+                                                   self.__rrset4 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+        # remove another
+        self.assertTrue(remove_method(self.__rrset4))
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset3 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+        # remove nonexistent should return False
+        self.assertFalse(remove_method(self.__rrset4))
+        buf = diff.get_single_update_buffers()[buf_nr]
+        expected = [ (op_str, str(rr)) for rr in [ self.__rrset_soa,
+                                                   self.__rrset3 ] ]
+        result = [ (op, str(rr)) for (op, rr) in buf ]
+        self.assertEqual(expected, result)
+
+    def test_remove_rr_from_additions(self):
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        self.__common_remove_rr_from_buffer(diff, diff.add_data,
+                                               diff._remove_rr_from_additions,
+                                               'add', 1)
+
+    def test_remove_rr_from_deletions(self):
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        self.__common_remove_rr_from_buffer(diff, diff.delete_data,
+                                            diff._remove_rr_from_deletions,
+                                            'delete', 0)
+
+    def __create_find(self, result, rrset, flags):
+        '''
+        Overwrites the local find() method with a method that returns
+        the tuple (result, rrset, flags)
+        '''
+        def new_find(name, rrtype, fflags):
+            return (result, rrset, flags)
+        self.find = new_find
+
+    def __create_find_all(self, result, rrsets, flags):
+        '''
+        Overwrites the local find() method with a method that returns
+        the tuple (result, rrsets, flags)
+        '''
+        def new_find_all(name, fflags):
+            return (result, rrsets, flags)
+        self.find_all = new_find_all
+
+    def __check_find_call(self, method, query_rrset, expected_rcode,
+                          expected_rdatas=None):
+        '''
+        Helper for find tests; calls the given method with the name and
+        type of the given rrset. Checks for the given rcode.
+        If expected_rdatas is not none, the result name, and type are
+        checked to match the given rrset ones, and the rdatas are checked
+        to be equal.
+        The given method must have the same arguments and return type
+        as find()
+        '''
+        result, rrset, _ = method(query_rrset.get_name(),
+                                  query_rrset.get_type())
+        self.assertEqual(expected_rcode, result)
+        if expected_rdatas is not None:
+            self.assertEqual(query_rrset.get_name(), rrset.get_name())
+            self.assertEqual(query_rrset.get_type(), rrset.get_type())
+            if expected_rdatas is not None:
+                self.assertEqual(expected_rdatas, rrset.get_rdata())
+        else:
+            self.assertEqual(None, rrset)
+
+    def __check_find_all_call(self, method, query_rrset, expected_rcode,
+                              expected_rrs=[]):
+        '''
+        Helper for find tests; calls the given method with the name and
+        type of the given rrset. Checks for the given rcode.
+        If expected_rdatas is not none, the result name, and type are
+        checked to match the given rrset ones, and the rdatas are checked
+        to be equal.
+        The given method must have the same arguments and return type
+        as find()
+        '''
+        result, rrsets, _ = method(query_rrset.get_name())
+        self.assertEqual(expected_rcode, result)
+        # We have no real equality function for rrsets, but since
+        # the rrsets in question are themselves returns, pointer equality
+        # works as well
+        self.assertEqual(expected_rrs, rrsets)
+
+    def test_find_updated_existing_data(self):
+        '''
+        Tests whether existent data is updated with the additions and
+        deletions from the Diff
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset_soa)
+
+        # override the actual find method
+        self.__create_find(ZoneFinder.SUCCESS, self.__rrset3, 0)
+
+        # sanity check
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # check that normal find also returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # Adding another should have it returned in the find_updated
+        diff.add_data(self.__rrset4)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata() +
+                               self.__rrset4.get_rdata())
+
+        # check that normal find still returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # Adding a different type should have no effect
+        diff.add_data(self.__rrset2)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata() +
+                               self.__rrset4.get_rdata())
+
+        # check that normal find still returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # Deleting 3 now should result in only 4 being updated
+        diff.delete_data(self.__rrset3)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset4.get_rdata())
+
+        # check that normal find still returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+        # Deleting 4 now should result in empty rrset
+        diff.delete_data(self.__rrset4)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.NXRRSET)
+
+        # check that normal find still returns the original data
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+
+    def test_find_updated_nonexistent_data(self):
+        '''
+        Test whether added data for a query that would originally result
+        in NXDOMAIN works
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset_soa)
+
+        # override the actual find method
+        self.__create_find(ZoneFinder.NXDOMAIN, None, 0)
+
+        # Sanity check
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+
+        # Add data and see it is returned
+        diff.add_data(self.__rrset3)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+
+        # Add unrelated data, result should be the same
+        diff.add_data(self.__rrset2)
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.SUCCESS, self.__rrset3.get_rdata())
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+
+        # Remove, result should now be NXDOMAIN again
+        diff.delete_data(self.__rrset3)
+        result, rrset, _ = diff.find_updated(self.__rrset3.get_name(),
+                                             self.__rrset3.get_type())
+        self.__check_find_call(diff.find_updated, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+        self.__check_find_call(diff.find, self.__rrset3,
+                               ZoneFinder.NXDOMAIN)
+
+    def test_find_updated_other(self):
+        '''
+        Test that any other ZoneFinder.result code is directly
+        passed on.
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+
+        # Add and delete some data to make sure it's not used
+        diff.add_data(self.__rrset_soa)
+        diff.add_data(self.__rrset3)
+        diff.delete_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset2)
+
+        for rcode in [ ZoneFinder.DELEGATION,
+                       ZoneFinder.CNAME,
+                       ZoneFinder.DNAME ]:
+            # override the actual find method
+            self.__create_find(rcode, None, 0)
+            self.__check_find_call(diff.find, self.__rrset3, rcode)
+            self.__check_find_call(diff.find_updated, self.__rrset3, rcode)
+
+    def test_find_all_existing_data(self):
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset_soa)
+
+        # override the actual find method
+        self.__create_find_all(ZoneFinder.SUCCESS, [ self.__rrset3 ], 0)
+
+        # Sanity check
+        result, rrsets, _ = diff.find_all_updated(self.__rrset3.get_name())
+        self.assertEqual(ZoneFinder.SUCCESS, result)
+        self.assertEqual([self.__rrset3], rrsets)
+
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+        self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+
+        # Add a second rr with different type at same name
+        add_rrset = RRset(self.__rrset3.get_name(), self.__rrclass,
+                          RRType.A(), self.__ttl)
+        add_rrset.add_rdata(Rdata(RRType.A(), self.__rrclass, "192.0.2.2"))
+        diff.add_data(add_rrset)
+
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                   ZoneFinder.SUCCESS,
+                                   [self.__rrset3, add_rrset])
+        self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+
+        # Remove original one
+        diff.delete_data(self.__rrset3)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [add_rrset])
+        self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+
+        # And remove new one, result should then become NXDOMAIN
+        diff.delete_data(add_rrset)
+        result, rrsets, _ = diff.find_all_updated(self.__rrset3.get_name())
+
+        self.assertEqual(ZoneFinder.NXDOMAIN, result)
+        self.assertEqual([ ], rrsets)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                   ZoneFinder.NXDOMAIN)
+        self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                   ZoneFinder.SUCCESS, [self.__rrset3])
+
+    def test_find_all_nonexistent_data(self):
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset_soa)
+
+        self.__create_find_all(ZoneFinder.NXDOMAIN, [], 0)
+
+        # Sanity check
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+        self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+
+        # Adding data should change the result
+        diff.add_data(self.__rrset2)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                   ZoneFinder.SUCCESS, [ self.__rrset2 ])
+        self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+
+        # Adding data at other name should not
+        diff.add_data(self.__rrset3)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                   ZoneFinder.SUCCESS, [ self.__rrset2 ])
+        self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+
+        # Deleting it should revert to original
+        diff.delete_data(self.__rrset2)
+        self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+        self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                   ZoneFinder.NXDOMAIN)
+
+    def test_find_all_other_results(self):
+        '''
+        Any result code other than SUCCESS and NXDOMAIN should cause
+        the results to be passed on directly
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+
+        # Add and delete some data to make sure it's not used
+        diff.add_data(self.__rrset_soa)
+        diff.add_data(self.__rrset3)
+        diff.delete_data(self.__rrset_soa)
+        diff.delete_data(self.__rrset2)
+
+        for rcode in [ ZoneFinder.NXRRSET,
+                       ZoneFinder.DELEGATION,
+                       ZoneFinder.CNAME,
+                       ZoneFinder.DNAME ]:
+            # override the actual find method
+            self.__create_find_all(rcode, [], 0)
+            self.__check_find_all_call(diff.find_all_updated, self.__rrset2,
+                                       rcode)
+            self.__check_find_all_call(diff.find_all_updated, self.__rrset3,
+                                       rcode)
+            self.__check_find_all_call(diff.find_all, self.__rrset2,
+                                       rcode)
+            self.__check_find_all_call(diff.find_all, self.__rrset3,
+                                       rcode)
+
 if __name__ == "__main__":
     isc.log.init("bind10")
     isc.log.resetUnitTestRootLogger()

src/lib/resolve/tests/recursive_query_unittest.cc

@@ -175,6 +175,10 @@ protected:
         resolver_.reset();
     }
 
+    void SetUp() {
+        callback_.reset(new ASIOCallBack(this));
+    }
+
     // Send a test UDP packet to a mock server
     void sendUDP(const int family) {
         ScopedAddrInfo sai(resolveAddress(family, IPPROTO_UDP, false));
@@ -190,7 +194,7 @@ protected:
         if (cc != sizeof(test_data)) {
             isc_throw(IOError, "unexpected sendto result: " << cc);
         }
-        io_service_->run();
+        io_service_.run();
     }
 
     // Send a test TCP packet to a mock server
@@ -210,7 +214,7 @@ protected:
         if (cc != sizeof(test_data)) {
             isc_throw(IOError, "unexpected send result: " << cc);
         }
-        io_service_->run();
+        io_service_.run();
     }
 
     // Receive a UDP packet from a mock server; used for testing
@@ -233,10 +237,10 @@ protected:
         // The IO service queue should have a RecursiveQuery object scheduled
         // to run at this point.  This call will cause it to begin an
         // async send, then return.
-        io_service_->run_one();
+        io_service_.run_one();
 
         // ... and this one will block until the send has completed
-        io_service_->run_one();
+        io_service_.run_one();
 
         // Now we attempt to recv() whatever was sent.
         // XXX: there's no guarantee the receiving socket can immediately get
@@ -326,9 +330,7 @@ protected:
     // Set up empty DNS Service
     // Set up an IO Service queue without any addresses
     void setDNSService() {
-        io_service_.reset(new IOService());
-        callback_.reset(new ASIOCallBack(this));
-        dns_service_.reset(new DNSService(*io_service_, callback_.get(), NULL,
+        dns_service_.reset(new DNSService(io_service_, callback_.get(), NULL,
                                           NULL));
     }
 
@@ -491,12 +493,10 @@ private:
             static_cast<const uint8_t*>(io_message.getData()),
             static_cast<const uint8_t*>(io_message.getData()) +
             io_message.getDataSize());
-        io_service_->stop();
+        io_service_.stop();
     }
 protected:
-    // We use a pointer for io_service_, because for some tests we
-    // need to recreate a new one within one onstance of this class
-    scoped_ptr<IOService> io_service_;
+    IOService io_service_;
     scoped_ptr<DNSService> dns_service_;
     scoped_ptr<isc::nsas::NameserverAddressStore> nsas_;
     isc::cache::ResolverCache cache_;
@@ -513,24 +513,26 @@ RecursiveQueryTest::RecursiveQueryTest() :
     dns_service_(NULL), callback_(NULL), callback_protocol_(0),
     callback_native_(-1), resolver_(new isc::util::unittests::TestResolver())
 {
-    io_service_.reset(new IOService());
-    setDNSService(true, true);
     nsas_.reset(new isc::nsas::NameserverAddressStore(resolver_));
 }
 
 TEST_F(RecursiveQueryTest, v6UDPSend) {
+    setDNSService(true, true);
     doTest(AF_INET6, IPPROTO_UDP);
 }
 
 TEST_F(RecursiveQueryTest, v6TCPSend) {
+    setDNSService(true, true);
     doTest(AF_INET6, IPPROTO_TCP);
 }
 
 TEST_F(RecursiveQueryTest, v4UDPSend) {
+    setDNSService(true, true);
     doTest(AF_INET, IPPROTO_UDP);
 }
 
 TEST_F(RecursiveQueryTest, v4TCPSend) {
+    setDNSService(true, true);
     doTest(AF_INET, IPPROTO_TCP);
 }
 
@@ -643,7 +645,7 @@ TEST_F(RecursiveQueryTest, forwarderSend) {
     // to the same port as the actual server
     uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
 
-    MockServer server(*io_service_);
+    MockServer server(io_service_);
     RecursiveQuery rq(*dns_service_,
                       *nsas_, cache_,
                       singleAddress(TEST_IPV4_ADDR, port),
@@ -766,7 +768,7 @@ TEST_F(RecursiveQueryTest, forwardQueryTimeout) {
 
     // Prepare the server
     bool done(true);
-    MockServerStop server(*io_service_, &done);
+    MockServerStop server(io_service_, &done);
 
     // Do the answer
     const uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
@@ -784,7 +786,7 @@ TEST_F(RecursiveQueryTest, forwardQueryTimeout) {
     boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
     query.forward(ConstMessagePtr(&query_message), answer, buffer, &server, callback);
     // Run the test
-    io_service_->run();
+    io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
 }
 
@@ -800,7 +802,7 @@ TEST_F(RecursiveQueryTest, forwardClientTimeout) {
 
     // Prepare the server
     bool done1(true);
-    MockServerStop server(*io_service_, &done1);
+    MockServerStop server(io_service_, &done1);
 
     MessagePtr answer(new Message(Message::RENDER));
 
@@ -819,7 +821,7 @@ TEST_F(RecursiveQueryTest, forwardClientTimeout) {
     boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
     query.forward(ConstMessagePtr(&query_message), answer, buffer, &server, callback);
     // Run the test
-    io_service_->run();
+    io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
 }
 
@@ -834,7 +836,7 @@ TEST_F(RecursiveQueryTest, forwardLookupTimeout) {
 
     // Prepare the server
     bool done(true);
-    MockServerStop server(*io_service_, &done);
+    MockServerStop server(io_service_, &done);
 
     MessagePtr answer(new Message(Message::RENDER));
 
@@ -854,7 +856,7 @@ TEST_F(RecursiveQueryTest, forwardLookupTimeout) {
     boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
     query.forward(ConstMessagePtr(&query_message), answer, buffer, &server, callback);
     // Run the test
-    io_service_->run();
+    io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
 }
 
@@ -869,7 +871,7 @@ TEST_F(RecursiveQueryTest, lowtimeouts) {
 
     // Prepare the server
     bool done(true);
-    MockServerStop server(*io_service_, &done);
+    MockServerStop server(io_service_, &done);
 
     MessagePtr answer(new Message(Message::RENDER));
 
@@ -889,7 +891,7 @@ TEST_F(RecursiveQueryTest, lowtimeouts) {
     boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
     query.forward(ConstMessagePtr(&query_message), answer, buffer, &server, callback);
     // Run the test
-    io_service_->run();
+    io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
 }
 
@@ -903,7 +905,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
     setDNSService(true, false);
     bool done;
     
-    MockServerStop server(*io_service_, &done);
+    MockServerStop server(io_service_, &done);
     vector<pair<string, uint16_t> > empty_vector;
     RecursiveQuery rq(*dns_service_, *nsas_, cache_, empty_vector,
                       empty_vector, 10000, 0);
@@ -912,7 +914,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
     rq.resolve(q, answer, buffer, &server);
-    io_service_->run();
+    io_service_.run();
 
     // Check that the answer we got matches the one we wanted
     EXPECT_EQ(Rcode::NOERROR(), answer->getRcode());
@@ -929,7 +931,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
     setDNSService(true, false);
     bool done;
     
-    MockServerStop server(*io_service_, &done);
+    MockServerStop server(io_service_, &done);
     vector<pair<string, uint16_t> > empty_vector;
     RecursiveQuery rq(*dns_service_, *nsas_, cache_, empty_vector,
                       empty_vector, 10000, 0);
@@ -938,7 +940,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
     rq.resolve(q, answer, buffer, &server);
-    io_service_->run();
+    io_service_.run();
 
     // Check that the answer we got matches the one we wanted
     EXPECT_EQ(Rcode::NXDOMAIN(), answer->getRcode());
@@ -1012,7 +1014,7 @@ TEST_F(RecursiveQueryTest, CachedNS) {
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
     // The server is here so we have something to pass there
-    MockServer server(*io_service_);
+    MockServer server(io_service_);
     rq.resolve(q, answer, buffer, &server);
     // We don't need to run the service in this test. We are interested only
     // in the place it starts resolving at

src/lib/util/tests/socketsession_unittest.cc

@@ -53,6 +53,7 @@ namespace {
 
 const char* const TEST_UNIX_FILE = TEST_DATA_TOPBUILDDIR "/test.unix";
 const char* const TEST_PORT = "53535";
+const char* const TEST_PORT2 = "53536"; // use this in case we need 2 ports
 const char TEST_DATA[] = "BIND10 test";
 
 // A simple helper structure to automatically close test sockets on return
@@ -540,8 +541,12 @@ ForwardTest::checkPushAndPop(int family, int type, int protocol,
 }
 
 TEST_F(ForwardTest, pushAndPop) {
-    // Pass a UDP/IPv6 session.
+    // Pass a UDP/IPv6 session.  We use different ports for different UDP
+    // tests because Solaris 11 seems to prohibit reusing the same port for
+    // some short period once the socket FD is forwarded, even if the sockets
+    // are closed.  See Trac #2028.
     const SockAddrInfo sai_local6(getSockAddr("::1", TEST_PORT));
+    const SockAddrInfo sai_local6_alt(getSockAddr("::1", TEST_PORT2));
     const SockAddrInfo sai_remote6(getSockAddr("2001:db8::1", "5300"));
     {
         SCOPED_TRACE("Passing UDP/IPv6 session");
@@ -559,6 +564,7 @@ TEST_F(ForwardTest, pushAndPop) {
     // receiver, which should be usable for multiple attempts of passing,
     // regardless of family of the passed session
     const SockAddrInfo sai_local4(getSockAddr("127.0.0.1", TEST_PORT));
+    const SockAddrInfo sai_local4_alt(getSockAddr("127.0.0.1", TEST_PORT2));
     const SockAddrInfo sai_remote4(getSockAddr("192.0.2.2", "5300"));
     {
         SCOPED_TRACE("Passing UDP/IPv4 session");
@@ -575,7 +581,7 @@ TEST_F(ForwardTest, pushAndPop) {
     // Also try large data
     {
         SCOPED_TRACE("Passing UDP/IPv6 session with large data");
-        checkPushAndPop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP, sai_local6,
+        checkPushAndPop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP, sai_local6_alt,
                         sai_remote6, large_text_.c_str(), large_text_.length(),
                         false);
     }
@@ -587,7 +593,7 @@ TEST_F(ForwardTest, pushAndPop) {
     }
     {
         SCOPED_TRACE("Passing UDP/IPv4 session with large data");
-        checkPushAndPop(AF_INET, SOCK_DGRAM, IPPROTO_UDP, sai_local4,
+        checkPushAndPop(AF_INET, SOCK_DGRAM, IPPROTO_UDP, sai_local4_alt,
                         sai_remote4, large_text_.c_str(), large_text_.length(),
                         false);
     }

tests/tools/perfdhcp/Makefile.am

@@ -6,6 +6,12 @@ AM_CPPFLAGS += $(BOOST_INCLUDES)
 
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
+# Some versions of GCC warn about some versions of Boost regarding
+# missing initializer for members in its posix_time.
+# https://svn.boost.org/trac/boost/ticket/3477
+# But older GCC compilers don't have the flag.
+AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
+
 AM_LDFLAGS = $(CLOCK_GETTIME_LDFLAGS)
 AM_LDFLAGS += -lm
 if USE_STATIC_LINK
@@ -14,10 +20,21 @@ endif
 
 lib_LTLIBRARIES = libperfdhcp++.la
 libperfdhcp___la_SOURCES = command_options.cc command_options.h
+libperfdhcp___la_SOURCES += localized_option.h
+libperfdhcp___la_SOURCES += perf_pkt6.cc perf_pkt6.h
+libperfdhcp___la_SOURCES += perf_pkt4.cc perf_pkt4.h
+libperfdhcp___la_SOURCES += pkt_transform.cc pkt_transform.h
+
 libperfdhcp___la_CXXFLAGS = $(AM_CXXFLAGS)
+if USE_CLANGPP
+# Disable unused parameter warning caused by some of the
+# Boost headers when compiling with clang.
+libperfdhcp___la_CXXFLAGS += -Wno-unused-parameter
+endif
+
 libperfdhcp___la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+libperfdhcp___la_LIBADD += $(top_builddir)/src/lib/dhcp/libdhcp++.la
+libperfdhcp___la_LIBADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 
 pkglibexec_PROGRAMS  = perfdhcp
 perfdhcp_SOURCES  = perfdhcp.c
-
-

tests/tools/perfdhcp/command_options.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

tests/tools/perfdhcp/command_options.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

tests/tools/perfdhcp/localized_option.h

@@ -0,0 +1,123 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __LOCALIZED_OPTION_H
+#define __LOCALIZED_OPTION_H
+
+#include <dhcp/pkt6.h>
+
+namespace isc {
+namespace perfdhcp {
+
+/// \brief DHCP option at specific offset
+///
+/// This class represents DHCP option with data placed at specified
+/// offset in DHCP message.
+/// Objects of this type are intended to be used when DHCP packets
+/// are created from templates (e.g. read from template file).
+/// Such packets have number of options with contents that have to be
+/// replaced before sending: e.g. DUID can be randomized.
+/// If option of this type is added to \ref PerfPkt6 options collection,
+/// \ref perfdhcp::PerfPkt6 will call \ref getOffset on this object
+/// to retrieve user-defined option position and replace contents of
+/// the output buffer at this offset before packet is sent to the server.
+/// (\see perfdhcp::PerfPkt6::rawPack).
+/// In order to read on-wire data from incoming packet client class
+/// has to specify options of \ref perfdhcp::LocalizedOption type
+/// with expected offsets of these options in a packet. The
+/// \ref perfdhcp::PerfPkt6 will use offsets to read fragments
+/// of packet and store them in options' buffers.
+/// (\see perfdhcp::PerfPkt6::rawUnpack).
+///
+class LocalizedOption : public dhcp::Option {
+public:
+    /// \brief Constructor, sets default (0) option offset
+    ///
+    /// \param u specifies universe (V4 or V6)
+    /// \param type option type (0-255 for V4 and 0-65535 for V6)
+    /// \param data content of the option
+    LocalizedOption(dhcp::Option::Universe u,
+                    uint16_t type,
+                    const dhcp::OptionBuffer& data) :
+        dhcp::Option(u, type, data),
+        offset_(0) {
+    }
+
+
+    /// \brief Constructor, used to create localized option from buffer
+    ///
+    /// \param u specifies universe (V4 or V6)
+    /// \param type option type (0-255 for V4 and 0-65535 for V6)
+    /// \param data content of the option
+    /// \param offset location of option in a packet (zero is default)
+    LocalizedOption(dhcp::Option::Universe u,
+                    uint16_t type,
+                    const dhcp::OptionBuffer& data,
+                    const size_t offset) :
+        dhcp::Option(u, type, data),
+        offset_(offset) {
+    }
+
+    /// \brief Constructor, sets default (0) option offset
+    ///
+    /// This contructor is similar to the previous one, but it does not take
+    /// the whole vector<uint8_t>, but rather subset of it.
+    ///
+    /// \param u specifies universe (V4 or V6)
+    /// \param type option type (0-255 for V4 and 0-65535 for V6)
+    /// \param first iterator to the first element that should be copied
+    /// \param last iterator to the next element after the last one
+    ///        to be copied.
+    LocalizedOption(dhcp::Option::Universe u,
+                    uint16_t type,
+                    dhcp::OptionBufferConstIter first,
+                    dhcp::OptionBufferConstIter last) :
+        dhcp::Option(u, type, first, last),
+        offset_(0) {
+    }
+
+
+    /// \brief Constructor, used to create option from buffer iterators
+    ///
+    /// This contructor is similar to the previous one, but it does not take
+    /// the whole vector<uint8_t>, but rather subset of it.
+    ///
+    /// \param u specifies universe (V4 or V6)
+    /// \param type option type (0-255 for V4 and 0-65535 for V6)
+    /// \param first iterator to the first element that should be copied
+    /// \param last iterator to the next element after the last one
+    ///        to be copied.
+    /// \param offset offset of option in a packet (zero is default)
+    LocalizedOption(dhcp::Option::Universe u,
+                    uint16_t type,
+                    dhcp::OptionBufferConstIter first,
+                    dhcp::OptionBufferConstIter last, const size_t offset) :
+        dhcp::Option(u, type, first, last),
+        offset_(offset) {
+    }
+
+    /// \brief Returns offset of an option in a DHCP packet.
+    ///
+    /// \return option offset in a packet
+    size_t getOffset() const { return offset_; };
+
+private:
+    size_t offset_;   ///< Offset of DHCP option in a packet
+};
+
+
+} // namespace perfdhcp
+} // namespace isc
+
+#endif // __LOCALIZED_OPTION_H

tests/tools/perfdhcp/perf_pkt4.cc

@@ -0,0 +1,62 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dhcp/libdhcp++.h>
+#include <dhcp/dhcp6.h>
+
+#include "perf_pkt4.h"
+#include "pkt_transform.h"
+
+using namespace std;
+using namespace isc;
+using namespace dhcp;
+
+namespace isc {
+namespace perfdhcp {
+
+PerfPkt4::PerfPkt4(const uint8_t* buf,
+                   size_t len,
+                   size_t transid_offset,
+                   uint32_t transid) :
+    Pkt4(buf, len),
+    transid_offset_(transid_offset) {
+    setTransid(transid);
+}
+
+bool
+PerfPkt4::rawPack() {
+    return (PktTransform::pack(dhcp::Option::V4,
+                               data_,
+                               options_,
+                               getTransidOffset(),
+                               getTransid(),
+                               bufferOut_));
+}
+
+bool
+PerfPkt4::rawUnpack() {
+    uint32_t transid = getTransid();
+    bool res = PktTransform::unpack(dhcp::Option::V4,
+                                    data_,
+                                    options_,
+                                    getTransidOffset(),
+                                    transid);
+    if (res) {
+        setTransid(transid);
+    }
+    return (res);
+}
+
+} // namespace perfdhcp
+} // namespace isc

tests/tools/perfdhcp/perf_pkt4.h

@@ -0,0 +1,113 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __PERF_PKT4_H
+#define __PERF_PKT4_H
+
+#include <time.h>
+#include <boost/shared_ptr.hpp>
+#include <dhcp/pkt4.h>
+
+#include "localized_option.h"
+
+namespace isc {
+namespace perfdhcp {
+
+/// \brief PerfPkt4 (DHCPv4 packet)
+///
+/// This class extends the functionality of \ref isc::dhcp::Pkt4 by adding the
+/// ability to specify an options offset in the DHCP message and to override
+/// options' contents.  This is particularly useful when we create a packet
+/// object using a template file (i.e. do not build it dynamically). The client
+/// class should read data from the template file and pass it to this class in
+/// a buffer.
+///
+/// The contents of such a packet can be later partially replaced, notably the
+/// selected options and the transaction ID.  (The transaction ID and its
+/// offset in the template file are passed via the constructor.)
+///
+/// In order to replace contents of the options, the client class has to
+/// create a collection of \ref LocalizedOption, adding them using
+/// \ref dhcp::Pkt4::addOption.
+///
+/// \note If you don't use template files simply use constructors
+/// inherited from parent class and isc::dhcp::Option type instead
+
+class PerfPkt4 : public dhcp::Pkt4 {
+public:
+
+    /// Localized option pointer type.
+    typedef boost::shared_ptr<LocalizedOption> LocalizedOptionPtr;
+
+    /// \brief Constructor, used to create  messages from packet
+    /// template files.
+    ///
+    /// Creates a new DHCPv4 message using the provided buffer.
+    /// The transaction ID and its offset are specified via this
+    /// constructor. The transaction ID is stored in outgoing message
+    /// when client class calls \ref PerfPkt4::rawPack. Transaction id
+    /// offset value is used for incoming and outgoing messages to
+    /// identify transaction ID field's position in incoming and outgoing
+    /// messages.
+    ///
+    /// \param buf buffer holding contents of the message (this can
+    /// be directly read from template file).
+    /// \param len length of the data in the buffer.
+    /// \param transid_offset transaction id offset in a message.
+    /// \param transid transaction id to be stored in outgoing message.
+    PerfPkt4(const uint8_t* buf,
+             size_t len,
+             size_t transid_offset = 1,
+             uint32_t transid = 0);
+
+    /// \brief Returns transaction id offset in packet buffer
+    ///
+    /// \return Transaction ID offset in packet buffer
+    size_t getTransidOffset() const { return transid_offset_; };
+
+    /// \brief Prepares on-wire format from raw buffer.
+    ///
+    /// The method copies the buffer provided in the constructor to the
+    /// output buffer and replaces the transaction ID and selected
+    /// options with new data.
+    ///
+    /// \note Use this method to prepare an on-wire DHCPv4 message
+    /// when you use template packets that require replacement
+    /// of selected options' contents before sending.
+    ///
+    /// \return false ID pack operation failed.
+    bool rawPack();
+
+    /// \brief Handles limited binary packet parsing for packets with
+    /// custom offsets of options and transaction ID
+    ///
+    /// This method handles the parsing of packets that have custom offsets
+    /// of options or transaction ID. Use
+    /// \ref isc::dhcp::Pkt4::addOption to specify which options to parse.
+    /// Options should be of the \ref isc::perfdhcp::LocalizedOption
+    /// type with offset values provided. Each added option will
+    /// be updated with actual data read from the binary packet buffer.
+    ///
+    /// \return false If unpack operation failed.
+    bool rawUnpack();
+
+private:
+    size_t transid_offset_;      ///< transaction id offset
+
+};
+
+} // namespace perfdhcp
+} // namespace isc
+
+#endif // __PERF_PKT4_H

tests/tools/perfdhcp/perf_pkt6.cc

@@ -0,0 +1,64 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <iostream>
+#include <exceptions/exceptions.h>
+#include <dhcp/libdhcp++.h>
+#include <dhcp/dhcp6.h>
+
+#include "perf_pkt6.h"
+#include "pkt_transform.h"
+
+using namespace std;
+using namespace isc;
+using namespace dhcp;
+
+namespace isc {
+namespace perfdhcp {
+
+PerfPkt6::PerfPkt6(const uint8_t* buf,
+                   size_t len,
+                   size_t transid_offset,
+                   uint32_t transid) :
+    Pkt6(buf, len, Pkt6::UDP),
+    transid_offset_(transid_offset) {
+    setTransid(transid);
+}
+
+bool
+PerfPkt6::rawPack() {
+    return (PktTransform::pack(dhcp::Option::V6,
+                               data_,
+                               options_,
+                               getTransidOffset(),
+                               getTransid(),
+                               bufferOut_));
+}
+
+bool
+PerfPkt6::rawUnpack() {
+    uint32_t transid = getTransid();
+    bool res =  PktTransform::unpack(dhcp::Option::V6,
+                                     data_,
+                                     options_,
+                                     getTransidOffset(),
+                                     transid);
+    if (res) {
+        setTransid(transid);
+    }
+    return (res);
+}
+
+} // namespace perfdhcp
+} // namespace isc

tests/tools/perfdhcp/perf_pkt6.h

@@ -0,0 +1,113 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __PERF_PKT6_H
+#define __PERF_PKT6_H
+
+#include <time.h>
+#include <boost/shared_ptr.hpp>
+#include <dhcp/pkt6.h>
+
+#include "localized_option.h"
+
+namespace isc {
+namespace perfdhcp {
+
+/// \brief PerfPkt6 (DHCPv6 packet)
+///
+/// This class extends the functionality of \ref isc::dhcp::Pkt6 by
+/// adding the ability to specify an options offset in the DHCP message
+/// and so override the options' contents. This is particularly useful when we
+/// create a packet object using a template file (i.e. do not build it
+/// dynamically). The client class should read the data from the template file
+/// and pass it to this class as a buffer.
+///
+/// The contents of such packet can be later partially replaced: in particular,
+/// selected options and the transaction ID can be altered. (The transaction
+/// ID and its offset in the template file is passed via the constructor.)
+///
+/// In order to replace the contents of options, the client class has to
+/// create a collection of \ref LocalizedOption by adding them using
+/// \ref dhcp::Pkt6::addOption.
+///
+/// \note If you don't use template files, simply use constructors
+/// inherited from parent class and the \ref isc::dhcp::Option type instead.
+
+class PerfPkt6 : public dhcp::Pkt6 {
+public:
+
+    /// Localized option pointer type.
+    typedef boost::shared_ptr<LocalizedOption> LocalizedOptionPtr;
+
+    /// \brief Constructor, used to create messages from packet
+    /// template files.
+    ///
+    /// Creates a new DHCPv6 message using the provided buffer.
+    /// The transaction ID and its offset are specified via this
+    /// constructor. The transaction ID is stored in outgoing message
+    /// when client class calls \ref PerfPkt6::rawPack. Transaction id
+    /// offset value is used for incoming and outgoing messages to
+    /// identify transaction ID field's position in incoming and outgoing
+    /// messages.
+    ///
+    /// \param buf buffer holding contents of the message (this can
+    /// be directly read from template file).
+    /// \param len length of the data in the buffer.
+    /// \param transid_offset transaction id offset in a message.
+    /// \param transid transaction id to be stored in outgoing message.
+    PerfPkt6(const uint8_t* buf,
+             size_t len,
+             size_t transid_offset = 1,
+             uint32_t transid = 0);
+
+    /// \brief Returns transaction id offset in packet buffer
+    ///
+    /// \return Transaction ID offset in the packet buffer.
+    size_t getTransidOffset() const { return transid_offset_; };
+
+    /// \brief Prepares on-wire format from raw buffer
+    ///
+    /// The method copies the buffer provided in constructor to the
+    /// output buffer and replaces the transaction ID and selected
+    /// options with new data.
+    ///
+    /// \note Use this method to prepare an on-wire DHCPv6 message
+    /// when you use template packets that require replacement
+    /// of selected options' contents before sending.
+    ///
+    /// \return false ID pack operation failed.
+    bool rawPack();
+
+    /// \brief Handles limited binary packet parsing for packets with
+    /// custom offsets of options and transaction id
+    ///
+    /// This methoid handles the parsing of packets that have custom offsets
+    /// of options or transaction ID. Use
+    /// \ref isc::dhcp::Pkt4::addOption to specify which options to parse.
+    /// Options should be of the \ref isc::perfdhcp::LocalizedOption
+    /// type with offset values provided. Each added option will
+    /// be updated with actual data read from the binary packet buffer.
+    ///
+    /// \return false if unpack operation failed.
+    bool rawUnpack();
+
+private:
+    size_t transid_offset_;      ///< transaction id offset
+
+};
+
+} // namespace perfdhcp
+} // namespace isc
+
+#endif // __PERF_PKT6_H

tests/tools/perfdhcp/pkt_transform.cc

@@ -0,0 +1,222 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <iostream>
+
+#include <exceptions/exceptions.h>
+#include <dhcp/option.h>
+#include <dhcp/libdhcp++.h>
+#include <dhcp/dhcp6.h>
+
+#include "pkt_transform.h"
+#include "localized_option.h"
+
+using namespace std;
+using namespace isc;
+using namespace dhcp;
+
+namespace isc {
+namespace perfdhcp {
+
+bool
+PktTransform::pack(const Option::Universe universe,
+                   const OptionBuffer& in_buffer,
+                   const Option::OptionCollection& options,
+                   const size_t transid_offset,
+                   const uint32_t transid,
+                   util::OutputBuffer& out_buffer) {
+
+    // Always override the packet if function is called.
+    out_buffer.clear();
+    // Write whole buffer to output buffer.
+    out_buffer.writeData(&in_buffer[0], in_buffer.size());
+
+    uint8_t transid_len = (universe == Option::V6) ? 3 : 4;
+
+    if ((transid_offset + transid_len >= in_buffer.size()) ||
+        (transid_offset == 0)) {
+        cout << "Failed to build packet: provided transaction id offset: "
+             << transid_offset <<  " is out of bounds (expected 1.."
+             << in_buffer.size()-1 << ")." << endl;
+        return (false);
+    }
+
+    try {
+        size_t offset_ptr = transid_offset;
+        if (universe == Option::V4) {
+            out_buffer.writeUint8At(transid >> 24 & 0xFF, offset_ptr++);
+        }
+        out_buffer.writeUint8At(transid >> 16 & 0xFF, offset_ptr++);
+        out_buffer.writeUint8At(transid >> 8 & 0xFF, offset_ptr++);
+        out_buffer.writeUint8At(transid & 0xFF, offset_ptr++);
+
+        // We already have packet template stored in output buffer
+        // but still some options have to be updated if client
+        // specified them along with their offsets in the buffer.
+        PktTransform::packOptions(in_buffer, options, out_buffer);
+    } catch (const isc::BadValue& e) {
+        cout << "Building packet failed: " << e.what() << endl;
+        return (false);
+    }
+    return (true);
+}
+
+bool
+PktTransform::unpack(const Option::Universe universe,
+                     const OptionBuffer& in_buffer,
+                     const Option::OptionCollection& options,
+                     const size_t transid_offset,
+                     uint32_t& transid) {
+
+    uint8_t transid_len = (universe == Option::V6) ? 3 : 4;
+
+    // Validate transaction id offset.
+    if ((transid_offset + transid_len + 1 > in_buffer.size()) ||
+        (transid_offset == 0)) {
+        cout << "Failed to parse packet: provided transaction id offset: "
+             << transid_offset <<  " is out of bounds (expected 1.."
+             << in_buffer.size()-1 << ")." << endl;
+        return (false);
+    }
+
+    // Read transaction id from the buffer.
+    // For DHCPv6 we transaction id is 3 bytes long so the high byte
+    // of transid will be zero.
+    OptionBufferConstIter it = in_buffer.begin() + transid_offset;
+    transid = 0;
+    for (int i = 0; i < transid_len; ++i, ++it) {
+        // Read next byte and shift it left to its position in
+        // transid (shift by the number of bytes read so far.
+        transid += *it << (transid_len - i - 1) * 8;
+    }
+
+    try {
+        PktTransform::unpackOptions(in_buffer, options);
+    } catch (const isc::BadValue& e) {
+        cout << "Packet parsing failed: " << e.what() << endl;
+        return (false);
+    }
+
+    return (true);
+}
+
+void
+PktTransform::packOptions(const OptionBuffer& in_buffer,
+                          const Option::OptionCollection& options,
+                          util::OutputBuffer& out_buffer) {
+    try {
+        // If there are any options on the list, we will use provided
+        // options offsets to override them in the output buffer
+        // with new contents.
+        for (Option::OptionCollection::const_iterator it = options.begin();
+             it != options.end(); ++it) {
+            // Get options with their position (offset).
+            boost::shared_ptr<LocalizedOption> option =
+                boost::dynamic_pointer_cast<LocalizedOption>(it->second);
+            if (option == NULL) {
+                isc_throw(isc::BadValue, "option is null");
+            }
+            uint32_t offset = option->getOffset();
+            if ((offset == 0) ||
+                (offset + option->len() > in_buffer.size())) {
+                isc_throw(isc::BadValue,
+                          "option offset for option: " << option->getType()
+                          << " is out of bounds (expected 1.."
+                          << in_buffer.size() - option->len() << ")");
+            }
+
+            // Create temporary buffer to store option contents.
+            util::OutputBuffer buf(option->len());
+            // Pack option contents into temporary buffer.
+            option->pack(buf);
+            // OutputBuffer class has nice functions that write
+            // data at the specified position so we can use it to
+            // inject contents of temporary buffer to output buffer.
+            const uint8_t *buf_data =
+                static_cast<const uint8_t*>(buf.getData());
+            for (int i = 0; i < buf.getLength(); ++i) {
+                out_buffer.writeUint8At(buf_data[i], offset + i);
+            }
+        }
+    }
+    catch (const Exception&) {
+        isc_throw(isc::BadValue, "failed to pack options into buffer.");
+    }
+}
+
+void
+PktTransform::unpackOptions(const OptionBuffer& in_buffer,
+                            const Option::OptionCollection& options) {
+    for (Option::OptionCollection::const_iterator it = options.begin();
+         it != options.end(); ++it) {
+
+        boost::shared_ptr<LocalizedOption> option =
+            boost::dynamic_pointer_cast<LocalizedOption>(it->second);
+        if (option == NULL) {
+            isc_throw(isc::BadValue, "option is null");
+        }
+        size_t opt_pos = option->getOffset();
+        if (opt_pos == 0) {
+            isc_throw(isc::BadValue, "failed to unpack packet from raw buffer "
+                      "(Option position not specified)");
+        } else if (opt_pos + option->getHeaderLen() > in_buffer.size()) {
+            isc_throw(isc::BadValue,
+                      "failed to unpack options from from raw buffer "
+                      "(Option position out of bounds)");
+        }
+
+        size_t offset = opt_pos;
+        size_t offset_step = 1;
+        uint16_t opt_type = 0;
+        if (option->getUniverse() == Option::V6) {
+            offset_step = 2;
+            // For DHCPv6 option type is in first two octets.
+            opt_type = in_buffer[offset] * 256 + in_buffer[offset + 1];
+        } else {
+            // For DHCPv4 option type is in first octet.
+            opt_type = in_buffer[offset];
+        }
+        // Check if we got expected option type.
+        if (opt_type != option->getType()) {
+            isc_throw(isc::BadValue,
+                      "failed to unpack option from raw buffer "
+                      "(option type mismatch)");
+        }
+
+        // Get option length which is supposed to be after option type.
+        offset += offset_step;
+        uint16_t opt_len = in_buffer[offset] * 256 + in_buffer[offset + 1];
+        if (option->getUniverse() == Option::V6) {
+            opt_len = in_buffer[offset] * 256 + in_buffer[offset + 1];
+        } else {
+            opt_len = in_buffer[offset];
+        }
+
+        // Check if packet is not truncated.
+        if (offset + option->getHeaderLen() + opt_len > in_buffer.size()) {
+            isc_throw(isc::BadValue,
+                      "failed to unpack option from raw buffer "
+                      "(option truncated)");
+        }
+
+        // Seek to actual option data and replace it.
+        offset += offset_step;
+        option->setData(in_buffer.begin() + offset,
+                        in_buffer.begin() + offset + opt_len);
+    }
+}
+
+
+} // namespace perfdhcp
+} // namespace isc

tests/tools/perfdhcp/pkt_transform.h

@@ -0,0 +1,139 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __PKT_TRANSFORM_H
+#define __PKT_TRANSFORM_H
+
+#include <dhcp/option.h>
+
+#include "localized_option.h"
+
+namespace isc {
+namespace perfdhcp {
+
+/// \brief Read and write raw data to DHCP packets.
+///
+/// This class provides static functions to read/write raw data from/to the
+/// packet buffer. When reading data with the unpack() method, the
+/// corresponding options objects are updated.  When writing to the packet
+/// buffer with pack(), options objects carry input data to be written.
+///
+/// This class is used both by \ref PerfPkt4 and
+/// \ref PerfPkt6 classes in case DHCP packets are created
+/// from template files. In this case, some of the template
+/// packet's options are replaced before sending it to the
+/// server. Offset of specific options are provided from the
+/// command line by the perfdhcp tool user, and passed in an
+/// options collection.
+class PktTransform {
+public:
+
+    /// \brief Prepares on-wire format from raw buffer.
+    ///
+    /// The method copies the input buffer and options contents
+    /// to the output buffer. The input buffer must contain whole
+    /// initial packet data. Parts of this data will be
+    /// overriden by options data specified in an options
+    /// collection. Such options must have their offsets within
+    /// a packet specified (see \ref LocalizedOption to find out
+    /// how to specify options offset).
+    ///
+    /// \note The specified options must fit into the size of the
+    /// initial packet data. A call to this method will fail
+    /// if the option's offset + its size is beyond the packet's size.
+    ///
+    /// \param universe Universe used, V4 or V6
+    /// \param in_buffer Input buffer holding intial packet
+    /// data, this can be directly read from template file
+    /// \param options Options collection with offsets
+    /// \param transid_offset offset of transaction id in a packet,
+    /// transaction ID will be written to output buffer at this
+    /// offset
+    /// \param transid Transaction ID value
+    /// \param out_buffer Output buffer holding "packed" data
+    ///
+    /// \return false, if pack operation failed.
+    static bool pack(const dhcp::Option::Universe universe,
+                     const dhcp::OptionBuffer& in_buffer,
+                     const dhcp::Option::OptionCollection& options,
+                     const size_t transid_offset,
+                     const uint32_t transid,
+                     util::OutputBuffer& out_buffer);
+
+    /// \brief Handles selective binary packet parsing.
+    ///
+    /// This method handles the parsing of packets that have non-default
+    /// options or transaction ID offsets. The client class has to use
+    /// \ref isc::dhcp::Pkt6::addOption to specify which options to parse.
+    /// Each option should be of the \ref isc::perfdhcp::LocalizedOption
+    /// type with the offset value specified.
+    ///
+    /// \param universe universe used, V4 or V6
+    /// \param in_buffer input buffer to be parsed
+    /// \param options options collection with options offsets
+    /// \param transid_offset offset of transaction id in input buffer
+    /// \param transid transaction id value read from input buffer
+    ///
+    /// \return false, if unpack operation failed.
+    static bool unpack(const dhcp::Option::Universe universe,
+                       const dhcp::OptionBuffer& in_buffer,
+                       const dhcp::Option::OptionCollection& options,
+                       const size_t transid_offset,
+                       uint32_t& transid);
+
+private:
+    /// \brief Replaces contents of options in a buffer.
+    ///
+    /// The method uses a localized options collection to
+    /// replace parts of packet data (e.g. data read
+    /// from template file).
+    /// This private method is called from \ref PktTransform::pack
+    ///
+    /// \param in_buffer input buffer holding initial packet data.
+    /// \param out_buffer output buffer with "packed" options.
+    /// \param options options collection with actual data and offsets.
+    ///
+    /// \throw isc::Unexpected if options update failed.
+    static void packOptions(const dhcp::OptionBuffer& in_buffer,
+                            const dhcp::Option::OptionCollection& options,
+                            util::OutputBuffer& out_buffer);
+
+    /// \brief Reads contents of specified options from buffer.
+    ///
+    /// The method reads options data from the input buffer
+    /// and stores it in options objects. Offsets of the options
+    /// must be specified (see \ref LocalizedOption to find out how to specify
+    /// the option offset).
+    /// This private method is called by \ref PktTransform::unpack.
+    ///
+    /// \note This method iterates through all options in an
+    /// options collection, checks the offset of the option
+    /// in input buffer and reads data from the buffer to
+    /// update the option's buffer. If the provided options collection
+    /// is empty, a call to this method will have no effect.
+    ///
+    /// \param universe universe used, V4 or V6
+    /// \param in_buffer input buffer to be parsed.
+    /// \param options oprions collection with their offsets
+    /// in input buffer specified.
+    ///
+    /// \throw isc::Unexpected if options unpack failed.
+    static void unpackOptions(const dhcp::OptionBuffer& in_buffer,
+                              const dhcp::Option::OptionCollection& options);
+};
+
+} // namespace perfdhcp
+} // namespace isc
+
+#endif // __PKT_TRANSFORM_H

tests/tools/perfdhcp/tests/Makefile.am

@@ -18,14 +18,28 @@ if HAVE_GTEST
 TESTS += run_unittests
 run_unittests_SOURCES  = run_unittests.cc
 run_unittests_SOURCES += command_options_unittest.cc
+run_unittests_SOURCES += perf_pkt6_unittest.cc
+run_unittests_SOURCES += perf_pkt4_unittest.cc
+run_unittests_SOURCES += localized_option_unittest.cc
 run_unittests_SOURCES += $(top_builddir)/tests/tools/perfdhcp/command_options.cc
+run_unittests_SOURCES += $(top_builddir)/tests/tools/perfdhcp/pkt_transform.cc
+run_unittests_SOURCES += $(top_builddir)/tests/tools/perfdhcp/perf_pkt6.cc
+run_unittests_SOURCES += $(top_builddir)/tests/tools/perfdhcp/perf_pkt4.cc
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
 
+if USE_CLANGPP
+# Disable unused parameter warning caused by some of the
+# Boost headers when compiling with clang.
+run_unittests_CXXFLAGS = -Wno-unused-parameter
+endif
+
 run_unittests_LDADD  = $(GTEST_LDADD)
 run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
+run_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libdhcp++.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 endif
 

tests/tools/perfdhcp/tests/command_options_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

tests/tools/perfdhcp/tests/localized_option_unittest.cc

@@ -0,0 +1,48 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+#include <gtest/gtest.h>
+
+#include <dhcp/option.h>
+#include <dhcp/dhcp6.h>
+
+#include "../localized_option.h"
+
+using namespace std;
+using namespace isc;
+using namespace isc::dhcp;
+using namespace isc::perfdhcp;
+
+namespace {
+
+TEST(LocalizedOptionTest, Constructor) {
+    OptionBuffer opt_buf;
+    // Create option with default offset.
+    boost::scoped_ptr<LocalizedOption> opt1(new LocalizedOption(Option::V6,
+                                                                D6O_CLIENTID,
+                                                                opt_buf));
+    EXPECT_EQ(Option::V6, opt1->getUniverse());
+    EXPECT_EQ(D6O_CLIENTID, opt1->getType());
+    EXPECT_EQ(0, opt1->getOffset());
+
+    // Create option with non-default offset.
+    boost::scoped_ptr<LocalizedOption> opt2(new LocalizedOption(Option::V6,
+                                                                D6O_CLIENTID,
+                                                                opt_buf,
+                                                                40));
+    EXPECT_EQ(40, opt2->getOffset());
+}
+
+}

tests/tools/perfdhcp/tests/perf_pkt4_unittest.cc

@@ -0,0 +1,384 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+#include <iostream>
+#include <sstream>
+#include <arpa/inet.h>
+#include <gtest/gtest.h>
+
+#include <asiolink/io_address.h>
+#include <dhcp/option.h>
+#include <dhcp/dhcp4.h>
+
+#include "../localized_option.h"
+#include "../perf_pkt4.h"
+
+using namespace std;
+using namespace isc;
+using namespace isc::asiolink;
+using namespace isc::dhcp;
+using namespace isc::perfdhcp;
+
+typedef PerfPkt4::LocalizedOptionPtr LocalizedOptionPtr;
+
+namespace {
+
+// A dummy MAC address, padded with 0s
+const uint8_t dummyChaddr[16] = {0, 1, 2, 3, 4, 5, 0, 0,
+                                 0, 0, 0, 0, 0, 0, 0, 0 };
+
+// Let's use some creative test content here (128 chars + \0)
+const uint8_t dummyFile[] = "Lorem ipsum dolor sit amet, consectetur "
+    "adipiscing elit. Proin mollis placerat metus, at "
+    "lacinia orci ornare vitae. Mauris amet.";
+
+// Yet another type of test content (64 chars + \0)
+const uint8_t dummySname[] = "Lorem ipsum dolor sit amet, consectetur "
+    "adipiscing elit posuere.";
+
+class PerfPkt4Test : public ::testing::Test {
+public:
+    PerfPkt4Test() {
+    }
+
+    /// \brief Returns buffer with sample DHCPDISCOVER message.
+    ///
+    /// This method creates buffer containing on-wire data of
+    /// DHCPDICOSVER message. This buffer is used by tests below
+    /// to create DHCPv4 test packets.
+    ///
+    /// \return vector containing on-wire data
+    std::vector<uint8_t>& capture() {
+
+        // That is only part of the header. It contains all "short" fields,
+        // larger fields are constructed separately.
+        uint8_t hdr[] = {
+            1, 6, 6, 13,            // op, htype, hlen, hops,
+            0x12, 0x34, 0x56, 0x78, // transaction-id
+            0, 42, 0x80, 0x00,      // 42 secs, BROADCAST flags
+            192, 0, 2, 1,           // ciaddr
+            1, 2, 3, 4,             // yiaddr
+            192, 0, 2, 255,         // siaddr
+            255, 255, 255, 255,     // giaddr
+        };
+
+        uint8_t v4Opts[] = {
+            DHO_HOST_NAME, 3, 0,   1,  2,  // Host name option.
+            DHO_BOOT_SIZE, 3, 10, 11, 12,  // Boot file size option
+            DHO_MERIT_DUMP, 3, 20, 21, 22, // Merit dump file
+            DHO_DHCP_MESSAGE_TYPE, 1, 1,   // DHCP message type.
+            128, 3, 30, 31, 32,
+            254, 3, 40, 41, 42,
+        };
+
+        // Initialize the vector with the header fields defined above.
+        static std::vector<uint8_t> buf(hdr, hdr + sizeof(hdr));
+
+        // If this is a first call to this function. Initialize
+        // remaining data.
+        if (buf.size() == sizeof(hdr)) {
+
+            // Append the large header fields.
+            std::copy(dummyChaddr, dummyChaddr + Pkt4::MAX_CHADDR_LEN,
+                      back_inserter(buf));
+            std::copy(dummySname, dummySname + Pkt4::MAX_SNAME_LEN,
+                      back_inserter(buf));
+            std::copy(dummyFile, dummyFile + Pkt4::MAX_FILE_LEN,
+                      back_inserter(buf));
+
+            // Append magic cookie.
+            buf.push_back(0x63);
+            buf.push_back(0x82);
+            buf.push_back(0x53);
+            buf.push_back(0x63);
+
+            // Append options.
+            std::copy(v4Opts, v4Opts + sizeof(v4Opts), back_inserter(buf));
+        }
+        return buf;
+    }
+};
+
+TEST_F(PerfPkt4Test, Constructor) {
+    // Initialize some dummy payload.
+    uint8_t data[250];
+    for (int i = 0; i < 250; ++i) {
+        data[i] = i;
+    }
+
+    // Test constructor to be used for incoming messages.
+    // Use default (1) offset value and don't specify transaction id.
+    const size_t offset_transid[] = { 1, 10 };
+    boost::scoped_ptr<PerfPkt4> pkt1(new PerfPkt4(data,
+                                                  sizeof(data),
+                                                  offset_transid[0]));
+    EXPECT_EQ(1, pkt1->getTransidOffset());
+
+    // Test constructor to be used for outgoing messages.
+    // Use non-zero offset and specify transaction id.
+    const uint32_t transid = 0x010203;
+    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(data, sizeof(data),
+                                                  offset_transid[1],
+                                                  transid));
+    EXPECT_EQ(transid, pkt2->getTransid());
+    EXPECT_EQ(offset_transid[1], pkt2->getTransidOffset());
+
+    // Test default constructor. Transaction id offset is expected to be 1.
+    boost::scoped_ptr<PerfPkt4> pkt3(new PerfPkt4(data, sizeof(data)));
+    EXPECT_EQ(1, pkt3->getTransidOffset());
+}
+
+TEST_F(PerfPkt4Test, RawPack) {
+    // Create new packet.
+    std::vector<uint8_t> buf = capture();
+    boost::scoped_ptr<PerfPkt4> pkt(new PerfPkt4(&buf[0], buf.size()));
+
+    // Initialize options data.
+    uint8_t buf_hostname[] = { DHO_HOST_NAME, 3, 4, 5, 6 };
+    uint8_t buf_boot_filesize[] = { DHO_BOOT_SIZE, 3, 1, 2, 3 };
+    OptionBuffer vec_hostname(buf_hostname + 2,
+                              buf_hostname + sizeof(buf_hostname));
+    OptionBuffer vec_boot_filesize(buf_boot_filesize + 2,
+                                   buf_boot_filesize + sizeof(buf_hostname));
+
+    // Create options objects.
+    const size_t offset_hostname = 240;
+    LocalizedOptionPtr pkt_hostname(new LocalizedOption(Option::V4,
+                                                        DHO_HOST_NAME,
+                                                        vec_hostname,
+                                                        offset_hostname));
+    const size_t offset_boot_filesize = 245;
+    LocalizedOptionPtr pkt_boot_filesize(new LocalizedOption(Option::V4,
+                                                             DHO_BOOT_SIZE,
+                                                             vec_boot_filesize,
+                                                             offset_boot_filesize));
+
+    // Try to add options to packet.
+    ASSERT_NO_THROW(pkt->addOption(pkt_boot_filesize));
+    ASSERT_NO_THROW(pkt->addOption(pkt_hostname));
+
+    // We have valid options addedwith valid offsets so
+    // pack operation should succeed.
+    ASSERT_TRUE(pkt->rawPack());
+
+    // Buffer should now contain new values of DHO_HOST_NAME and
+    // DHO_BOOT_SIZE options.
+    util::OutputBuffer pkt_output = pkt->getBuffer();
+    ASSERT_EQ(buf.size(), pkt_output.getLength());
+    const uint8_t* out_buf_data =
+        static_cast<const uint8_t*>(pkt_output.getData());
+
+    // Check if options we read from buffer is valid.
+    EXPECT_EQ(0, memcmp(buf_hostname,
+                        out_buf_data + offset_hostname,
+                        sizeof(buf_hostname)));
+    EXPECT_EQ(0, memcmp(buf_boot_filesize,
+                        out_buf_data + offset_boot_filesize,
+                        sizeof(buf_boot_filesize)));
+}
+
+TEST_F(PerfPkt4Test, RawUnpack) {
+    // Create new packet.
+    std::vector<uint8_t> buf = capture();
+    boost::scoped_ptr<PerfPkt4> pkt(new PerfPkt4(&buf[0], buf.size()));
+
+    // Create options (existing in the packet) and specify their offsets.
+    const size_t offset_merit = 250;
+    LocalizedOptionPtr opt_merit(new LocalizedOption(Option::V4,
+                                                     DHO_MERIT_DUMP,
+                                                     OptionBuffer(),
+                                                     offset_merit));
+
+    const size_t  offset_msg_type = 255;
+    LocalizedOptionPtr opt_msg_type(new LocalizedOption(Option::V4,
+                                                        DHO_DHCP_MESSAGE_TYPE,
+                                                        OptionBuffer(),
+                                                        offset_msg_type));
+    // Addition should be successful
+    ASSERT_NO_THROW(pkt->addOption(opt_merit));
+    ASSERT_NO_THROW(pkt->addOption(opt_msg_type));
+
+    // Option fit to packet boundaries and offsets are valid,
+    // so this should unpack successfully.
+    ASSERT_TRUE(pkt->rawUnpack());
+
+    // At this point we should have updated options data (read from buffer).
+    // Let's try to retrieve them.
+    opt_merit = boost::dynamic_pointer_cast<LocalizedOption>
+        (pkt->getOption(DHO_MERIT_DUMP));
+    opt_msg_type = boost::dynamic_pointer_cast<LocalizedOption>
+        (pkt->getOption(DHO_DHCP_MESSAGE_TYPE));
+    ASSERT_TRUE(opt_merit);
+    ASSERT_TRUE(opt_msg_type);
+
+    // Get first option payload.
+    OptionBuffer opt_merit_data = opt_merit->getData();
+
+    // Define reference data.
+    uint8_t buf_merit[] = { 20, 21, 22 };
+
+    // Validate first option data.
+    ASSERT_EQ(sizeof(buf_merit), opt_merit_data.size());
+    EXPECT_TRUE(std::equal(opt_merit_data.begin(),
+                           opt_merit_data.end(),
+                           buf_merit));
+
+    // Get second option payload.
+    OptionBuffer opt_msg_type_data = opt_msg_type->getData();
+
+    // Expect one byte of message type payload.
+    ASSERT_EQ(1, opt_msg_type_data.size());
+    EXPECT_EQ(1, opt_msg_type_data[0]);
+}
+
+TEST_F(PerfPkt4Test, InvalidOptions) {
+    // Create new packet.
+    std::vector<uint8_t> buf = capture();
+    boost::scoped_ptr<PerfPkt4> pkt1(new PerfPkt4(&buf[0], buf.size()));
+
+    // Create option with invalid offset.
+    // This option is at offset 250 (not 251).
+    const size_t offset_merit = 251;
+    LocalizedOptionPtr opt_merit(new LocalizedOption(Option::V4,
+                                                     DHO_MERIT_DUMP,
+                                                     OptionBuffer(),
+                                                     offset_merit));
+    ASSERT_NO_THROW(pkt1->addOption(opt_merit));
+
+    cout << "Testing unpack of invalid options. "
+         << "This may produce spurious errors." << endl;
+
+    // Unpack is expected to fail because it is supposed to read
+    // option type from buffer and match it with DHO_MERIT_DUMP.
+    // It will not match because option is shifted by on byte.
+    ASSERT_FALSE(pkt1->rawUnpack());
+
+    // Create another packet.
+    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(&buf[0], buf.size()));
+
+    // Create DHO_DHCP_MESSAGE_TYPE option that has the wrong offset.
+    // With this offset, option goes beyond packet size (268).
+    const size_t offset_msg_type = 266;
+    LocalizedOptionPtr opt_msg_type(new LocalizedOption(Option::V4,
+                                                        DHO_DHCP_MESSAGE_TYPE,
+                                                        OptionBuffer(1, 2),
+                                                        offset_msg_type));
+    // Adding option is expected to be successful because no
+    // offset validation takes place at this point.
+    ASSERT_NO_THROW(pkt2->addOption(opt_msg_type));
+
+    // This is expected to fail because option is out of bounds.
+    ASSERT_FALSE(pkt2->rawPack());
+}
+
+TEST_F(PerfPkt4Test, TruncatedPacket) {
+    // Get the whole packet and truncate it to 249 bytes.
+    std::vector<uint8_t> buf = capture();
+    buf.resize(249);
+    boost::scoped_ptr<PerfPkt4> pkt(new PerfPkt4(&buf[0], buf.size()));
+
+    // Option DHO_BOOT_SIZE is now truncated because whole packet
+    // is truncated. This option ends at 249 while last index of
+    // truncated packet is now 248.
+    const size_t offset_boot_filesize = 245;
+    LocalizedOptionPtr opt_boot_filesize(new LocalizedOption(Option::V4,
+                                                             DHO_BOOT_SIZE,
+                                                             OptionBuffer(3, 1),
+                                                             offset_boot_filesize));
+    ASSERT_NO_THROW(pkt->addOption(opt_boot_filesize));
+
+    cout << "Testing pack and unpack of options in truncated "
+         << "packet. This may produce spurious errors." << endl;
+
+    // Both pack and unpack are expected to fail because
+    // added option is out of bounds.
+    EXPECT_FALSE(pkt->rawUnpack());
+    EXPECT_FALSE(pkt->rawPack());
+}
+
+TEST_F(PerfPkt4Test, PackTransactionId) {
+    // Create dummy packet that consists of zeros.
+    std::vector<uint8_t> buf(268, 0);
+
+    const size_t offset_transid[] = { 10, 265 };
+    const uint32_t transid = 0x0102;
+    // Initialize transaction id 0x00000102 at offset 10.
+    boost::scoped_ptr<PerfPkt4> pkt1(new PerfPkt4(&buf[0], buf.size(),
+                                                  offset_transid[0],
+                                                  transid));
+
+    // Pack will inject transaction id at offset 10 into the
+    // packet buffer.
+    ASSERT_TRUE(pkt1->rawPack());
+
+    // Get packet's output buffer and make sure it has valid size.
+    util::OutputBuffer out_buf = pkt1->getBuffer();
+    ASSERT_EQ(buf.size(), out_buf.getLength());
+    const uint8_t *out_buf_data =
+        static_cast<const uint8_t*>(out_buf.getData());
+
+    // Initialize reference data for transaction id.
+    const uint8_t ref_data[] = { 0, 0, 1, 2 };
+
+    // Expect that reference transaction id matches what we have
+    // read from buffer.
+    EXPECT_EQ(0, memcmp(ref_data, out_buf_data + offset_transid[0], 4));
+
+    cout << "Testing pack with invalid transaction id offset. "
+         << "This may produce spurious errors" << endl;
+
+    // Create packet with invalid transaction id offset.
+    // Packet length is 268, transaction id is 4 bytes long so last byte of
+    // transaction id is out of bounds.
+    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(&buf[0], buf.size(),
+                                                  offset_transid[1],
+                                                  transid));
+    EXPECT_FALSE(pkt2->rawPack());
+}
+
+TEST_F(PerfPkt4Test, UnpackTransactionId) {
+    // Initialize packet data, lebgth 268, zeros only.
+    std::vector<uint8_t> in_data(268, 0);
+
+    // Assume that transaction id is at offset 100.
+    // Fill 4 bytes at offset 100 with dummy transaction id.
+    for (int i = 100; i < 104; ++i) {
+        in_data[i] = i - 99;
+    }
+
+    // Create packet from initialized buffer.
+    const size_t offset_transid[] = { 100, 270 };
+    boost::scoped_ptr<PerfPkt4> pkt1(new PerfPkt4(&in_data[0],
+                                                  in_data.size(),
+                                                  offset_transid[0]));
+    ASSERT_TRUE(pkt1->rawUnpack());
+
+    // Get unpacked transaction id and compare with reference.
+    EXPECT_EQ(0x01020304, pkt1->getTransid());
+
+    // Create packet with transaction id at invalid offset.
+    boost::scoped_ptr<PerfPkt4> pkt2(new PerfPkt4(&in_data[0],
+                                                  in_data.size(),
+                                                  offset_transid[1]));
+
+    cout << "Testing unpack of transaction id at invalid offset. "
+         << "This may produce spurious errors." << endl;
+
+    // Unpack is supposed to fail because transaction id is at
+    // out of bounds offset.
+    EXPECT_FALSE(pkt2->rawUnpack());
+}
+
+}

tests/tools/perfdhcp/tests/perf_pkt6_unittest.cc

@@ -0,0 +1,327 @@
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+#include <iostream>
+#include <sstream>
+#include <arpa/inet.h>
+#include <gtest/gtest.h>
+
+#include <asiolink/io_address.h>
+#include <dhcp/option.h>
+#include <dhcp/dhcp6.h>
+
+#include "../localized_option.h"
+#include "../perf_pkt6.h"
+
+using namespace std;
+using namespace isc;
+using namespace isc::dhcp;
+using namespace isc::perfdhcp;
+
+typedef PerfPkt6::LocalizedOptionPtr LocalizedOptionPtr;
+
+namespace {
+
+class PerfPkt6Test : public ::testing::Test {
+public:
+    PerfPkt6Test() {
+    }
+
+    /// \brief Returns captured SOLICIT packet.
+    ///
+    /// Captured SOLICIT packet with transid=0x3d79fb and options: client-id,
+    /// in_na, dns-server, elapsed-time, option-request
+    /// This code was autogenerated
+    /// (see src/bin/dhcp6/tests/iface_mgr_unittest.c),
+    /// but we spent some time to make is less ugly than it used to be.
+    ///
+    /// \return pointer to Pkt6 that represents received SOLICIT
+    PerfPkt6* capture() {
+        uint8_t data[98];
+        data[0]  = 1;
+        data[1]  = 1;       data[2]  = 2;     data[3] = 3;      data[4]  = 0;
+        data[5]  = 1;       data[6]  = 0;     data[7] = 14;     data[8]  = 0;
+        data[9]  = 1;       data[10] = 0;     data[11] = 1;     data[12] = 21;
+        data[13] = 158;     data[14] = 60;    data[15] = 22;    data[16] = 0;
+        data[17] = 30;      data[18] = 140;   data[19] = 155;   data[20] = 115;
+        data[21] = 73;      data[22] = 0;     data[23] = 3;     data[24] = 0;
+        data[25] = 40;      data[26] = 0;     data[27] = 0;     data[28] = 0;
+        data[29] = 1;       data[30] = 255;   data[31] = 255;   data[32] = 255;
+        data[33] = 255;     data[34] = 255;   data[35] = 255;   data[36] = 255;
+        data[37] = 255;     data[38] = 0;     data[39] = 5;     data[40] = 0;
+        data[41] = 24;      data[42] = 32;    data[43] = 1;     data[44] = 13;
+        data[45] = 184;     data[46] = 0;     data[47] = 1;     data[48] = 0;
+        data[49] = 0;       data[50] = 0;     data[51] = 0;     data[52] = 0;
+        data[53] = 0;       data[54] = 0;     data[55] = 0;     data[56] = 18;
+        data[57] = 52;      data[58] = 255;   data[59] = 255;   data[60] = 255;
+        data[61] = 255;     data[62] = 255;   data[63] = 255;   data[64] = 255;
+        data[65] = 255;     data[66] = 0;     data[67] = 23;    data[68] = 0;
+        data[69] = 16;      data[70] = 32;    data[71] = 1;     data[72] = 13;
+        data[73] = 184;     data[74] = 0;     data[75] = 1;     data[76] = 0;
+        data[77] = 0;       data[78] = 0;     data[79] = 0;     data[80] = 0;
+        data[81] = 0;       data[82] = 0;     data[83] = 0;     data[84] = 221;
+        data[85] = 221;     data[86] = 0;     data[87] = 8;     data[88] = 0;
+        data[89] = 2;       data[90] = 0;     data[91] = 100;   data[92] = 0;
+        data[93] = 6;       data[94] = 0;     data[95] = 2;     data[96] = 0;
+        data[97] = 23;
+
+        PerfPkt6* pkt = new PerfPkt6(data, sizeof(data));
+
+        return (pkt);
+    }
+
+    /// \brief Returns truncated SOLICIT packet.
+    ///
+    /// Returns truncated SOLICIT packet which will be used for
+    /// negative tests: e.g. pack options out of packet.
+    ///
+    /// \return pointer to Pkt6 that represents truncated SOLICIT
+    PerfPkt6* captureTruncated() {
+        uint8_t data[17];
+        data[0]  = 1;
+        data[1]  = 1;       data[2]  = 2;     data[3] = 3;      data[4]  = 0;
+        data[5]  = 1;       data[6]  = 0;     data[7] = 14;     data[8]  = 0;
+        data[9]  = 1;       data[10] = 0;     data[11] = 1;     data[12] = 21;
+        data[13] = 158;     data[14] = 60;    data[15] = 22;    data[16] = 0;
+
+        PerfPkt6* pkt = new PerfPkt6(data, sizeof(data));
+
+        return (pkt);
+    }
+
+
+};
+
+TEST_F(PerfPkt6Test, Constructor) {
+    // Data to be used to create packet.
+    uint8_t data[] = { 0, 1, 2, 3, 4, 5 };
+
+    // Test constructor to be used for incoming messages.
+    // Use default (1) offset value and don't specify transaction id.
+    boost::scoped_ptr<PerfPkt6> pkt1(new PerfPkt6(data, sizeof(data)));
+    EXPECT_EQ(sizeof(data), pkt1->getData().size());
+    EXPECT_EQ(0, memcmp(&pkt1->getData()[0], data, sizeof(data)));
+    EXPECT_EQ(1, pkt1->getTransidOffset());
+
+    // Test constructor to be used for outgoing messages.
+    // Use non-zero offset and specify transaction id.
+    const size_t offset_transid = 10;
+    const uint32_t transid = 0x010203;
+    boost::scoped_ptr<PerfPkt6> pkt2(new PerfPkt6(data, sizeof(data),
+                                                  offset_transid, transid));
+    EXPECT_EQ(sizeof(data), pkt2->getData().size());
+    EXPECT_EQ(0, memcmp(&pkt2->getData()[0], data, sizeof(data)));
+    EXPECT_EQ(0x010203, pkt2->getTransid());
+    EXPECT_EQ(10, pkt2->getTransidOffset());
+}
+
+TEST_F(PerfPkt6Test, RawPackUnpack) {
+    // Create first packet.
+    boost::scoped_ptr<PerfPkt6> pkt1(capture());
+
+    // Create some input buffers to initialize options.
+    uint8_t buf_elapsed_time[] = { 1, 1 };
+    uint8_t buf_duid[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14 };
+
+    // Create options.
+    const size_t offset_elapsed_time = 86;
+    OptionBuffer vec_elapsed_time(buf_elapsed_time,
+                                  buf_elapsed_time + sizeof(buf_elapsed_time));
+    LocalizedOptionPtr pkt1_elapsed_time(new LocalizedOption(Option::V6,
+                                                             D6O_ELAPSED_TIME,
+                                                             vec_elapsed_time,
+                                                             offset_elapsed_time));
+    const size_t offset_duid = 4;
+    OptionBuffer vec_duid(buf_duid, buf_duid + sizeof(buf_duid));
+    LocalizedOptionPtr pkt1_duid(new LocalizedOption(Option::V6,
+                                                     D6O_CLIENTID,
+                                                     vec_duid,
+                                                     offset_duid));
+
+    // Add option to packet and create on-wire format from added options.
+    // Contents of options will override contents of packet buffer.
+    ASSERT_NO_THROW(pkt1->addOption(pkt1_elapsed_time));
+    ASSERT_NO_THROW(pkt1->addOption(pkt1_duid));
+    ASSERT_TRUE(pkt1->rawPack());
+
+    // Reset so as we can reuse them for another packet.
+    vec_elapsed_time.clear();
+    vec_duid.clear();
+
+    // Get output buffer from packet 1 to create new packet
+    // that will be later validated.
+    util::OutputBuffer pkt1_output = pkt1->getBuffer();
+    ASSERT_EQ(pkt1_output.getLength(), pkt1->getData().size());
+    const uint8_t* pkt1_output_data = static_cast<const uint8_t*>
+        (pkt1_output.getData());
+    boost::scoped_ptr<PerfPkt6> pkt2(new PerfPkt6(pkt1_output_data,
+                                                  pkt1_output.getLength()));
+
+    // Create objects specifying options offset in a packet.
+    // Offsets will inform pkt2 object where to read data from.
+    LocalizedOptionPtr pkt2_elapsed_time(new LocalizedOption(Option::V6,
+                                                             D6O_ELAPSED_TIME,
+                                                             vec_elapsed_time,
+                                                             offset_elapsed_time));
+    LocalizedOptionPtr pkt2_duid(new LocalizedOption(Option::V6,
+                                                     D6O_CLIENTID,
+                                                     vec_duid,
+                                                     offset_duid));
+    // Add options to packet to pass their offsets.
+    pkt2->addOption(pkt2_elapsed_time);
+    pkt2->addOption(pkt2_duid);
+
+    // Unpack: get relevant parts of buffer data into option objects.
+    ASSERT_TRUE(pkt2->rawUnpack());
+
+    // Once option data is stored in options objects we pull it out.
+    pkt2_elapsed_time = boost::dynamic_pointer_cast<LocalizedOption>
+        (pkt2->getOption(D6O_ELAPSED_TIME));
+    pkt2_duid = boost::dynamic_pointer_cast<LocalizedOption>
+        (pkt2->getOption(D6O_CLIENTID));
+
+    // Check if options are present. They have to be there since
+    // we have added them ourselfs.
+    ASSERT_TRUE(pkt2_elapsed_time);
+    ASSERT_TRUE(pkt2_duid);
+
+    // Expecting option contents be the same as original.
+    OptionBuffer pkt2_elapsed_time_data = pkt2_elapsed_time->getData();
+    OptionBuffer pkt2_duid_data = pkt2_duid->getData();
+    EXPECT_EQ(0x0101, pkt2_elapsed_time->getUint16());
+    EXPECT_TRUE(std::equal(pkt2_duid_data.begin(),
+                           pkt2_duid_data.end(),
+                           buf_duid));
+}
+
+TEST_F(PerfPkt6Test, InvalidOptions) {
+    // Create packet.
+    boost::scoped_ptr<PerfPkt6> pkt1(capture());
+    OptionBuffer vec_server_id;
+    vec_server_id.resize(10);
+    // Testing invalid offset of the option (greater than packet size)
+    const size_t offset_serverid[] = { 150, 85 };
+    LocalizedOptionPtr pkt1_serverid(new LocalizedOption(Option::V6,
+                                                         D6O_SERVERID,
+                                                         vec_server_id,
+                                                         offset_serverid[0]));
+    pkt1->addOption(pkt1_serverid);
+    // Pack has to fail due to invalid offset.
+    EXPECT_FALSE(pkt1->rawPack());
+
+    // Create packet.
+    boost::scoped_ptr<PerfPkt6> pkt2(capture());
+    // Testing offset of the option (lower than pakcet size but
+    // tail of the option out of bounds).
+    LocalizedOptionPtr pkt2_serverid(new LocalizedOption(Option::V6,
+                                                         D6O_SERVERID,
+                                                         vec_server_id,
+                                                         offset_serverid[1]));
+    pkt2->addOption(pkt2_serverid);
+    // Pack must fail due to invalid offset.
+    EXPECT_FALSE(pkt2->rawPack());
+}
+
+
+TEST_F(PerfPkt6Test, TruncatedPacket) {
+    cout << "Testing parsing options from truncated packet."
+         << "This may produce spurious errors" << endl;
+
+    // Create truncated (in the middle of duid options)
+    boost::scoped_ptr<PerfPkt6> pkt1(captureTruncated());
+    OptionBuffer vec_duid;
+    vec_duid.resize(30);
+    const size_t offset_duid = 4;
+    LocalizedOptionPtr pkt1_duid(new LocalizedOption(Option::V6,
+                                                     D6O_CLIENTID,
+                                                     vec_duid,
+                                                     offset_duid));
+    pkt1->addOption(pkt1_duid);
+    // Pack/unpack must fail because length of the option read from buffer
+    // will extend over the actual packet length.
+    EXPECT_FALSE(pkt1->rawUnpack());
+    EXPECT_FALSE(pkt1->rawPack());
+}
+
+TEST_F(PerfPkt6Test, PackTransactionId) {
+    uint8_t data[100];
+    memset(&data, 0, sizeof(data));
+
+    const size_t offset_transid[] = { 50, 100 };
+    const uint32_t transid = 0x010203;
+
+    // Create dummy packet that is simply filled with zeros.
+    boost::scoped_ptr<PerfPkt6> pkt1(new PerfPkt6(data,
+                                                  sizeof(data),
+                                                  offset_transid[0],
+                                                  transid));
+
+    // Reference data are non zero so we can detect them in dummy packet.
+    uint8_t ref_data[3] = { 1, 2, 3 };
+
+    // This will store given transaction id in the packet data at
+    // offset of 50.
+    ASSERT_TRUE(pkt1->rawPack());
+
+    // Get the output buffer so we can validate it.
+    util::OutputBuffer out_buf = pkt1->getBuffer();
+    ASSERT_EQ(sizeof(data), out_buf.getLength());
+    const uint8_t *out_buf_data = static_cast<const uint8_t*>
+        (out_buf.getData());
+
+    // Validate transaction id.
+    EXPECT_EQ(0, memcmp(out_buf_data + offset_transid[0], ref_data, 3));
+
+
+    // Out of bounds transaction id offset.
+    boost::scoped_ptr<PerfPkt6> pkt2(new PerfPkt6(data,
+                                                  sizeof(data),
+                                                  offset_transid[1],
+                                                  transid));
+    cout << "Testing out of bounds offset. "
+        "This may produce spurious errors ..." << endl;
+    EXPECT_FALSE(pkt2->rawPack());
+}
+
+TEST_F(PerfPkt6Test, UnpackTransactionId) {
+    // Initialize data for dummy packet (zeros only).
+    uint8_t data[100] = { 0 };
+
+    // Generate transaction id = 0x010203 and inject at offset = 50.
+    for (int i = 50; i <  53; ++i) {
+        data[i] = i - 49;
+    }
+    // Create packet and point out that transaction id is at offset 50.
+    const size_t offset_transid[] = { 50, 300 };
+    boost::scoped_ptr<PerfPkt6> pkt1(new PerfPkt6(data,
+                                                  sizeof(data),
+                                                  offset_transid[0]));
+
+    // Get transaction id out of buffer and store in class member.
+    ASSERT_TRUE(pkt1->rawUnpack());
+    // Test value of transaction id.
+    EXPECT_EQ(0x010203, pkt1->getTransid());
+
+    // Out of bounds transaction id offset.
+    boost::scoped_ptr<PerfPkt6> pkt2(new PerfPkt6(data,
+                                                  sizeof(data),
+                                                  offset_transid[1]));
+    cout << "Testing out of bounds offset. "
+        "This may produce spurious errors ..." << endl;
+    EXPECT_FALSE(pkt2->rawUnpack());
+
+}
+
+}