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/daemon/face/ethernet-factory.cpp b/daemon/face/ethernet-factory.cpp
index cfb4b79..53a0815 100644
--- a/daemon/face/ethernet-factory.cpp
+++ b/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 @@
}
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
diff --git a/daemon/face/ethernet-factory.hpp b/daemon/face/ethernet-factory.hpp
index 8e11024..1dec480 100644
--- a/daemon/face/ethernet-factory.hpp
+++ b/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
diff --git a/daemon/face/ethernet-transport.cpp b/daemon/face/ethernet-transport.cpp
index aba8b59..5ea385b 100644
--- a/daemon/face/ethernet-transport.cpp
+++ b/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 @@
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());
}
diff --git a/daemon/face/face-system.cpp b/daemon/face/face-system.cpp
index b084220..6ce6afe 100644
--- a/daemon/face/face-system.cpp
+++ b/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 @@
: 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 @@
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 @@
///\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);
@@ -302,89 +303,6 @@
}
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)
{
// ; the websocket section contains settings of WebSocket faces and channels
diff --git a/daemon/face/face-system.hpp b/daemon/face/face-system.hpp
index f5d26d3..5696aa7 100644
--- a/daemon/face/face-system.hpp
+++ b/daemon/face/face-system.hpp
@@ -76,10 +76,10 @@
{
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 @@
///\todo add NetworkMonitor
private:
- std::vector<NetworkInterfaceInfo> m_nicList;
+ std::vector<NetworkInterfaceInfo> m_netifs;
friend class FaceSystem;
};
@@ -103,10 +103,6 @@
const std::vector<NetworkInterfaceInfo>& nicList);
void
- processSectionEther(const ConfigSection& configSection, bool isDryRun,
- const std::vector<NetworkInterfaceInfo>& nicList);
-
- void
processSectionWebSocket(const ConfigSection& configSection, bool isDryRun);
PUBLIC_WITH_TESTS_ELSE_PRIVATE: