[5390] Simplified the logic (addressed comment)

diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml
index 9df4dd62a82885bea4601ce5d6190b4fe85c7778..66a39e6cb2ed131568072e5e5adf42687b63019e 100644 (file)
--- a/doc/guide/dhcp4-srv.xml
+++ b/doc/guide/dhcp4-srv.xml
@@ -773,8 +773,8 @@ temporarily override a list of interface names and listen on all interfaces.
   </para>
 
   <para>Usually loopback interfaces (e.g. the "lo" or "lo0" interface)
-  may not configured but if only one interface is configured and
-  IP/UDP sockets are specified a loopback interface is accepted.
+  may not configured but if a loopback interface is explicitely configured
+  and IP/UDP sockets are specified the loopback interface is accepted.
   </para>
 
   <para>It can be used for instance to run Kea in a FreeBSD jail having

diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml
index 3ff642e1d6e60f9bd82acedaedb685b45e50b313..c26b7ab205f391e88e3537f774a451f5e8546e35 100644 (file)
--- a/doc/guide/dhcp6-srv.xml
+++ b/doc/guide/dhcp6-srv.xml
@@ -655,8 +655,8 @@ temporarily override a list of interface names and listen on all interfaces.
   </screen>
 
   <para>Usually loopback interfaces (e.g. the "lo" or "lo0" interface)
-  may not configured but if only one interface is configured a loopback
-  interface is accepted. Note Kea requires link-local address which does
+  may not configured but if a loopback interface is explicitely configured
+  it is accepted. Note Kea requires a link-local address which does
   not exist on all systems, or a specified unicast address as in:
   </para>
 

diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc
index 540ccac0bff612fbf908a26652e76eb22c991913..e6ba36f44ea93f9428d37f33857072542b971542 100644 (file)
--- a/src/lib/dhcpsrv/cfg_iface.cc
+++ b/src/lib/dhcpsrv/cfg_iface.cc
@@ -57,38 +57,22 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port,
     // of the IfaceMgr. Those modifications require that sockets are closed.
     closeSockets();
     // The loopback interface can be used only when:
-    //  - wildcard is not used
     //  - UDP socket will be used, i.e. not IPv4 and RAW socket
-    //  - there is one interface name only in the interface set
-    //    and this interface is a loopback interface.
-    //  - or the interface set is empty and all interfaces in the address
-    //    map are the same and a loopback interface.
+    //  - the loopback interface is in the interface set or the address map.
     bool loopback_used_ = false;
-    if (!wildcard_used_ &&
-        ((family == AF_INET6) || (socket_type_ == SOCKET_UDP)) &&
-        (iface_set_.size() == 1) &&
-        (address_map_.empty())) {
-        // Get the first and only interface.
-        IfacePtr iface = IfaceMgr::instance().getIface(*iface_set_.begin());
-        if (iface && iface->flag_loopback_) {
-            loopback_used_ = true;
+    if ((family == AF_INET6) || (socket_type_ == SOCKET_UDP)) {
+        // Check interface set
+        for (IfaceSet::const_iterator iface_name = iface_set_.begin();
+             iface_name != iface_set_.end(); ++iface_name) {
+            IfacePtr iface = IfaceMgr::instance().getIface(*iface_name);
+            if (iface && iface->flag_loopback_) {
+                loopback_used_ = true;
+            }
         }
-    } else if (!wildcard_used_ &&
-               ((family == AF_INET6) || (socket_type_ == SOCKET_UDP)) &&
-               iface_set_.empty() &&
-               !address_map_.empty()) {
-        // Get the first interface
-        const std::string& name = address_map_.begin()->first;
-        bool same = true;
+        // Check address map
         for (ExplicitAddressMap::const_iterator unicast = address_map_.begin();
              unicast != address_map_.end(); ++unicast) {
-            if (unicast->first != name) {
-                same = false;
-                break;
-            }
-        }
-        if (same) {
-            IfacePtr iface = IfaceMgr::instance().getIface(name);
+            IfacePtr iface = IfaceMgr::instance().getIface(unicast->first);
             if (iface && iface->flag_loopback_) {
                 loopback_used_ = true;
             }
@@ -99,7 +83,7 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port,
     // interface names specified by the user. If wildcard interface was
     // specified, mark all interfaces active. Mark loopback inactive when
     // not explicitely allowed.
-    setState(family, !wildcard_used_, loopback_used_);
+    setState(family, !wildcard_used_, !loopback_used_);
     IfaceMgr& iface_mgr = IfaceMgr::instance();
     // Remove selection of unicast addresses from all interfaces.
     iface_mgr.clearUnicasts();

diff --git a/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc b/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc
index 4246fc735eaff8061cc10e704dbd08115b9092b9..25c36056b29a4197fcd8466b44b276b1d651c44c 100644 (file)
--- a/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_iface_unittest.cc
@@ -209,8 +209,10 @@ TEST_F(CfgIfaceTest, explicitLoopbackV4) {
     ASSERT_NO_THROW(cfg.use(AF_INET, "lo"));
     ASSERT_NO_THROW(cfg.useSocketType(AF_INET, CfgIface::SOCKET_UDP));
     cfg.openSockets(AF_INET, DHCP4_SERVER_PORT);
-    // No loopback socket
-    EXPECT_FALSE(socketOpen("lo", "127.0.0.1"));
+    // No wildcard is no longer a constraint
+    EXPECT_TRUE(socketOpen("lo", "127.0.0.1"));
+    cfg.closeSockets();
+    ASSERT_FALSE(socketOpen("lo", "127.0.0.1"));
 
     // Retry without UDP sockets
     cfg.reset();
@@ -225,31 +227,21 @@ TEST_F(CfgIfaceTest, explicitLoopbackV4) {
     ASSERT_NO_THROW(cfg.use(AF_INET, "lo"));
     ASSERT_NO_THROW(cfg.useSocketType(AF_INET, CfgIface::SOCKET_UDP));
     cfg.openSockets(AF_INET, DHCP4_SERVER_PORT);
-    // No loopback socket
+    // Only loopback is no longer a constraint
+    EXPECT_TRUE(socketOpen("lo", "127.0.0.1"));
+    cfg.closeSockets();
     EXPECT_FALSE(socketOpen("lo", "127.0.0.1"));
 
-    // Finally with a second interface and address
+    // Finally with interfaces and addresses
     cfg.reset();
     ASSERT_NO_THROW(cfg.use(AF_INET, "eth0/10.0.0.1"));
-    ASSERT_NO_THROW(cfg.use(AF_INET, "lo"));
+    ASSERT_NO_THROW(cfg.use(AF_INET, "lo/127.0.0.1"));
     ASSERT_NO_THROW(cfg.useSocketType(AF_INET, CfgIface::SOCKET_UDP));
     cfg.openSockets(AF_INET, DHCP4_SERVER_PORT);
-    // No loopback socket
-    EXPECT_FALSE(socketOpen("lo", "127.0.0.1"));
-}
-
-// Tests that the loopback socket is not opened in raw mode.
-TEST_F(CfgIfaceTest, explicitLoopbackV4Negative) {
-    CfgIface cfg;
-    ASSERT_NO_THROW(cfg.use(AF_INET, "lo"));
-
-    // Use UDP sockets
-    ASSERT_NO_THROW(cfg.useSocketType(AF_INET, CfgIface::SOCKET_RAW));
-
-    // Open sockets on specified interfaces and addresses.
-    cfg.openSockets(AF_INET, DHCP4_SERVER_PORT);
+    // Only loopback is no longer a constraint
+    EXPECT_TRUE(socketOpen("lo", "127.0.0.1"));
+    cfg.closeSockets();
     EXPECT_FALSE(socketOpen("lo", "127.0.0.1"));
-    EXPECT_FALSE(socketOpen("lo", AF_INET));
 }
 
 // This test checks that the interface names can be explicitly selected
@@ -291,20 +283,6 @@ TEST_F(CfgIfaceTest, explicitNamesV6) {
     EXPECT_FALSE(socketOpen("lo", AF_INET6));
 }
 
-// This test checks that the loopback socket can be opened.
-TEST_F(CfgIfaceTest, loopback6) {
-    CfgIface cfg;
-    // Specify valid interface names. There should be no error.
-    ASSERT_NO_THROW(cfg.use(AF_INET6, "lo"));
-
-    // Open sockets on specified interfaces.
-    cfg.openSockets(AF_INET6, DHCP6_SERVER_PORT);
-
-    // There should be a socket open.
-    EXPECT_FALSE(socketOpen("lo", AF_INET6));
-    EXPECT_FALSE(socketOpen("lo", "::1"));
-}
-
 // This test checks that the wildcard interface name can be specified to
 // select all interfaces to open IPv4 sockets.
 TEST_F(CfgIfaceTest, wildcardV4) {
@@ -403,24 +381,30 @@ TEST_F(CfgIfaceTest, explicitLoopbackV6) {
     ASSERT_NO_THROW(cfg.use(AF_INET6, "*"));
     ASSERT_NO_THROW(cfg.use(AF_INET6, "lo/::1"));
     cfg.openSockets(AF_INET6, DHCP6_SERVER_PORT);
-    // No loopback socket
-    EXPECT_FALSE(socketOpen("lo", AF_INET6));
+    // No wildcard is no longer a constraint
+    EXPECT_TRUE(socketOpen("lo", AF_INET6));
+    cfg.closeSockets();
+    ASSERT_FALSE(socketOpen("lo", AF_INET6));
 
     // Retry with a second interface
     cfg.reset();
     ASSERT_NO_THROW(cfg.use(AF_INET6, "eth0"));
     ASSERT_NO_THROW(cfg.use(AF_INET6, "lo/::1"));
     cfg.openSockets(AF_INET6, DHCP6_SERVER_PORT);
-    // No loopback socket
-    EXPECT_FALSE(socketOpen("lo", AF_INET6));
+    // Only loopback is no longer a constraint
+    EXPECT_TRUE(socketOpen("lo", AF_INET6));
+    cfg.closeSockets();
+    ASSERT_FALSE(socketOpen("lo", AF_INET6));
 
-    // Finally with a second interface and address
+    // Finally with interfaces and addresses
     cfg.reset();
     ASSERT_NO_THROW(cfg.use(AF_INET6, "eth0/2001:db8:1::1"));
     ASSERT_NO_THROW(cfg.use(AF_INET6, "lo/::1"));
     cfg.openSockets(AF_INET6, DHCP6_SERVER_PORT);
-    // No loopback socket
-    EXPECT_FALSE(socketOpen("lo", AF_INET6));
+    // Only loopback is no longer a constraint
+    EXPECT_TRUE(socketOpen("lo", AF_INET6));
+    cfg.closeSockets();
+    ASSERT_FALSE(socketOpen("lo", AF_INET6));
 }
 
 // Test that the equality and inequality operators work fine for CfgIface.