face: process face_system.ether config section in EthernetFactory

This commit also fixes a potential memory access error in EthernetTransport.

refs #3904

Change-Id: I08296e7c6f1039b59b2859d277fc95326af34f52
diff --git a/tests/daemon/face/ethernet-factory.t.cpp b/tests/daemon/face/ethernet-factory.t.cpp
index 248a668..7c7f295 100644
--- a/tests/daemon/face/ethernet-factory.t.cpp
+++ b/tests/daemon/face/ethernet-factory.t.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2016,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -24,17 +24,275 @@
  */
 
 #include "face/ethernet-factory.hpp"
+#include "face/face.hpp"
 
 #include "factory-test-common.hpp"
-#include "network-interface-fixture.hpp"
+#include "ethernet-fixture.hpp"
+#include "face-system-fixture.hpp"
+#include <boost/algorithm/string/replace.hpp>
+#include <boost/range/algorithm/count_if.hpp>
 
 namespace nfd {
+namespace face {
 namespace tests {
 
-using nfd::face::tests::NetworkInterfaceFixture;
+using namespace nfd::tests;
 
 BOOST_AUTO_TEST_SUITE(Face)
-BOOST_FIXTURE_TEST_SUITE(TestEthernetFactory, NetworkInterfaceFixture)
+BOOST_FIXTURE_TEST_SUITE(TestEthernetFactory, EthernetFixture)
+
+using face::Face;
+
+class EthernetConfigFixture : public EthernetFixture
+                            , public FaceSystemFixture
+{
+public:
+  std::vector<const Face*>
+  listEtherMcastFaces() const
+  {
+    return this->listFacesByScheme("ether", ndn::nfd::LINK_TYPE_MULTI_ACCESS);
+  }
+
+  size_t
+  countEtherMcastFaces() const
+  {
+    return this->listEtherMcastFaces().size();
+  }
+};
+
+BOOST_FIXTURE_TEST_SUITE(ProcessConfig, EthernetConfigFixture)
+
+BOOST_AUTO_TEST_CASE(Normal)
+{
+  SKIP_IF_ETHERNET_NETIF_COUNT_LT(1);
+
+  const std::string CONFIG = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        mcast yes
+        mcast_group 01:00:5E:00:17:AA
+        whitelist
+        {
+          *
+        }
+        blacklist
+        {
+        }
+      }
+    }
+  )CONFIG";
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG, true));
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG, false));
+
+  BOOST_CHECK_EQUAL(this->countEtherMcastFaces(), netifs.size());
+}
+
+BOOST_AUTO_TEST_CASE(Omitted)
+{
+  const std::string CONFIG = R"CONFIG(
+    face_system
+    {
+    }
+  )CONFIG";
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG, true));
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG, false));
+
+  BOOST_CHECK_EQUAL(this->countEtherMcastFaces(), 0);
+}
+
+BOOST_AUTO_TEST_CASE(Whitelist)
+{
+  SKIP_IF_ETHERNET_NETIF_COUNT_LT(1);
+
+  std::string CONFIG = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        whitelist
+        {
+          ifname %ifname
+        }
+      }
+    }
+  )CONFIG";
+  boost::replace_first(CONFIG, "%ifname", netifs.front().name);
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG, false));
+  auto etherMcastFaces = this->listEtherMcastFaces();
+  BOOST_REQUIRE_EQUAL(etherMcastFaces.size(), 1);
+  BOOST_CHECK_EQUAL(etherMcastFaces.front()->getLocalUri().getHost(), netifs.front().name);
+}
+
+BOOST_AUTO_TEST_CASE(Blacklist)
+{
+  SKIP_IF_ETHERNET_NETIF_COUNT_LT(1);
+
+  std::string CONFIG = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        blacklist
+        {
+          ifname %ifname
+        }
+      }
+    }
+  )CONFIG";
+  boost::replace_first(CONFIG, "%ifname", netifs.front().name);
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG, false));
+  auto etherMcastFaces = this->listEtherMcastFaces();
+  BOOST_CHECK_EQUAL(etherMcastFaces.size(), netifs.size() - 1);
+  BOOST_CHECK_EQUAL(boost::count_if(etherMcastFaces, [=] (const Face* face) {
+    return face->getLocalUri().getHost() == netifs.front().name;
+  }), 0);
+}
+
+BOOST_AUTO_TEST_CASE(EnableDisableMcast)
+{
+  const std::string CONFIG_WITH_MCAST = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        mcast yes
+      }
+    }
+  )CONFIG";
+  const std::string CONFIG_WITHOUT_MCAST = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        mcast no
+      }
+    }
+  )CONFIG";
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG_WITHOUT_MCAST, false));
+  BOOST_CHECK_EQUAL(this->countEtherMcastFaces(), 0);
+
+  SKIP_IF_ETHERNET_NETIF_COUNT_LT(1);
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG_WITH_MCAST, false));
+  g_io.poll();
+  BOOST_CHECK_EQUAL(this->countEtherMcastFaces(), netifs.size());
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG_WITHOUT_MCAST, false));
+  g_io.poll();
+  BOOST_CHECK_EQUAL(this->countEtherMcastFaces(), 0);
+}
+
+BOOST_AUTO_TEST_CASE(ChangeMcastGroup)
+{
+  SKIP_IF_ETHERNET_NETIF_COUNT_LT(1);
+
+  const std::string CONFIG1 = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        mcast_group 01:00:00:00:00:01
+      }
+    }
+  )CONFIG";
+  const std::string CONFIG2 = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        mcast_group 01:00:00:00:00:02
+      }
+    }
+  )CONFIG";
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG1, false));
+  auto etherMcastFaces = this->listEtherMcastFaces();
+  BOOST_REQUIRE_EQUAL(etherMcastFaces.size(), netifs.size());
+  BOOST_CHECK_EQUAL(etherMcastFaces.front()->getRemoteUri(),
+                    FaceUri(ethernet::Address(0x01, 0x00, 0x00, 0x00, 0x00, 0x01)));
+
+  BOOST_CHECK_NO_THROW(parseConfig(CONFIG2, false));
+  g_io.poll();
+  etherMcastFaces = this->listEtherMcastFaces();
+  BOOST_REQUIRE_EQUAL(etherMcastFaces.size(), netifs.size());
+  BOOST_CHECK_EQUAL(etherMcastFaces.front()->getRemoteUri(),
+                    FaceUri(ethernet::Address(0x01, 0x00, 0x00, 0x00, 0x00, 0x02)));
+}
+
+BOOST_AUTO_TEST_CASE(BadMcast)
+{
+  const std::string CONFIG = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        mcast hello
+      }
+    }
+  )CONFIG";
+
+  BOOST_CHECK_THROW(parseConfig(CONFIG, true), ConfigFile::Error);
+  BOOST_CHECK_THROW(parseConfig(CONFIG, false), ConfigFile::Error);
+}
+
+BOOST_AUTO_TEST_CASE(BadMcastGroup)
+{
+  const std::string CONFIG = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        mcast yes
+        mcast_group hello
+      }
+    }
+  )CONFIG";
+
+  BOOST_CHECK_THROW(parseConfig(CONFIG, true), ConfigFile::Error);
+  BOOST_CHECK_THROW(parseConfig(CONFIG, false), ConfigFile::Error);
+}
+
+BOOST_AUTO_TEST_CASE(UnicastMcastGroup)
+{
+  const std::string CONFIG = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        mcast yes
+        mcast_group 02:00:00:00:00:01
+      }
+    }
+  )CONFIG";
+
+  BOOST_CHECK_THROW(parseConfig(CONFIG, true), ConfigFile::Error);
+  BOOST_CHECK_THROW(parseConfig(CONFIG, false), ConfigFile::Error);
+}
+
+BOOST_AUTO_TEST_CASE(UnknownOption)
+{
+  const std::string CONFIG = R"CONFIG(
+    face_system
+    {
+      ether
+      {
+        hello
+      }
+    }
+  )CONFIG";
+
+  BOOST_CHECK_THROW(parseConfig(CONFIG, true), ConfigFile::Error);
+  BOOST_CHECK_THROW(parseConfig(CONFIG, false), ConfigFile::Error);
+}
+
+BOOST_AUTO_TEST_SUITE_END() // ProcessConfig
 
 BOOST_AUTO_TEST_CASE(GetChannels)
 {
@@ -44,24 +302,6 @@
   BOOST_CHECK_EQUAL(channels.empty(), true);
 }
 
-BOOST_AUTO_TEST_CASE(MulticastFacesMap)
-{
-  SKIP_IF_NETWORK_INTERFACE_COUNT_LT(1);
-
-  EthernetFactory factory;
-  auto face1 = factory.createMulticastFace(m_interfaces.front(), ethernet::getBroadcastAddress());
-  auto face1bis = factory.createMulticastFace(m_interfaces.front(), ethernet::getBroadcastAddress());
-  BOOST_CHECK_EQUAL(face1, face1bis);
-
-  auto face2 = factory.createMulticastFace(m_interfaces.front(), ethernet::getDefaultMulticastAddress());
-  BOOST_CHECK_NE(face1, face2);
-
-  SKIP_IF_NETWORK_INTERFACE_COUNT_LT(2);
-
-  auto face3 = factory.createMulticastFace(m_interfaces.back(), ethernet::getBroadcastAddress());
-  BOOST_CHECK_NE(face1, face3);
-}
-
 BOOST_AUTO_TEST_CASE(UnsupportedFaceCreate)
 {
   EthernetFactory factory;
@@ -89,4 +329,5 @@
 BOOST_AUTO_TEST_SUITE_END() // Face
 
 } // namespace tests
+} // namespace face
 } // namespace nfd
diff --git a/tests/daemon/face/network-interface-fixture.hpp b/tests/daemon/face/ethernet-fixture.hpp
similarity index 65%
rename from tests/daemon/face/network-interface-fixture.hpp
rename to tests/daemon/face/ethernet-fixture.hpp
index bb85ee7..4228f79 100644
--- a/tests/daemon/face/network-interface-fixture.hpp
+++ b/tests/daemon/face/ethernet-fixture.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2015,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -23,8 +23,8 @@
  * NFD, e.g., in COPYING.md file.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef NFD_TESTS_DAEMON_FACE_NETWORK_INTERFACE_FIXTURE_HPP
-#define NFD_TESTS_DAEMON_FACE_NETWORK_INTERFACE_FIXTURE_HPP
+#ifndef NFD_TESTS_DAEMON_FACE_ETHERNET_FIXTURE_HPP
+#define NFD_TESTS_DAEMON_FACE_ETHERNET_FIXTURE_HPP
 
 #include "core/network-interface.hpp"
 #include "face/ethernet-transport.hpp"
@@ -35,39 +35,41 @@
 namespace face {
 namespace tests {
 
-class NetworkInterfaceFixture : public nfd::tests::BaseFixture
+class EthernetFixture : public virtual nfd::tests::BaseFixture
 {
 protected:
-  NetworkInterfaceFixture()
+  EthernetFixture()
   {
     for (const auto& netif : listNetworkInterfaces()) {
       if (!netif.isLoopback() && netif.isUp()) {
         try {
-          face::EthernetTransport transport(netif, ethernet::getBroadcastAddress());
-          m_interfaces.push_back(netif);
+          EthernetTransport transport(netif, ethernet::getBroadcastAddress());
+          netifs.push_back(netif);
         }
-        catch (const face::EthernetTransport::Error&) {
-          // pass
+        catch (const EthernetTransport::Error&) {
+          // ignore
         }
       }
     }
   }
 
 protected:
-  std::vector<NetworkInterfaceInfo> m_interfaces;
+  /** \brief EthernetTransport-capable network interfaces
+   */
+  std::vector<NetworkInterfaceInfo> netifs;
 };
 
-#define SKIP_IF_NETWORK_INTERFACE_COUNT_LT(n) \
-  do {                                        \
-    if (this->m_interfaces.size() < (n)) {    \
-      BOOST_WARN_MESSAGE(false, "skipping assertions that require " \
-                                #n " or more network interfaces");  \
-      return;                                 \
-    }                                         \
+#define SKIP_IF_ETHERNET_NETIF_COUNT_LT(n) \
+  do { \
+    if (this->netifs.size() < (n)) { \
+      BOOST_WARN_MESSAGE(false, "skipping assertions that require " #n \
+                                " or more EthernetTransport-capable network interfaces"); \
+      return; \
+    } \
   } while (false)
 
 } // namespace tests
 } // namespace face
 } // namespace nfd
 
-#endif // NFD_TESTS_DAEMON_FACE_NETWORK_INTERFACE_FIXTURE_HPP
+#endif // NFD_TESTS_DAEMON_FACE_ETHERNET_FIXTURE_HPP
diff --git a/tests/daemon/face/ethernet-transport.t.cpp b/tests/daemon/face/ethernet-transport.t.cpp
index f7948a2..4cc9ceb 100644
--- a/tests/daemon/face/ethernet-transport.t.cpp
+++ b/tests/daemon/face/ethernet-transport.t.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2015,  Regents of the University of California,
+ * Copyright (c) 2014-2017,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -24,9 +24,9 @@
  */
 
 #include "face/ethernet-transport.hpp"
-#include "transport-test-common.hpp"
 
-#include "network-interface-fixture.hpp"
+#include "transport-test-common.hpp"
+#include "ethernet-fixture.hpp"
 
 namespace nfd {
 namespace face {
@@ -35,13 +35,13 @@
 using namespace nfd::tests;
 
 BOOST_AUTO_TEST_SUITE(Face)
-BOOST_FIXTURE_TEST_SUITE(TestEthernetTransport, NetworkInterfaceFixture)
+BOOST_FIXTURE_TEST_SUITE(TestEthernetTransport, EthernetFixture)
 
 BOOST_AUTO_TEST_CASE(StaticProperties)
 {
-  SKIP_IF_NETWORK_INTERFACE_COUNT_LT(1);
+  SKIP_IF_ETHERNET_NETIF_COUNT_LT(1);
 
-  auto netif = m_interfaces.front();
+  auto netif = netifs.front();
   EthernetTransport transport(netif, ethernet::getDefaultMulticastAddress());
   checkStaticPropertiesInitialized(transport);
 
@@ -52,9 +52,9 @@
   BOOST_CHECK_EQUAL(transport.getLinkType(), ndn::nfd::LINK_TYPE_MULTI_ACCESS);
 }
 
-// TODO add the equivalent of these test cases from ethernet.t.cpp as of commit:65caf200924b28748037750449e28bcb548dbc9c
-// SendPacket
-// ProcessIncomingPacket
+///\todo #3369 add the equivalent of these test cases from ethernet.t.cpp
+///      as of commit:65caf200924b28748037750449e28bcb548dbc9c
+///      SendPacket, ProcessIncomingPacket
 
 BOOST_AUTO_TEST_SUITE_END() // TestEthernetTransport
 BOOST_AUTO_TEST_SUITE_END() // Face
diff --git a/tests/daemon/face/face-system-fixture.hpp b/tests/daemon/face/face-system-fixture.hpp
index 6bdbb41..2198c7e 100644
--- a/tests/daemon/face/face-system-fixture.hpp
+++ b/tests/daemon/face/face-system-fixture.hpp
@@ -26,6 +26,7 @@
 #ifndef NFD_TESTS_DAEMON_FACE_FACE_SYSTEM_FIXTURE_HPP
 #define NFD_TESTS_DAEMON_FACE_FACE_SYSTEM_FIXTURE_HPP
 
+#include "face/face.hpp"
 #include "face/face-system.hpp"
 #include "fw/face-table.hpp"
 
@@ -37,7 +38,7 @@
 
 using namespace nfd::tests;
 
-class FaceSystemFixture : public BaseFixture
+class FaceSystemFixture : public virtual BaseFixture
 {
 public:
   FaceSystemFixture()
@@ -82,6 +83,26 @@
     return *factory;
   }
 
+  /** \brief list faces of specified scheme from FaceTable
+   *  \param scheme local or remote FaceUri scheme
+   *  \param linkType if not NONE, filter by specified LinkType
+   */
+  std::vector<const Face*>
+  listFacesByScheme(const std::string& scheme,
+                    ndn::nfd::LinkType linkType = ndn::nfd::LINK_TYPE_NONE) const
+  {
+    std::vector<const Face*> faces;
+    for (const Face& face : faceTable) {
+      if ((face.getLocalUri().getScheme() == scheme ||
+           face.getRemoteUri().getScheme() == scheme) &&
+          (linkType == ndn::nfd::LINK_TYPE_NONE ||
+           face.getLinkType() == linkType)) {
+        faces.push_back(&face);
+      }
+    }
+    return faces;
+  }
+
 protected:
   ConfigFile configFile;
   FaceTable faceTable;
diff --git a/tests/daemon/face/face-system.t.cpp b/tests/daemon/face/face-system.t.cpp
index bc398ca..826ebc4 100644
--- a/tests/daemon/face/face-system.t.cpp
+++ b/tests/daemon/face/face-system.t.cpp
@@ -27,13 +27,7 @@
 #include "face-system-fixture.hpp"
 
 // ProtocolFactory includes, sorted alphabetically
-#ifdef HAVE_LIBPCAP
-#include "face/ethernet-factory.hpp"
-#endif // HAVE_LIBPCAP
 #include "face/udp-factory.hpp"
-#ifdef HAVE_UNIX_SOCKETS
-#include "face/unix-stream-factory.hpp"
-#endif // HAVE_UNIX_SOCKETS
 #ifdef HAVE_WEBSOCKET
 #include "face/websocket-factory.hpp"
 #endif // HAVE_WEBSOCKET
@@ -409,128 +403,6 @@
 }
 BOOST_AUTO_TEST_SUITE_END() // ConfigUdp
 
-#ifdef HAVE_LIBPCAP
-BOOST_AUTO_TEST_SUITE(ConfigEther)
-
-BOOST_AUTO_TEST_CASE(Normal)
-{
-  SKIP_IF_NOT_SUPERUSER();
-
-  const std::string CONFIG = R"CONFIG(
-    face_system
-    {
-      ether
-      {
-        mcast yes
-        mcast_group 01:00:5E:00:17:AA
-        whitelist
-        {
-          *
-        }
-        blacklist
-        {
-        }
-      }
-    }
-  )CONFIG";
-
-  BOOST_CHECK_NO_THROW(parseConfig(CONFIG, true));
-  BOOST_CHECK_NO_THROW(parseConfig(CONFIG, false));
-
-  auto& factory = this->getFactoryByScheme<EthernetFactory>("ether");
-  BOOST_CHECK_EQUAL(factory.getChannels().size(), 0);
-}
-
-BOOST_AUTO_TEST_CASE(BadMcast)
-{
-  const std::string CONFIG = R"CONFIG(
-    face_system
-    {
-      ether
-      {
-        mcast hello
-      }
-    }
-  )CONFIG";
-
-  BOOST_CHECK_THROW(parseConfig(CONFIG, true), ConfigFile::Error);
-  BOOST_CHECK_THROW(parseConfig(CONFIG, false), ConfigFile::Error);
-}
-
-BOOST_AUTO_TEST_CASE(BadMcastGroup)
-{
-  const std::string CONFIG = R"CONFIG(
-    face_system
-    {
-      ether
-      {
-        mcast yes
-        mcast_group
-      }
-    }
-  )CONFIG";
-
-  BOOST_CHECK_THROW(parseConfig(CONFIG, true), ConfigFile::Error);
-  BOOST_CHECK_THROW(parseConfig(CONFIG, false), ConfigFile::Error);
-}
-
-BOOST_AUTO_TEST_CASE(UnknownOption)
-{
-  const std::string CONFIG = R"CONFIG(
-    face_system
-    {
-      ether
-      {
-        hello
-      }
-    }
-  )CONFIG";
-
-  BOOST_CHECK_THROW(parseConfig(CONFIG, true), ConfigFile::Error);
-  BOOST_CHECK_THROW(parseConfig(CONFIG, false), ConfigFile::Error);
-}
-
-BOOST_AUTO_TEST_CASE(MulticastReinit)
-{
-  SKIP_IF_NOT_SUPERUSER();
-
-  const std::string CONFIG_WITH_MCAST = R"CONFIG(
-    face_system
-    {
-      ether
-      {
-        mcast yes
-      }
-    }
-  )CONFIG";
-
-  BOOST_CHECK_NO_THROW(parseConfig(CONFIG_WITH_MCAST, false));
-
-  auto& factory = this->getFactoryByScheme<EthernetFactory>("ether");
-
-  if (factory.getMulticastFaces().empty()) {
-    BOOST_WARN_MESSAGE(false, "skipping assertions that require at least one Ethernet multicast face");
-    return;
-  }
-
-  const std::string CONFIG_WITHOUT_MCAST = R"CONFIG(
-    face_system
-    {
-      ether
-      {
-        mcast no
-      }
-    }
-  )CONFIG";
-
-  BOOST_CHECK_NO_THROW(parseConfig(CONFIG_WITHOUT_MCAST, false));
-  BOOST_REQUIRE_NO_THROW(g_io.poll());
-  BOOST_CHECK_EQUAL(factory.getMulticastFaces().size(), 0);
-}
-
-BOOST_AUTO_TEST_SUITE_END() // ConfigEther
-#endif // HAVE_LIBPCAP
-
 #ifdef HAVE_WEBSOCKET
 BOOST_AUTO_TEST_SUITE(ConfigWebSocket)