[github23] Corrected setting port number for DHCPv4 server.

For the DHCPINFORM case set destination port 68 if:
- direct message sent without ciaddr,
- ciaddr present

Also updated unit test to verify port number for the directly
sent DHCPINFORM without ciaddr.

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 6faab3dca4469ac1aa82c7f5d4a656de80a491b5..1ef9d7f0f88c3738f5b215a9cc03c1786a75bdb6 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1776,15 +1776,25 @@ Dhcpv4Srv::adjustIfaceData(Dhcpv4Exchange& ex) {
     Pkt4Ptr query = ex.getQuery();
     Pkt4Ptr response = ex.getResponse();
 
-    // For the non-relayed message, the destination port is the client's port.
-    // For the relayed message, the server/relay port is a destination unless
-    // the message is DHCPINFORM.
+    // The DHCPINFORM is generally unicast to the client. The only situation
+    // when the server is unable to unicast to the client is when the client
+    // doesn't include ciaddr and the message is relayed. In this case the
+    // server has to reply via relay agent. For other messages we send back
+    // through relay if message is relayed, and unicast to the client if the
+    // message is not relayed.
     // Note that the call to this function may throw if invalid combination
     // of hops and giaddr is found (hops = 0 if giaddr = 0 and hops != 0 if
     // giaddr != 0). The exception will propagate down and eventually cause the
     // packet to be discarded.
-    const bool server_port = query->isRelayed() && ((query->getType() == DHCPINFORM) ? query->getCiaddr().isV4Zero() : true);
-    response->setRemotePort(server_port ? DHCP4_SERVER_PORT : DHCP4_CLIENT_PORT);
+    if (((query->getType() == DHCPINFORM) &&
+         ((!query->getCiaddr().isV4Zero()) ||
+          (!query->isRelayed() && !query->getRemoteAddr().isV4Zero()))) ||
+        ((query->getType() != DHCPINFORM) && !query->isRelayed())) {
+        response->setRemotePort(DHCP4_CLIENT_PORT);
+
+    } else {
+        response->setRemotePort(DHCP4_SERVER_PORT);
+    }
 
     IOAddress local_addr = query->getLocalAddr();
 

diff --git a/src/bin/dhcp4/tests/dhcp4_client.cc b/src/bin/dhcp4/tests/dhcp4_client.cc
index 82c78e7a60feba740dd3d6bfe40f19a784bcd548..28b6cc08bf3534269d4cef0ae4fcd73346c5b06f 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_client.cc
+++ b/src/bin/dhcp4/tests/dhcp4_client.cc
@@ -467,6 +467,8 @@ Dhcp4Client::receiveOneMsg() {
                               msg->getBuffer().getLength()));
     msg_copy->setRemoteAddr(msg->getLocalAddr());
     msg_copy->setLocalAddr(msg->getRemoteAddr());
+    msg_copy->setRemotePort(msg->getLocalPort());
+    msg_copy->setLocalPort(msg->getRemotePort());
     msg_copy->setIface(msg->getIface());
 
     msg_copy->unpack();

diff --git a/src/bin/dhcp4/tests/inform_unittest.cc b/src/bin/dhcp4/tests/inform_unittest.cc
index e00f7c78b5971b425e2c8307a41175532a208ef7..5ed038b4dcabf8e8b4eee3bc7fc29301789f2137 100644 (file)
--- a/src/bin/dhcp4/tests/inform_unittest.cc
+++ b/src/bin/dhcp4/tests/inform_unittest.cc
@@ -268,6 +268,8 @@ TEST_F(InformTest, directClientNoCiaddr) {
     EXPECT_EQ(IOAddress("10.0.0.56"), resp->getLocalAddr());
     // Response must not be relayed.
     EXPECT_FALSE(resp->isRelayed());
+    EXPECT_EQ(DHCP4_CLIENT_PORT, resp->getLocalPort());
+    EXPECT_EQ(DHCP4_SERVER_PORT, resp->getRemotePort());
     // Make sure that the server id is present.
     EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText());
     // Make sure that the Routers option has been received.