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

core/network-interface-predicate.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,
@@ -95,12 +95,12 @@ NetworkInterfacePredicate::parseBlacklist(const boost::property_tree::ptree& lis
 }
 
 static bool
-doesMatchRule(const NetworkInterfaceInfo& nic, const std::string& rule)
+doesMatchRule(const NetworkInterfaceInfo& netif, const std::string& rule)
 {
   // if '/' is in rule, this is a subnet, check if IP in subnet
   if (rule.find('/') != std::string::npos) {
     Network n = boost::lexical_cast<Network>(rule);
-    for (const auto& addr : nic.ipv4Addresses) {
+    for (const auto& addr : netif.ipv4Addresses) {
       if (n.doesContain(addr)) {
         return true;
       }
@@ -108,15 +108,22 @@ doesMatchRule(const NetworkInterfaceInfo& nic, const std::string& rule)
   }
 
   return rule == "*" ||
-         nic.name == rule ||
-         nic.etherAddress.toString() == rule;
+         netif.name == rule ||
+         netif.etherAddress.toString() == rule;
 }
 
 bool
-NetworkInterfacePredicate::operator()(const NetworkInterfaceInfo& nic) const
+NetworkInterfacePredicate::operator()(const NetworkInterfaceInfo& netif) const
 {
-  return std::any_of(m_whitelist.begin(), m_whitelist.end(), bind(&doesMatchRule, nic, _1)) &&
-         std::none_of(m_blacklist.begin(), m_blacklist.end(), bind(&doesMatchRule, nic, _1));
+  return std::any_of(m_whitelist.begin(), m_whitelist.end(), bind(&doesMatchRule, netif, _1)) &&
+         std::none_of(m_blacklist.begin(), m_blacklist.end(), bind(&doesMatchRule, netif, _1));
+}
+
+bool
+NetworkInterfacePredicate::operator==(const NetworkInterfacePredicate& other) const
+{
+  return this->m_whitelist == other.m_whitelist &&
+         this->m_blacklist == other.m_blacklist;
 }
 
 } // namespace nfd

core/network-interface-predicate.hpp

@@ -1,12 +1,12 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /**
- * Copyright (c) 2014-2016, Regents of the University of California,
- *                          Arizona Board of Regents,
- *                          Colorado State University,
- *                          University Pierre & Marie Curie, Sorbonne University,
- *                          Washington University in St. Louis,
- *                          Beijing Institute of Technology,
- *                          The University of Memphis
+ * Copyright (c) 2014-2017,  Regents of the University of California,
+ *                           Arizona Board of Regents,
+ *                           Colorado State University,
+ *                           University Pierre & Marie Curie, Sorbonne University,
+ *                           Washington University in St. Louis,
+ *                           Beijing Institute of Technology,
+ *                           The University of Memphis.
  *
  * This file is part of NFD (Named Data Networking Forwarding Daemon).
  * See AUTHORS.md for complete list of NFD authors and contributors.
@@ -41,7 +41,6 @@ class NetworkInterfaceInfo;
  * all interfaces. A NetworkInterfaceInfo is accepted if it matches any entry in the whitelist and none
  * of the entries in the blacklist.
  */
-
 class NetworkInterfacePredicate
 {
 public:
@@ -60,7 +59,16 @@ public:
   parseBlacklist(const boost::property_tree::ptree& list);
 
   bool
-  operator()(const NetworkInterfaceInfo& nic) const;
+  operator()(const NetworkInterfaceInfo& netif) const;
+
+  bool
+  operator==(const NetworkInterfacePredicate& other) const;
+
+  bool
+  operator!=(const NetworkInterfacePredicate& other) const
+  {
+    return !this->operator==(other);
+  }
 
 private:
   std::set<std::string> m_whitelist;

daemon/face/ethernet-factory.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,
@@ -26,33 +26,93 @@
 #include "ethernet-factory.hpp"
 #include "ethernet-transport.hpp"
 #include "generic-link-service.hpp"
+#include "core/logger.hpp"
+#include <boost/range/adaptors.hpp>
+#include <boost/range/algorithm/copy.hpp>
 
 namespace nfd {
+namespace face {
 
-shared_ptr<Face>
-EthernetFactory::createMulticastFace(const NetworkInterfaceInfo& interface,
-                                     const ethernet::Address& address)
+NFD_LOG_INIT("EthernetFactory");
+
+void
+EthernetFactory::processConfig(OptionalConfigSection configSection,
+                               FaceSystem::ConfigContext& context)
 {
-  if (!address.isMulticast())
-    BOOST_THROW_EXCEPTION(Error(address.toString() + " is not a multicast address"));
+  // ether
+  // {
+  //   mcast yes
+  //   mcast_group 01:00:5E:00:17:AA
+  //   whitelist
+  //   {
+  //     *
+  //   }
+  //   blacklist
+  //   {
+  //   }
+  // }
 
-  auto face = findMulticastFace(interface.name, address);
-  if (face)
-    return face;
+  MulticastConfig mcastConfig;
 
-  face::GenericLinkService::Options opts;
-  opts.allowFragmentation = true;
-  opts.allowReassembly = true;
+  if (configSection) {
+    // face_system.ether.mcast defaults to 'yes' but only if face_system.ether section is present
+    mcastConfig.isEnabled = true;
 
-  auto linkService = make_unique<face::GenericLinkService>(opts);
-  auto transport = make_unique<face::EthernetTransport>(interface, address);
-  face = make_shared<Face>(std::move(linkService), std::move(transport));
+    for (const auto& pair : *configSection) {
+      const std::string& key = pair.first;
+      const ConfigSection& value = pair.second;
 
-  auto key = std::make_pair(interface.name, address);
-  m_multicastFaces[key] = face;
-  connectFaceClosedSignal(*face, [this, key] { m_multicastFaces.erase(key); });
+      if (key == "mcast") {
+        mcastConfig.isEnabled = ConfigFile::parseYesNo(pair, "ether");
+      }
+      else if (key == "mcast_group") {
+        const std::string& valueStr = value.get_value<std::string>();
+        mcastConfig.group = ethernet::Address::fromString(valueStr);
+        if (mcastConfig.group.isNull()) {
+          BOOST_THROW_EXCEPTION(ConfigFile::Error("face_system.ether.mcast_group: '" +
+                                valueStr + "' cannot be parsed as an Ethernet address"));
+        }
+        else if (!mcastConfig.group.isMulticast()) {
+          BOOST_THROW_EXCEPTION(ConfigFile::Error("face_system.ether.mcast_group: '" +
+                                valueStr + "' is not a multicast address"));
+        }
+      }
+      else if (key == "whitelist") {
+        mcastConfig.netifPredicate.parseWhitelist(value);
+      }
+      else if (key == "blacklist") {
+        mcastConfig.netifPredicate.parseBlacklist(value);
+      }
+      else {
+        BOOST_THROW_EXCEPTION(ConfigFile::Error("Unrecognized option face_system.ether." + key));
+      }
+    }
+  }
 
-  return face;
+  if (!context.isDryRun) {
+    if (m_mcastConfig.isEnabled != mcastConfig.isEnabled) {
+      if (mcastConfig.isEnabled) {
+        NFD_LOG_INFO("enabling multicast on " << mcastConfig.group);
+      }
+      else {
+        NFD_LOG_INFO("disabling multicast");
+      }
+    }
+    else if (m_mcastConfig.group != mcastConfig.group) {
+      NFD_LOG_INFO("changing multicast group from " << m_mcastConfig.group <<
+                   " to " << mcastConfig.group);
+    }
+    else if (m_mcastConfig.netifPredicate != mcastConfig.netifPredicate) {
+      NFD_LOG_INFO("changing whitelist/blacklist");
+    }
+    else {
+      // There's no configuration change, but we still need to re-apply configuration because
+      // netifs may have changed.
+    }
+
+    m_mcastConfig = mcastConfig;
+    this->applyConfig(context);
+  }
 }
 
 void
@@ -72,14 +132,75 @@ EthernetFactory::getChannels() const
 }
 
 shared_ptr<Face>
-EthernetFactory::findMulticastFace(const std::string& interfaceName,
-                                   const ethernet::Address& address) const
+EthernetFactory::createMulticastFace(const NetworkInterfaceInfo& netif,
+                                     const ethernet::Address& address)
 {
-  auto i = m_multicastFaces.find({interfaceName, address});
-  if (i != m_multicastFaces.end())
-    return i->second;
-  else
-    return nullptr;
+  BOOST_ASSERT(address.isMulticast());
+
+  auto key = std::make_pair(netif.name, address);
+  auto found = m_mcastFaces.find(key);
+  if (found != m_mcastFaces.end()) {
+    return found->second;
+  }
+
+  face::GenericLinkService::Options opts;
+  opts.allowFragmentation = true;
+  opts.allowReassembly = true;
+
+  auto linkService = make_unique<face::GenericLinkService>(opts);
+  auto transport = make_unique<face::EthernetTransport>(netif, address);
+  auto face = make_shared<Face>(std::move(linkService), std::move(transport));
+
+  m_mcastFaces[key] = face;
+  connectFaceClosedSignal(*face, [this, key] { m_mcastFaces.erase(key); });
+
+  return face;
 }
 
+void
+EthernetFactory::applyConfig(const FaceSystem::ConfigContext& context)
+{
+  // collect old faces
+  std::set<shared_ptr<Face>> oldFaces;
+  boost::copy(m_mcastFaces | boost::adaptors::map_values,
+              std::inserter(oldFaces, oldFaces.end()));
+
+  if (m_mcastConfig.isEnabled) {
+    // determine interfaces on which faces should be created or retained
+    auto capableNetifs = context.listNetifs() |
+                         boost::adaptors::filtered([this] (const NetworkInterfaceInfo& netif) {
+                           return netif.isUp() && netif.isMulticastCapable() &&
+                                  m_mcastConfig.netifPredicate(netif);
+                         });
+
+    // create faces
+    for (const auto& netif : capableNetifs) {
+      shared_ptr<Face> face;
+      try {
+        face = this->createMulticastFace(netif, m_mcastConfig.group);
+      }
+      catch (const EthernetTransport::Error& e) {
+        NFD_LOG_ERROR("Cannot create Ethernet multicast face on " << netif.name << ": " <<
+                      e.what() << ", continuing");
+        continue;
+      }
+
+      if (face->getId() == face::INVALID_FACEID) {
+        // new face: register with forwarding
+        context.addFace(face);
+      }
+      else {
+        // existing face: don't destroy
+        oldFaces.erase(face);
+      }
+    }
+  }
+
+  // destroy old faces that are not needed in new configuration
+  for (const auto& face : oldFaces) {
+    face->close();
+  }
+}
+
+} // namespace face
 } // namespace nfd

daemon/face/ethernet-factory.hpp

@@ -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,
@@ -27,86 +27,66 @@
 #define NFD_DAEMON_FACE_ETHERNET_FACTORY_HPP
 
 #include "protocol-factory.hpp"
-#include "core/network-interface.hpp"
 
 namespace nfd {
+namespace face {
 
+/** \brief protocol factory for Ethernet
+ *
+ *  Currently, only Ethernet multicast is supported.
+ */
 class EthernetFactory : public ProtocolFactory
 {
 public:
-  /**
-   * \brief Exception of EthernetFactory
+  /** \brief process face_system.ether config section
    */
-  class Error : public ProtocolFactory::Error
-  {
-  public:
-    explicit
-    Error(const std::string& what)
-      : ProtocolFactory::Error(what)
-    {
-    }
-  };
+  void
+  processConfig(OptionalConfigSection configSection,
+                FaceSystem::ConfigContext& context) override;
 
-  typedef std::map<std::pair<std::string, ethernet::Address>,
-                   shared_ptr<Face>> MulticastFaceMap;
-
-  /**
-   * \brief Create an EthernetFace to communicate with the given multicast group
-   *
-   * If this method is called twice with the same interface and group, only
-   * one face will be created. Instead, the second call will just retrieve
-   * the existing face.
-   *
-   * \param interface Local network interface
-   * \param address   Ethernet broadcast/multicast destination address
-   *
-   * \returns always a valid shared pointer to an EthernetFace object,
-   *          an exception will be thrown if the creation fails
-   *
-   * \throws EthernetFactory::Error or EthernetTransport::Error
+  /** \brief unicast face creation is not supported and will always fail
    */
-  shared_ptr<Face>
-  createMulticastFace(const NetworkInterfaceInfo& interface,
-                      const ethernet::Address& address);
-
-  /**
-   * \brief Get map of configured multicast faces
-   */
-  const MulticastFaceMap&
-  getMulticastFaces() const;
-
-public: // from ProtocolFactory
-  virtual void
+  void
   createFace(const FaceUri& uri,
              ndn::nfd::FacePersistency persistency,
              bool wantLocalFieldsEnabled,
              const FaceCreatedCallback& onCreated,
              const FaceCreationFailedCallback& onFailure) override;
 
-  virtual std::vector<shared_ptr<const Channel>>
+  /** \return empty container, because Ethernet unicast is not supported
+   */
+  std::vector<shared_ptr<const Channel>>
   getChannels() const override;
 
 private:
-  /**
-   * \brief Look up EthernetFace using specified interface and address
-   *
-   * \returns shared pointer to the existing EthernetFace object
-   *          or nullptr when such face does not exist
+  /** \brief Create a face to communicate on the given Ethernet multicast group
+   *  \param netif local network interface
+   *  \param group multicast group address
+   *  \note Calling this method again with same arguments returns the existing face on the given
+   *        interface and multicast group rather than creating a new one.
+   *  \throw EthernetTransport::Error transport creation fails
    */
   shared_ptr<Face>
-  findMulticastFace(const std::string& interfaceName,
-                    const ethernet::Address& address) const;
+  createMulticastFace(const NetworkInterfaceInfo& netif, const ethernet::Address& group);
+
+  void
+  applyConfig(const FaceSystem::ConfigContext& context);
 
 private:
-  MulticastFaceMap m_multicastFaces;
+  struct MulticastConfig
+  {
+    bool isEnabled = false;
+    ethernet::Address group = ethernet::getDefaultMulticastAddress();
+    NetworkInterfacePredicate netifPredicate;
+  };
+
+  MulticastConfig m_mcastConfig;
+
+  /// (ifname, group) => face
+  std::map<std::pair<std::string, ethernet::Address>, shared_ptr<Face>> m_mcastFaces;
 };
 
-inline const EthernetFactory::MulticastFaceMap&
-EthernetFactory::getMulticastFaces() const
-{
-  return m_multicastFaces;
-}
-
+} // namespace face
 } // namespace nfd
 
 #endif // NFD_DAEMON_FACE_ETHERNET_FACTORY_HPP

daemon/face/ethernet-transport.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,
@@ -374,14 +374,15 @@ EthernetTransport::processIncomingPacket(const pcap_pkthdr* header, const uint8_
 void
 EthernetTransport::processErrorCode(const boost::system::error_code& error)
 {
-  NFD_LOG_FACE_TRACE(__func__);
-
-  if (getState() == TransportState::CLOSING ||
+  // boost::asio::error::operation_aborted must be checked first. In that situation, the Transport
+  // may already have been destructed, and it's unsafe to call getState() or do logging.
+  if (error == boost::asio::error::operation_aborted ||
+      getState() == TransportState::CLOSING ||
       getState() == TransportState::FAILED ||
-      getState() == TransportState::CLOSED ||
-      error == boost::asio::error::operation_aborted)
+      getState() == TransportState::CLOSED) {
     // transport is shutting down, ignore any errors
     return;
+  }
 
   NFD_LOG_FACE_WARN("Receive operation failed: " << error.message());
 }

daemon/face/face-system.cpp

@@ -30,7 +30,6 @@
 // ProtocolFactory includes, sorted alphabetically
 #ifdef HAVE_LIBPCAP
 #include "ethernet-factory.hpp"
-#include "ethernet-transport.hpp"
 #endif // HAVE_LIBPCAP
 #include "tcp-factory.hpp"
 #include "udp-factory.hpp"
@@ -50,6 +49,11 @@ FaceSystem::FaceSystem(FaceTable& faceTable)
   : m_faceTable(faceTable)
 {
   ///\todo #3904 make a registry, and construct instances from registry
+
+#ifdef HAVE_LIBPCAP
+  m_factories["ether"] = make_shared<EthernetFactory>();
+#endif // HAVE_LIBPCAP
+
   m_factories["tcp"] = make_shared<TcpFactory>();
 
 #ifdef HAVE_UNIX_SOCKETS
@@ -93,7 +97,7 @@ FaceSystem::processConfig(const ConfigSection& configSection, bool isDryRun, con
   ConfigContext context;
   context.isDryRun = isDryRun;
   context.addFace = bind(&FaceTable::add, &m_faceTable, _1);
-  context.m_nicList = listNetworkInterfaces();
+  context.m_netifs = listNetworkInterfaces();
 
   // process sections in protocol factories
   for (const auto& pair : m_factories) {
@@ -132,10 +136,7 @@ FaceSystem::processConfig(const ConfigSection& configSection, bool isDryRun, con
 
     ///\todo #3904 process these in protocol factory
     if (sectionName == "udp") {
-      processSectionUdp(subSection, isDryRun, context.m_nicList);
-    }
-    else if (sectionName == "ether") {
-      processSectionEther(subSection, isDryRun, context.m_nicList);
+      processSectionUdp(subSection, isDryRun, context.m_netifs);
     }
     else if (sectionName == "websocket") {
       processSectionWebSocket(subSection, isDryRun);
@@ -301,89 +302,6 @@ FaceSystem::processSectionUdp(const ConfigSection& configSection, bool isDryRun,
   }
 }
 
-void
-FaceSystem::processSectionEther(const ConfigSection& configSection, bool isDryRun,
-                                const std::vector<NetworkInterfaceInfo>& nicList)
-{
-  // ; the ether section contains settings of Ethernet faces and channels
-  // ether
-  // {
-  //   ; NFD creates one Ethernet multicast face per NIC
-  //   mcast yes ; set to 'no' to disable Ethernet multicast, default 'yes'
-  //   mcast_group 01:00:5E:00:17:AA ; Ethernet multicast group
-  // }
-
-#if defined(HAVE_LIBPCAP)
-  NetworkInterfacePredicate nicPredicate;
-  bool useMcast = true;
-  ethernet::Address mcastGroup(ethernet::getDefaultMulticastAddress());
-
-  for (const auto& i : configSection) {
-    if (i.first == "mcast") {
-      useMcast = ConfigFile::parseYesNo(i, "ether");
-    }
-    else if (i.first == "mcast_group") {
-      mcastGroup = ethernet::Address::fromString(i.second.get_value<std::string>());
-      if (mcastGroup.isNull()) {
-        BOOST_THROW_EXCEPTION(ConfigFile::Error("Invalid value for option \"" +
-                                                i.first + "\" in \"ether\" section"));
-      }
-      NFD_LOG_TRACE("Ethernet multicast group set to " << mcastGroup);
-    }
-    else if (i.first == "whitelist") {
-      nicPredicate.parseWhitelist(i.second);
-    }
-    else if (i.first == "blacklist") {
-      nicPredicate.parseBlacklist(i.second);
-    }
-    else {
-      BOOST_THROW_EXCEPTION(ConfigFile::Error("Unrecognized option \"" +
-                                              i.first + "\" in \"ether\" section"));
-    }
-  }
-
-  if (!isDryRun) {
-    shared_ptr<EthernetFactory> factory;
-    if (m_factoryByScheme.count("ether") > 0) {
-      factory = static_pointer_cast<EthernetFactory>(m_factoryByScheme["ether"]);
-    }
-    else {
-      factory = make_shared<EthernetFactory>();
-      m_factoryByScheme.emplace("ether", factory);
-    }
-
-    std::set<shared_ptr<Face>> multicastFacesToRemove;
-    for (const auto& i : factory->getMulticastFaces()) {
-      multicastFacesToRemove.insert(i.second);
-    }
-
-    if (useMcast) {
-      for (const auto& nic : nicList) {
-        if (nic.isUp() && nic.isMulticastCapable() && nicPredicate(nic)) {
-          try {
-            auto newFace = factory->createMulticastFace(nic, mcastGroup);
-            m_faceTable.add(newFace);
-            multicastFacesToRemove.erase(newFace);
-          }
-          catch (const EthernetFactory::Error& factoryError) {
-            NFD_LOG_ERROR(factoryError.what() << ", continuing");
-          }
-          catch (const EthernetTransport::Error& faceError) {
-            NFD_LOG_ERROR(faceError.what() << ", continuing");
-          }
-        }
-      }
-    }
-
-    for (const auto& face : multicastFacesToRemove) {
-      face->close();
-    }
-  }
-#else
-  BOOST_THROW_EXCEPTION(ConfigFile::Error("NFD was compiled without libpcap, cannot process \"ether\" section"));
-#endif // HAVE_LIBPCAP
-}
-
 void
 FaceSystem::processSectionWebSocket(const ConfigSection& configSection, bool isDryRun)
 {

daemon/face/face-system.hpp

@@ -76,10 +76,10 @@ public:
   {
   public:
     const std::vector<NetworkInterfaceInfo>&
-    listNics() const
+    listNetifs() const
     {
-      ///\todo get NIC list from NetworkMonitor
-      return m_nicList;
+      ///\todo get netifs from NetworkMonitor
+      return m_netifs;
     }
 
   public:
@@ -88,7 +88,7 @@ public:
     ///\todo add NetworkMonitor
 
   private:
-    std::vector<NetworkInterfaceInfo> m_nicList;
+    std::vector<NetworkInterfaceInfo> m_netifs;
 
     friend class FaceSystem;
   };
@@ -102,10 +102,6 @@ private:
   processSectionUdp(const ConfigSection& configSection, bool isDryRun,
                     const std::vector<NetworkInterfaceInfo>& nicList);
 
-  void
-  processSectionEther(const ConfigSection& configSection, bool isDryRun,
-                      const std::vector<NetworkInterfaceInfo>& nicList);
-
   void
   processSectionWebSocket(const ConfigSection& configSection, bool isDryRun);
 

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_AUTO_TEST_CASE(GetChannels)
   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() // TestEthernetFactory
 BOOST_AUTO_TEST_SUITE_END() // Face
 
 } // namespace tests
+} // namespace face
 } // namespace nfd

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 nfd {
 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

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 @@ namespace tests {
 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_AUTO_TEST_CASE(StaticProperties)
   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

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 @@ namespace tests {
 
 using namespace nfd::tests;
 
-class FaceSystemFixture : public BaseFixture
+class FaceSystemFixture : public virtual BaseFixture
 {
 public:
   FaceSystemFixture()
@@ -82,6 +83,26 @@ public:
     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;

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_CASE(MulticastReinit)
 }
 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)