[master] Merge branch 'trac4121' (RAI option packed last)

# Conflicts:
#	src/lib/dhcp/libdhcp++.cc
#	src/lib/dhcp/libdhcp++.h

src/lib/dhcp/libdhcp++.cc

@@ -744,11 +744,42 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
     return (offset);
 }
 
+void
+LibDHCP::packOptions4(isc::util::OutputBuffer& buf,
+                     const OptionCollection& options) {
+    OptionPtr agent;
+    OptionPtr end;
+    for (OptionCollection::const_iterator it = options.begin();
+         it != options.end(); ++it) {
+
+        // RAI and END options must be last.
+        switch (it->first) {
+            case DHO_DHCP_AGENT_OPTIONS:
+                agent = it->second;
+                break;
+            case DHO_END:
+                end = it->second;
+                break;
+            default:
+                it->second->pack(buf);
+                break;
+        }
+    }
+
+    // Add the RAI option if it exists.
+    if (agent) {
+       agent->pack(buf);
+    }
 
+    // And at the end the END option.
+    if (end)  {
+       end->pack(buf);
+    }
+}
 
 void
-LibDHCP::packOptions(isc::util::OutputBuffer& buf,
-                     const OptionCollection& options) {
+LibDHCP::packOptions6(isc::util::OutputBuffer& buf,
+                      const OptionCollection& options) {
     for (OptionCollection::const_iterator it = options.begin();
          it != options.end(); ++it) {
         it->second->pack(buf);

src/lib/dhcp/libdhcp++.h

@@ -155,7 +155,7 @@ public:
                                               uint16_t type,
                                               const OptionBuffer& buf);
 
-    /// @brief Stores options in a buffer.
+    /// @brief Stores DHCPv4 options in a buffer.
     ///
     /// Stores all options defined in options containers in a on-wire
     /// format in output buffer specified by buf.
@@ -164,10 +164,30 @@ public:
     /// may be different reasons (option too large, option malformed,
     /// too many options etc.)
     ///
+    /// This is v4 specific version, which stores Relay Agent Information
+    /// option and END options last.
+    ///
+    /// @param buf output buffer (assembled options will be stored here)
+    /// @param options collection of options to store to
+    static void packOptions4(isc::util::OutputBuffer& buf,
+                             const isc::dhcp::OptionCollection& options);
+
+    /// @brief Stores DHCPv6 options in a buffer.
+    ///
+    /// Stores all options defined in options containers in a on-wire
+    /// format in output buffer specified by buf.
+    ///
+    /// May throw different exceptions if option assembly fails. There
+    /// may be different reasons (option too large, option malformed,
+    /// too many options etc.)
+    ///
+    /// Currently there's no special logic in it. Options are stored in
+    /// the order of their option codes.
+    ///
     /// @param buf output buffer (assembled options will be stored here)
     /// @param options collection of options to store to
-    static void packOptions(isc::util::OutputBuffer& buf,
-                            const isc::dhcp::OptionCollection& options);
+    static void packOptions6(isc::util::OutputBuffer& buf,
+                             const isc::dhcp::OptionCollection& options);
 
     /// @brief Parses provided buffer as DHCPv6 options and creates
     /// Option objects.

src/lib/dhcp/option.cc

@@ -118,7 +118,16 @@ Option::packHeader(isc::util::OutputBuffer& buf) {
 
 void
 Option::packOptions(isc::util::OutputBuffer& buf) {
-    LibDHCP::packOptions(buf, options_);
+    switch (universe_) {
+    case V4:
+        LibDHCP::packOptions4(buf, options_);
+        return;
+    case V6:
+        LibDHCP::packOptions6(buf, options_);
+        return;
+    default:
+        isc_throw(isc::BadValue, "Invalid universe type " << universe_);
+    }
 }
 
 void Option::unpack(OptionBufferConstIter begin,

src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -530,6 +530,7 @@ static uint8_t v4_opts[] = {
     0x01, 0x02, 0x03, 0x00  // Vendor Specific Information continued
 };
 
+// This test verifies that pack options for v4 is working correctly.
 TEST_F(LibDhcpTest, packOptions4) {
 
     vector<uint8_t> payload[5];
@@ -595,6 +596,41 @@ TEST_F(LibDhcpTest, packOptions4) {
     EXPECT_EQ(0, memcmp(v4_opts, buf.getData(), sizeof(v4_opts)));
 }
 
+// This test verifies that pack options for v4 is working correctly
+// and RAI option is packed last.
+TEST_F(LibDhcpTest, packOptions4Order) {
+
+    uint8_t expected[] = {
+        12,  3, 0,   1,  2, // Just a random option
+        99,  3, 10, 11, 12, // Another random option
+        82,  3, 20, 21, 22 // Relay Agent Info option
+    };
+
+    vector<uint8_t> payload[3];
+    for (unsigned i = 0; i < 3; i++) {
+        payload[i].resize(3);
+        payload[i][0] = i*10;
+        payload[i][1] = i*10+1;
+        payload[i][2] = i*10+2;
+    }
+
+    OptionPtr opt12(new Option(Option::V4, 12, payload[0]));
+    OptionPtr opt99(new Option(Option::V4, 99, payload[1]));
+    OptionPtr opt82(new Option(Option::V4, 82, payload[2]));
+
+    // Let's create options. They are added in 82,12,99, but the should be
+    // packed in 12,99,82 order (82, which is RAI, should go last)
+    isc::dhcp::OptionCollection opts;
+    opts.insert(make_pair(opt82->getType(), opt82));
+    opts.insert(make_pair(opt12->getType(), opt12));
+    opts.insert(make_pair(opt99->getType(), opt99));
+
+    OutputBuffer buf(100);
+    EXPECT_NO_THROW(LibDHCP::packOptions4(buf, opts));
+    ASSERT_EQ(buf.getLength(), sizeof(expected));
+    EXPECT_EQ(0, memcmp(expected, buf.getData(), sizeof(expected)));
+}
+
 TEST_F(LibDhcpTest, unpackOptions4) {
 
     vector<uint8_t> v4packed(v4_opts, v4_opts + sizeof(v4_opts));