[2902] Added data fields storing src/dest HW Address in Pkt4 class.

src/lib/dhcp/pkt4.cc

@@ -276,8 +276,23 @@ Pkt4::toText() {
 }
 
 void
-Pkt4::setHWAddr(uint8_t hType, uint8_t hlen,
+Pkt4::setHWAddr(uint8_t htype, uint8_t hlen,
                 const std::vector<uint8_t>& mac_addr) {
+    setHWAddrMember(htype, hlen, mac_addr, hwaddr_);
+}
+
+void
+Pkt4::setHWAddr(const HWAddrPtr& addr) {
+    if (!addr) {
+        isc_throw(BadValue, "Setting hw address to NULL is forbidden");
+    }
+    hwaddr_ = addr;
+}
+
+void
+Pkt4::setHWAddrMember(const uint8_t htype, const uint8_t hlen,
+                      const std::vector<uint8_t>& mac_addr,
+                      HWAddrPtr& hw_addr) {
     /// @todo Rewrite this once support for client-identifier option
     /// is implemented (ticket 1228?)
     if (hlen > MAX_CHADDR_LEN) {
@@ -288,15 +303,35 @@ Pkt4::setHWAddr(uint8_t hType, uint8_t hlen,
         isc_throw(OutOfRange, "Invalid HW Address specified");
     }
 
-    hwaddr_.reset(new HWAddr(mac_addr, hType));
+    hw_addr.reset(new HWAddr(mac_addr, htype));
 }
 
 void
-Pkt4::setHWAddr(const HWAddrPtr& addr) {
+Pkt4::setLocalHWAddr(const uint8_t htype, const uint8_t hlen,
+                      const std::vector<uint8_t>& mac_addr) {
+    setHWAddrMember(htype, hlen, mac_addr, local_hwaddr_);
+}
+
+void
+Pkt4::setLocalHWAddr(const HWAddrPtr& addr) {
     if (!addr) {
-        isc_throw(BadValue, "Setting hw address to NULL is forbidden");
+        isc_throw(BadValue, "Setting HW address to NULL is forbidden");
     }
-    hwaddr_ = addr;
+    local_hwaddr_ = addr;
+}
+
+void
+Pkt4::setRemoteHWAddr(const uint8_t htype, const uint8_t hlen,
+                      const std::vector<uint8_t>& mac_addr) {
+    setHWAddrMember(htype, hlen, mac_addr, remote_hwaddr_);
+}
+
+void
+Pkt4::setRemoteHWAddr(const HWAddrPtr& addr) {
+    if (!addr) {
+        isc_throw(BadValue, "Setting HW address to NULL is forbidden");
+    }
+    remote_hwaddr_ = addr;
 }
 
 void

src/lib/dhcp/pkt4.h

@@ -267,10 +267,10 @@ public:
     ///
     /// Note: mac_addr must be a buffer of at least hlen bytes.
     ///
-    /// @param hType hardware type (will be sent in htype field)
+    /// @param htype hardware type (will be sent in htype field)
     /// @param hlen hardware length (will be sent in hlen field)
     /// @param mac_addr pointer to hardware address
-    void setHWAddr(uint8_t hType, uint8_t hlen,
+    void setHWAddr(uint8_t htype, uint8_t hlen,
                    const std::vector<uint8_t>& mac_addr);
 
     /// @brief Sets hardware address
@@ -363,6 +363,72 @@ public:
     /// @return interface index
     uint32_t getIndex() const { return (ifindex_); };
 
+    /// @brief Sets remote HW address.
+    ///
+    /// Sets the destination HW address for the outgoing packet
+    /// or source HW address for the incoming packet. When this
+    /// is an outgoing packet this address will be used to construct
+    /// the link layer header.
+    ///
+    /// @note mac_addr must be a buffer of at least hlen bytes.
+    ///
+    /// @param htype hardware type (will be sent in htype field)
+    /// @param hlen hardware length (will be sent in hlen field)
+    /// @param mac_addr pointer to hardware address
+    void setRemoteHWAddr(const uint8_t htype, const uint8_t hlen,
+                         const std::vector<uint8_t>& mac_addr);
+
+    /// @brief Sets remote HW address.
+    ///
+    /// Sets hardware address from an existing HWAddr structure.
+    /// The remote address is a destination address for outgoing
+    /// packet and source address for incoming packet. When this
+    /// is an outgoing packet, this address will be used to
+    /// construct the link layer header.
+    ///
+    /// @param addr structure representing HW address.
+    ///
+    /// @throw BadValue if addr is null
+    void setRemoteHWAddr(const HWAddrPtr& addr);
+
+    /// @brief Returns the remote HW address.
+    ///
+    /// @return remote HW address.
+    HWAddrPtr getRemoteHWAddr() const {
+        return (remote_hwaddr_);
+    }
+
+    /// @brief Sets local HW address.
+    ///
+    /// Sets the source HW address for the outgoing packet or
+    /// destination HW address for the incoming packet.
+    ///
+    /// @note mac_addr must be a buffer of at least hlen bytes.
+    ///
+    /// @param htype hardware type (will be sent in htype field)
+    /// @param hlen hardware length (will be sent in hlen field)
+    /// @param mac_addr pointer to hardware address
+    void setLocalHWAddr(const uint8_t htype, const uint8_t hlen,
+                        const std::vector<uint8_t>& mac_addr);
+
+    /// @brief Sets local HW address.
+    ///
+    /// Sets hardware address from an existing HWAddr structure.
+    /// The local address is a source address for outgoing
+    /// packet and destination address for incoming packet.
+    ///
+    /// @param addr structure representing HW address.
+    ///
+    /// @throw BadValue if addr is null
+    void setLocalHWAddr(const HWAddrPtr& addr);
+
+    /// @brief Returns local HW address.
+    ///
+    /// @return local HW addr.
+    HWAddrPtr getLocalHWAddr() const {
+        return (local_hwaddr_);
+    }
+
     /// @brief Sets remote address.
     ///
     /// @param remote specifies remote address
@@ -419,6 +485,23 @@ public:
     /// @throw isc::Unexpected if timestamp update failed
     void updateTimestamp();
 
+private:
+
+    /// @brief Generic method that validates and sets HW address.
+    ///
+    /// This is a generic method used by all modifiers of this class
+    /// which set class members representing HW address.
+    ///
+    /// @param htype hardware type.
+    /// @param hlen hardware length.
+    /// @param mac_addr pointer to actual hardware address.
+    /// @param [out] hw_addr pointer to a class member to be modified.
+    ///
+    /// @trow isc::OutOfRange if invalid HW address specified.
+    void setHWAddrMember(const uint8_t htype, const uint8_t hlen,
+                         const std::vector<uint8_t>& mac_addr,
+                         HWAddrPtr& hw_addr);
+
 protected:
 
     /// converts DHCP message type to BOOTP op type
@@ -429,6 +512,12 @@ protected:
     uint8_t
     DHCPTypeToBootpType(uint8_t dhcpType);
 
+    /// local HW address (dst if receiving packet, src if sending packet)
+    HWAddrPtr local_hwaddr_;
+
+    // remote HW address (src if receiving packet, dst if sending packet)
+    HWAddrPtr remote_hwaddr_;
+
     /// local address (dst if receiving packet, src if sending packet)
     isc::asiolink::IOAddress local_addr_;
 
@@ -533,6 +622,7 @@ protected:
 
     /// packet timestamp
     boost::posix_time::ptime timestamp_;
+
 }; // Pkt4 class
 
 typedef boost::shared_ptr<Pkt4> Pkt4Ptr;

src/lib/dhcp/protocol_util.cc

@@ -48,13 +48,14 @@ decodeEthernetHeader(InputBuffer& buf, Pkt4Ptr& pkt) {
     // Remember initial position.
     size_t start_pos = buf.getPosition();
 
-    // Skip destination address.
-    buf.setPosition(start_pos + HWAddr::ETHERNET_HWADDR_LEN);
-    // Read the source HW address.
+    // Read the destination HW address.
     std::vector<uint8_t> dest_addr;
     buf.readVector(dest_addr, HWAddr::ETHERNET_HWADDR_LEN);
-    // Set the address we have read.
-    pkt->setHWAddr(HWTYPE_ETHERNET, HWAddr::ETHERNET_HWADDR_LEN, dest_addr);
+    pkt->setLocalHWAddr(HWTYPE_ETHERNET, HWAddr::ETHERNET_HWADDR_LEN, dest_addr);
+    // Read the source HW address.
+    std::vector<uint8_t> src_addr;
+    buf.readVector(src_addr, HWAddr::ETHERNET_HWADDR_LEN);
+    pkt->setRemoteHWAddr(HWTYPE_ETHERNET, HWAddr::ETHERNET_HWADDR_LEN, src_addr);
     // Move the buffer read pointer to the end of the Ethernet frame header.
     buf.setPosition(start_pos + ETHERNET_HEADER_LEN);
 }

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -670,4 +670,49 @@ TEST(Pkt4Test, hwaddr) {
     EXPECT_TRUE(hwaddr == pkt->getHWAddr());
 }
 
+// This test verifies that the packet remte and local HW address can
+// be set and returned.
+TEST(Pkt4Test, hwaddrSrcRemote) {
+    scoped_ptr<Pkt4> pkt(new Pkt4(DHCPOFFER, 1234));
+    const uint8_t src_hw[] = { 1, 2, 3, 4, 5, 6 };
+    const uint8_t dst_hw[] = { 7, 8, 9, 10, 11, 12 };
+    const uint8_t hw_type = 123;
+
+    HWAddrPtr dst_hwaddr(new HWAddr(dst_hw, sizeof(src_hw), hw_type));
+    HWAddrPtr src_hwaddr(new HWAddr(src_hw, sizeof(src_hw), hw_type));
+
+    // Check that we can set the local address.
+    EXPECT_NO_THROW(pkt->setLocalHWAddr(dst_hwaddr));
+    EXPECT_TRUE(dst_hwaddr == pkt->getLocalHWAddr());
+
+    // Check that we can set the remote address.
+    EXPECT_NO_THROW(pkt->setRemoteHWAddr(dst_hwaddr));
+    EXPECT_TRUE(dst_hwaddr == pkt->getRemoteHWAddr());
+
+    // Can't set the NULL addres.
+    EXPECT_THROW(pkt->setRemoteHWAddr(HWAddrPtr()), BadValue);
+    EXPECT_THROW(pkt->setLocalHWAddr(HWAddrPtr()), BadValue);
+
+    // Test alternative way to set local address.
+    const uint8_t dst_hw2[] = { 19, 20, 21, 22, 23, 24 };
+    std::vector<uint8_t> dst_hw_vec(dst_hw2, dst_hw2 + sizeof(dst_hw2));
+    const uint8_t hw_type2 = 234;
+    EXPECT_NO_THROW(pkt->setLocalHWAddr(hw_type2, sizeof(dst_hw2), dst_hw_vec));
+    HWAddrPtr local_addr = pkt->getLocalHWAddr();
+    ASSERT_TRUE(local_addr);
+    EXPECT_EQ(hw_type2, local_addr->htype_);
+    EXPECT_TRUE(std::equal(dst_hw_vec.begin(), dst_hw_vec.end(),
+                           local_addr->hwaddr_.begin()));
+
+    // Set remote address.
+    const uint8_t src_hw2[] = { 25, 26, 27, 28, 29, 30 };
+    std::vector<uint8_t> src_hw_vec(src_hw2, src_hw2 + sizeof(src_hw2));
+    EXPECT_NO_THROW(pkt->setRemoteHWAddr(hw_type2, sizeof(src_hw2), src_hw_vec));
+    HWAddrPtr remote_addr = pkt->getRemoteHWAddr();
+    ASSERT_TRUE(remote_addr);
+    EXPECT_EQ(hw_type2, remote_addr->htype_);
+    EXPECT_TRUE(std::equal(src_hw_vec.begin(), src_hw_vec.end(),
+                           remote_addr->hwaddr_.begin()));
+}
+
 } // end of anonymous namespace

src/lib/dhcp/tests/protocol_util_unittest.cc

@@ -81,7 +81,7 @@ TEST(ProtocolUtilTest, decodeEthernetHeader) {
     // Prepare a buffer holding Ethernet frame header and 4 bytes of
     // dummy data.
     OutputBuffer buf(1);
-    buf.writeData(dest_hw_addr, sizeof(src_hw_addr));
+    buf.writeData(dest_hw_addr, sizeof(dest_hw_addr));
     buf.writeData(src_hw_addr, sizeof(src_hw_addr));
     buf.writeUint16(0x800);
     // Append dummy data. We will later check that this data is not
@@ -108,14 +108,24 @@ TEST(ProtocolUtilTest, decodeEthernetHeader) {
     pkt.reset(new Pkt4(DHCPDISCOVER, 0));
     // It should not throw now.
     ASSERT_NO_THROW(decodeEthernetHeader(in_buf, pkt));
+    // Verify that the destination HW address has been initialized...
+    HWAddrPtr checked_dest_hwaddr = pkt->getLocalHWAddr();
+    ASSERT_TRUE(checked_dest_hwaddr);
+    // and is correct.
+    EXPECT_EQ(HWTYPE_ETHERNET, checked_dest_hwaddr->htype_);
+    ASSERT_EQ(sizeof(dest_hw_addr), checked_dest_hwaddr->hwaddr_.size());
+    EXPECT_TRUE(std::equal(dest_hw_addr, dest_hw_addr + sizeof(dest_hw_addr),
+                           checked_dest_hwaddr->hwaddr_.begin()));
+
     // Verify that the HW address of the source has been initialized.
-    HWAddrPtr hwaddr = pkt->getHWAddr();
-    ASSERT_TRUE(hwaddr);
+    HWAddrPtr checked_src_hwaddr = pkt->getRemoteHWAddr();
+    ASSERT_TRUE(checked_src_hwaddr);
     // And that it is correct.
-    EXPECT_EQ(HWTYPE_ETHERNET, hwaddr->htype_);
-    ASSERT_EQ(sizeof(dest_hw_addr), hwaddr->hwaddr_.size());
+    EXPECT_EQ(HWTYPE_ETHERNET, checked_src_hwaddr->htype_);
+    ASSERT_EQ(sizeof(src_hw_addr), checked_src_hwaddr->hwaddr_.size());
     EXPECT_TRUE(std::equal(src_hw_addr, src_hw_addr + sizeof(src_hw_addr),
-                           hwaddr->hwaddr_.begin()));
+                           checked_src_hwaddr->hwaddr_.begin()));
+
     // The entire ethernet packet header should have been read. This means
     // that the internal buffer pointer should now point to its tail.
     ASSERT_EQ(ETHERNET_HEADER_LEN, in_buf.getPosition());