[2320] HWAddr structure implemented.

src/lib/dhcp/Makefile.am

@@ -15,6 +15,7 @@ CLEANFILES = *.gcno *.gcda
 lib_LTLIBRARIES = libb10-dhcp++.la
 libb10_dhcp___la_SOURCES  =
 libb10_dhcp___la_SOURCES += duid.cc duid.h
+libb10_dhcp___la_SOURCES += hwaddr.cc hwaddr.h
 libb10_dhcp___la_SOURCES += iface_mgr.cc iface_mgr.h
 libb10_dhcp___la_SOURCES += iface_mgr_bsd.cc
 libb10_dhcp___la_SOURCES += iface_mgr_linux.cc

src/lib/dhcp/duid.h

@@ -86,7 +86,7 @@ typedef boost::shared_ptr<DUID> DuidPtr;
 ///
 /// This class is intended to be a generic IPv4 client identifier. It can hold
 /// a client-id
-class ClientId : DUID {
+class ClientId : public DUID {
 public:
     /// @brief Maximum size of a client ID
     ///

src/lib/dhcp/hwaddr.cc

@@ -0,0 +1,43 @@
+// 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/hwaddr.h>
+#include <dhcp/dhcp4.h>
+
+namespace isc {
+namespace dhcp {
+
+HWAddr::HWAddr()
+    :htype_(HTYPE_ETHER) {
+}
+
+HWAddr::HWAddr(const uint8_t* hwaddr, size_t len, uint8_t htype)
+    :hwaddr_(hwaddr, hwaddr + len), htype_(htype) {
+}
+
+HWAddr::HWAddr(const std::vector<uint8_t>& hwaddr, uint8_t htype)
+    :hwaddr_(hwaddr), htype_(htype) {
+}
+
+bool HWAddr::operator==(const HWAddr& other) const {
+    return ((this->htype_  == other.htype_) && 
+            (this->hwaddr_ == other.hwaddr_));
+}
+
+bool HWAddr::operator!=(const HWAddr& other) const {
+    return !(*this == other);
+}
+
+}; // end of isc::dhcp namespace
+}; // end of isc namespace

src/lib/dhcp/hwaddr.h

@@ -0,0 +1,52 @@
+// 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 HWADDR_H
+#define HWADDR_H
+
+#include <vector>
+#include <stdint.h>
+#include <stddef.h>
+#include <boost/shared_ptr.hpp>
+
+namespace isc {
+namespace dhcp {
+
+struct HWAddr {
+public:
+    HWAddr();
+    HWAddr(const uint8_t* hwaddr, size_t len, uint8_t htype);
+    HWAddr(const std::vector<uint8_t>& hwaddr, uint8_t htype);
+
+    // Vector that keeps the actual hardware address
+    std::vector<uint8_t> hwaddr_;
+
+    // Hardware type
+    uint8_t htype_;
+
+    /// @brief Compares two hardware addresses for equality
+    bool operator==(const HWAddr& other) const;
+
+    /// @brief Compares two hardware addresses for inequality
+    bool operator!=(const HWAddr& other) const;
+};
+
+/// @brief Shared pointer to a hardware address structure
+typedef boost::shared_ptr<HWAddr> HWAddrPtr;
+
+
+}; // end of isc::dhcp namespace
+}; // end of isc namespace
+
+#endif // HWADDR_H

src/lib/dhcp/pkt4.cc

@@ -40,8 +40,7 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
       local_port_(DHCP4_SERVER_PORT),
       remote_port_(DHCP4_CLIENT_PORT),
       op_(DHCPTypeToBootpType(msg_type)),
-      htype_(HTYPE_ETHER),
-      hlen_(0),
+      hwaddr_(new HWAddr()),
       hops_(0),
       transid_(transid),
       secs_(0),
@@ -53,7 +52,6 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
       bufferOut_(DHCPV4_PKT_HDR_LEN),
       msg_type_(msg_type)
 {
-    memset(chaddr_, 0, MAX_CHADDR_LEN);
     memset(sname_, 0, MAX_SNAME_LEN);
     memset(file_, 0, MAX_FILE_LEN);
 }
@@ -66,6 +64,7 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
       local_port_(DHCP4_SERVER_PORT),
       remote_port_(DHCP4_CLIENT_PORT),
       op_(BOOTREQUEST),
+      hwaddr_(new HWAddr()),
       transid_(0),
       secs_(0),
       flags_(0),
@@ -105,9 +104,15 @@ Pkt4::len() {
 
 bool
 Pkt4::pack() {
+    if (!hwaddr_) {
+        isc_throw(InvalidOperation, "Can't build Pkt4 packet. HWAddr not set.");
+    }
+
+    size_t hw_len = hwaddr_->hwaddr_.size();
+
     bufferOut_.writeUint8(op_);
-    bufferOut_.writeUint8(htype_);
-    bufferOut_.writeUint8(hlen_);
+    bufferOut_.writeUint8(hwaddr_->htype_);
+    bufferOut_.writeUint8(hw_len < 16 ? hw_len : 16);
     bufferOut_.writeUint8(hops_);
     bufferOut_.writeUint32(transid_);
     bufferOut_.writeUint16(secs_);
@@ -116,7 +121,22 @@ Pkt4::pack() {
     bufferOut_.writeUint32(yiaddr_);
     bufferOut_.writeUint32(siaddr_);
     bufferOut_.writeUint32(giaddr_);
-    bufferOut_.writeData(chaddr_, MAX_CHADDR_LEN);
+
+
+    if (hw_len <=16) {
+        // write up to 16 bytes of the hardware address (CHADDR field is 16
+        // bytes long in DHCPv4 message).
+        bufferOut_.writeData(&hwaddr_->hwaddr_[0], (hw_len<16?hw_len:16) );
+        hw_len = 16 - hw_len;
+    } else {
+        hw_len = 16;
+    }
+
+    // write (len) bytes of padding
+    vector<uint8_t> zeros(hw_len, 0);
+    bufferOut_.writeData(&zeros[0], hw_len);
+    // bufferOut_.writeData(chaddr_, MAX_CHADDR_LEN);
+
     bufferOut_.writeData(sname_, MAX_SNAME_LEN);
     bufferOut_.writeData(file_, MAX_FILE_LEN);
 
@@ -145,8 +165,8 @@ Pkt4::unpack() {
     }
 
     op_ = bufferIn.readUint8();
-    htype_ = bufferIn.readUint8();
-    hlen_ = bufferIn.readUint8();
+    uint8_t htype = bufferIn.readUint8();
+    uint8_t hlen = bufferIn.readUint8();
     hops_ = bufferIn.readUint8();
     transid_ = bufferIn.readUint32();
     secs_ = bufferIn.readUint16();
@@ -155,10 +175,16 @@ Pkt4::unpack() {
     yiaddr_ = IOAddress(bufferIn.readUint32());
     siaddr_ = IOAddress(bufferIn.readUint32());
     giaddr_ = IOAddress(bufferIn.readUint32());
-    bufferIn.readData(chaddr_, MAX_CHADDR_LEN);
+
+    vector<uint8_t> hw_addr(MAX_CHADDR_LEN, 0);
+    bufferIn.readVector(hw_addr, MAX_CHADDR_LEN);
     bufferIn.readData(sname_, MAX_SNAME_LEN);
     bufferIn.readData(file_, MAX_FILE_LEN);
 
+    hw_addr.resize(hlen);
+
+    hwaddr_ = HWAddrPtr(new HWAddr(hw_addr, htype));
+
     if (bufferIn.getLength() == bufferIn.getPosition()) {
         // this is *NOT* DHCP packet. It does not have any DHCPv4 options. In
         // particular, it does not have magic cookie, a 4 byte sequence that
@@ -239,10 +265,7 @@ Pkt4::setHWAddr(uint8_t hType, uint8_t hlen,
         isc_throw(OutOfRange, "Invalid HW Address specified");
     }
 
-    htype_ = hType;
-    hlen_ = hlen;
-    std::copy(&mac_addr[0], &mac_addr[hlen], &chaddr_[0]);
-    std::fill(&chaddr_[hlen], &chaddr_[MAX_CHADDR_LEN], 0);
+    hwaddr_ = HWAddrPtr(new HWAddr(mac_addr, hType));
 }
 
 void
@@ -302,6 +325,23 @@ Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
     }
 }
 
+uint8_t
+Pkt4::getHtype() const {
+    if (!hwaddr_) {
+        isc_throw(InvalidOperation, "Can't get HType. HWAddr not defined");
+    }
+    return (hwaddr_->htype_);
+}
+
+uint8_t
+Pkt4::getHlen() const {
+    if (!hwaddr_) {
+        isc_throw(InvalidOperation, "Can't get HType. HWAddr not defined");
+    }
+    uint8_t len = hwaddr_->hwaddr_.size();
+    return (len <= 16 ? len : 16);
+}
+
 void
 Pkt4::addOption(boost::shared_ptr<Option> opt) {
     // Check for uniqueness (DHCPv4 options must be unique)

src/lib/dhcp/pkt4.h

@@ -18,6 +18,7 @@
 #include <asiolink/io_address.h>
 #include <util/buffer.h>
 #include <dhcp/option.h>
+#include <dhcp/hwaddr.h>
 
 #include <boost/date_time/posix_time/posix_time.hpp>
 #include <boost/shared_ptr.hpp>
@@ -277,23 +278,17 @@ public:
     ///
     /// @return hardware type
     uint8_t
-    getHtype() const { return (htype_); };
+    getHtype() const;
 
     /// Returns hlen field
     ///
     /// @return hardware address length
     uint8_t
-    getHlen() const { return (hlen_); };
-
-    /// @brief Returns chaddr field.
-    ///
-    /// Note: This is 16 bytes long field. It doesn't have to be
-    /// null-terminated. Do no use strlen() or similar on it.
-    ///
-    /// @return pointer to hardware address
-    const uint8_t*
-    getChaddr() const { return (chaddr_); };
+    getHlen() const;
 
+    /// @brief returns hardware address information
+    /// @return hardware address structure
+    HWAddrPtr getHWAddr() const { return (hwaddr_); }
 
     /// @brief Returns reference to output buffer.
     ///
@@ -454,11 +449,11 @@ protected:
     /// type is kept in message type option).
     uint8_t op_;
 
-    /// link-layer address type
-    uint8_t htype_;
-
-    /// link-layer address length
-    uint8_t hlen_;
+    /// @brief link-layer address and hardware information
+    /// represents 3 fields: htype (hardware type, 1 byte), hlen (length of the
+    /// hardware address, up to 16) and chaddr (hardware address field,
+    /// 16 bytes)
+    HWAddrPtr hwaddr_;
 
     /// Number of relay agents traversed
     uint8_t hops_;
@@ -484,9 +479,6 @@ protected:
     /// giaddr field (32 bits): Gateway IP address
     isc::asiolink::IOAddress giaddr_;
 
-    /// Hardware address field (16 bytes)
-    uint8_t chaddr_[MAX_CHADDR_LEN];
-
     /// sname field (64 bytes)
     uint8_t sname_[MAX_SNAME_LEN];
 

src/lib/dhcp/tests/Makefile.am

@@ -27,6 +27,7 @@ if HAVE_GTEST
 TESTS += libdhcp++_unittests
 
 libdhcp___unittests_SOURCES  = run_unittests.cc
+libdhcp___unittests_SOURCES += hwaddr_unittest.cc
 libdhcp___unittests_SOURCES += iface_mgr_unittest.cc
 libdhcp___unittests_SOURCES += libdhcp++_unittest.cc
 libdhcp___unittests_SOURCES += option4_addrlst_unittest.cc

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -220,7 +220,7 @@ TEST(Pkt4Test, fixedFields) {
     EXPECT_EQ(dummyGiaddr.toText(), pkt->getGiaddr().toText());
 
     // chaddr is always 16 bytes long and contains link-layer addr (MAC)
-    EXPECT_EQ(0, memcmp(dummyChaddr, pkt->getChaddr(), 16));
+    EXPECT_EQ(0, memcmp(dummyChaddr, &pkt->getHWAddr()->hwaddr_[0], 16));
 
     EXPECT_EQ(0, memcmp(dummySname, &pkt->getSname()[0], 64));
 
@@ -282,7 +282,7 @@ TEST(Pkt4Test, fixedFieldsUnpack) {
     EXPECT_EQ(string("255.255.255.255"), pkt->getGiaddr().toText());
 
     // chaddr is always 16 bytes long and contains link-layer addr (MAC)
-    EXPECT_EQ(0, memcmp(dummyChaddr, pkt->getChaddr(), Pkt4::MAX_CHADDR_LEN));
+    EXPECT_EQ(0, memcmp(dummyChaddr, &pkt->getHWAddr()->hwaddr_[0], dummyHlen));
 
     ASSERT_EQ(static_cast<size_t>(Pkt4::MAX_SNAME_LEN), pkt->getSname().size());
     EXPECT_EQ(0, memcmp(dummySname, &pkt->getSname()[0], Pkt4::MAX_SNAME_LEN));
@@ -321,7 +321,7 @@ TEST(Pkt4Test, hwAddr) {
         pkt->setHWAddr(255-macLen*10, // just weird htype
                        macLen,
                        mac);
-        EXPECT_EQ(0, memcmp(expectedChaddr, pkt->getChaddr(),
+        EXPECT_EQ(0, memcmp(expectedChaddr, &pkt->getHWAddr()->hwaddr_[0],
                             Pkt4::MAX_CHADDR_LEN));
 
         EXPECT_NO_THROW(