Commit 34f40035 authored by Thomas Markwalder's avatar Thomas Markwalder

[master] kea-dhcp4 now emits message type option first in outbound packets

    Merge branch '363-13966-dhcp-option-start-sequence'
parents 3c926717 c6e2ee63
......@@ -787,14 +787,29 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
void
LibDHCP::packOptions4(isc::util::OutputBuffer& buf,
const OptionCollection& options) {
const OptionCollection& options,
bool top /* = false */) {
OptionPtr agent;
OptionPtr end;
// We only look for type when we're the top level
// call that starts packing for options for a packet.
// This way we avoid doing type logic in all ensuing
// recursive calls.
if (top) {
auto x = options.find(DHO_DHCP_MESSAGE_TYPE);
if (x != options.end()) {
x->second->pack(buf);
}
}
for (OptionCollection::const_iterator it = options.begin();
it != options.end(); ++it) {
// RAI and END options must be last.
// TYPE is already done, RAI and END options must be last.
switch (it->first) {
case DHO_DHCP_MESSAGE_TYPE:
break;
case DHO_DHCP_AGENT_OPTIONS:
agent = it->second;
break;
......
......@@ -178,13 +178,22 @@ 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.
/// This is v4 specific version, which stores DHCP message type first,
/// and the Relay Agent Information option and END options last. This
/// function is initially called to pack the options for a packet in
/// @ref Pkt4::pack(). That call leads to it being called recursively in
/// @ref Option::packOptions(). Thus the logic used to output the
/// message type should only be executed by the top-most. This is governed
/// by the paramater top, below.
///
/// @param buf output buffer (assembled options will be stored here)
/// @param options collection of options to store to
/// @param top indicates if this is the first call to pack the options.
/// When true logic to emit the message type first is executed. It
/// defaults to false.
static void packOptions4(isc::util::OutputBuffer& buf,
const isc::dhcp::OptionCollection& options);
const isc::dhcp::OptionCollection& options,
bool top = false);
/// @brief Stores DHCPv6 options in a buffer.
///
......
......@@ -135,7 +135,9 @@ Pkt4::pack() {
// write DHCP magic cookie
buffer_out_.writeUint32(DHCP_OPTIONS_COOKIE);
LibDHCP::packOptions4(buffer_out_, options_);
// Call packOptions4() with parameter,"top", true. This invokes
// logic to emit the message type option first.
LibDHCP::packOptions4(buffer_out_, options_, true);
// add END option that indicates end of options
// (End option is very simple, just a 255 octet)
......
......@@ -47,9 +47,9 @@ namespace {
/// variable length data so as there are no restrictions
/// on a length of their data.
static uint8_t v4_opts[] = {
53, 1, 2, // Message Type (required to not throw exception during unpack)
12, 3, 0, 1, 2, // Hostname
14, 3, 10, 11, 12, // Merit Dump File
53, 1, 2, // Message Type (required to not throw exception during unpack)
60, 3, 20, 21, 22, // Class Id
128, 3, 30, 31, 32, // Vendor specific
254, 3, 40, 41, 42, // Reserved
......@@ -177,16 +177,23 @@ public:
EXPECT_TRUE(pkt->getOption(128));
EXPECT_TRUE(pkt->getOption(254));
boost::shared_ptr<Option> x = pkt->getOption(12);
ASSERT_TRUE(x); // option 1 should exist
// Verify the packet type is correct.
ASSERT_EQ(DHCPOFFER, pkt->getType());
// First option after message type starts at 3.
uint8_t *opt_data_ptr = v4_opts + 3;
// Option 12 is represented by the OptionString class so let's do
// the appropriate conversion.
boost::shared_ptr<Option> x = pkt->getOption(12);
ASSERT_TRUE(x); // option 1 should exist
OptionStringPtr option12 = boost::static_pointer_cast<OptionString>(x);
ASSERT_TRUE(option12);
EXPECT_EQ(12, option12->getType()); // this should be option 12
ASSERT_EQ(3, option12->getValue().length()); // it should be of length 3
EXPECT_EQ(5, option12->len()); // total option length 5
EXPECT_EQ(0, memcmp(&option12->getValue()[0], v4_opts + 2, 3)); // data len=3
EXPECT_EQ(0, memcmp(&option12->getValue()[0], opt_data_ptr + 2, 3)); // data len=3
opt_data_ptr += x->len();
x = pkt->getOption(14);
ASSERT_TRUE(x); // option 14 should exist
......@@ -197,28 +204,31 @@ public:
EXPECT_EQ(14, option14->getType()); // this should be option 14
ASSERT_EQ(3, option14->getValue().length()); // it should be of length 3
EXPECT_EQ(5, option14->len()); // total option length 5
EXPECT_EQ(0, memcmp(&option14->getValue()[0], v4_opts + 7, 3)); // data len=3
EXPECT_EQ(0, memcmp(&option14->getValue()[0], opt_data_ptr + 2, 3)); // data len=3
opt_data_ptr += x->len();
x = pkt->getOption(60);
ASSERT_TRUE(x); // option 60 should exist
EXPECT_EQ(60, x->getType()); // this should be option 60
ASSERT_EQ(3, x->getData().size()); // it should be of length 3
EXPECT_EQ(5, x->len()); // total option length 5
EXPECT_EQ(0, memcmp(&x->getData()[0], v4_opts + 15, 3)); // data len=3
EXPECT_EQ(0, memcmp(&x->getData()[0], opt_data_ptr + 2, 3)); // data len=3
opt_data_ptr += x->len();
x = pkt->getOption(128);
ASSERT_TRUE(x); // option 3 should exist
EXPECT_EQ(128, x->getType()); // this should be option 254
ASSERT_EQ(3, x->getData().size()); // it should be of length 3
EXPECT_EQ(5, x->len()); // total option length 5
EXPECT_EQ(0, memcmp(&x->getData()[0], v4_opts + 20, 3)); // data len=3
EXPECT_EQ(0, memcmp(&x->getData()[0], opt_data_ptr + 2, 3)); // data len=3
opt_data_ptr += x->len();
x = pkt->getOption(254);
ASSERT_TRUE(x); // option 3 should exist
EXPECT_EQ(254, x->getType()); // this should be option 254
ASSERT_EQ(3, x->getData().size()); // it should be of length 3
EXPECT_EQ(5, x->len()); // total option length 5
EXPECT_EQ(0, memcmp(&x->getData()[0], v4_opts + 25, 3)); // data len=3
EXPECT_EQ(0, memcmp(&x->getData()[0], opt_data_ptr + 2, 3)); // data len=3
}
};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment