face: more robust handling of multicast face creation errors

Refs: #5030, #5297
Change-Id: I3ca985498d9d3756cbff94873063f3734bcea288

daemon/face/ethernet-channel.cpp

@@ -101,8 +101,9 @@ void
 EthernetChannel::asyncRead(const FaceCreatedCallback& onFaceCreated,
                            const FaceCreationFailedCallback& onReceiveFailed)
 {
-  m_socket.async_wait(boost::asio::posix::stream_descriptor::wait_read,
-                      [=] (const auto& e) { this->handleRead(e, onFaceCreated, onReceiveFailed); });
+  m_socket.async_wait(boost::asio::posix::stream_descriptor::wait_read, [=] (const auto& e) {
+    handleRead(e, onFaceCreated, onReceiveFailed);
+  });
 }
 
 void

daemon/face/ethernet-factory.cpp

@@ -26,7 +26,6 @@
 #include "ethernet-factory.hpp"
 #include "generic-link-service.hpp"
 #include "multicast-ethernet-transport.hpp"
-#include "pcap-helper.hpp"
 
 #include <boost/range/adaptors.hpp>
 #include <boost/range/algorithm/copy.hpp>
@@ -170,13 +169,13 @@ EthernetFactory::doCreateFace(const CreateFaceRequest& req,
                               const FaceCreationFailedCallback& onFailure)
 {
   if (!req.localUri || req.localUri->getScheme() != "dev") {
-    NFD_LOG_TRACE("Cannot create unicast Ethernet face without dev:// LocalUri");
+    NFD_LOG_TRACE("createFace: dev:// LocalUri required");
     onFailure(406, "Creation of unicast Ethernet faces requires a LocalUri with dev:// scheme");
     return;
   }
 
   if (req.params.persistency == ndn::nfd::FACE_PERSISTENCY_ON_DEMAND) {
-    NFD_LOG_TRACE("createFace does not support FACE_PERSISTENCY_ON_DEMAND");
+    NFD_LOG_TRACE("createFace: unsupported FacePersistency");
     onFailure(406, "Outgoing Ethernet faces do not support on-demand persistency");
     return;
   }
@@ -185,14 +184,14 @@ EthernetFactory::doCreateFace(const CreateFaceRequest& req,
   std::string localEndpoint(req.localUri->getHost());
 
   if (remoteEndpoint.isMulticast()) {
-    NFD_LOG_TRACE("createFace does not support multicast faces");
+    NFD_LOG_TRACE("createFace: unsupported multicast endpoint");
     onFailure(406, "Cannot create multicast Ethernet faces");
     return;
   }
 
   if (req.params.wantLocalFields) {
     // Ethernet faces are never local
-    NFD_LOG_TRACE("createFace cannot create non-local face with local fields enabled");
+    NFD_LOG_TRACE("createFace: cannot create non-local face with local fields enabled");
     onFailure(406, "Local fields can only be enabled on faces with local scope");
     return;
   }
@@ -258,7 +257,9 @@ EthernetFactory::createMulticastFace(const ndn::net::NetworkInterface& netif,
   connectFaceClosedSignal(*face, [this, key] { m_mcastFaces.erase(key); });
 
   auto channelIt = m_channels.find(netif.getName());
-  face->setChannel(channelIt != m_channels.end() ? channelIt->second : nullptr);
+  if (channelIt != m_channels.end()) {
+    face->setChannel(channelIt->second);
+  }
 
   return face;
 }
@@ -332,15 +333,11 @@ EthernetFactory::applyMcastConfigToNetif(const ndn::net::NetworkInterface& netif
   NFD_LOG_DEBUG("Creating multicast face on " << netif.getName());
   shared_ptr<Face> face;
   try {
-    face = this->createMulticastFace(netif, m_mcastConfig.group);
+    face = createMulticastFace(netif, m_mcastConfig.group);
   }
-  catch (const EthernetTransport::Error& e) {
+  catch (const std::runtime_error& e) {
     NFD_LOG_WARN("Cannot create multicast face on " << netif.getName() << ": " << e.what());
-    return nullptr;
-  }
-  catch (const PcapHelper::Error& e) {
-    NFD_LOG_WARN("Cannot create multicast face on " << netif.getName() << ": " << e.what());
-    return nullptr;
+    return nullptr; // not a fatal error
   }
 
   if (face->getId() == INVALID_FACEID) {

daemon/face/ethernet-factory.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2022,  Regents of the University of California,
+ * Copyright (c) 2014-2023,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -28,10 +28,12 @@
 
 #include "protocol-factory.hpp"
 #include "ethernet-channel.hpp"
+#include "network-predicate.hpp"
 
 namespace nfd::face {
 
-/** \brief Protocol factory for Ethernet.
+/**
+ * \brief Protocol factory for Ethernet.
  */
 class EthernetFactory final : public ProtocolFactory
 {
@@ -48,7 +50,7 @@ class EthernetFactory final : public ProtocolFactory
    * If this method is called twice with the same endpoint, only one channel
    * will be created. The second call will just return the existing channel.
    *
-   * \return always a valid pointer to a EthernetChannel object, an exception
+   * \return Always a valid pointer to a EthernetChannel object, an exception
    *         is thrown if it cannot be created.
    * \throw PcapHelper::Error channel creation failed
    */
@@ -65,7 +67,7 @@ class EthernetFactory final : public ProtocolFactory
    * \param localEndpoint local network interface
    * \param group multicast group address
    *
-   * \throw EthernetTransport::Error transport creation fails
+   * \throw std::runtime_error %Face creation failed
    */
   shared_ptr<Face>
   createMulticastFace(const ndn::net::NetworkInterface& localEndpoint,
@@ -84,14 +86,16 @@ class EthernetFactory final : public ProtocolFactory
   std::vector<shared_ptr<const Channel>>
   doGetChannels() const final;
 
-  /** \brief Create EthernetChannel on \p netif if requested by \p m_unicastConfig.
-   *  \return new or existing channel, or nullptr if no channel should be created
+  /**
+   * \brief Create EthernetChannel on \p netif if requested by \p m_unicastConfig.
+   * \return New or existing channel, or nullptr if no channel should be created.
    */
   shared_ptr<EthernetChannel>
   applyUnicastConfigToNetif(const shared_ptr<const ndn::net::NetworkInterface>& netif);
 
-  /** \brief Create Ethernet multicast face on \p netif if requested by \p m_mcastConfig.
-   *  \return new or existing face, or nullptr if no face should be created
+  /**
+   * \brief Create Ethernet multicast face on \p netif if requested by \p m_mcastConfig.
+   * \return New or existing face, or nullptr if no face should be created.
    */
   shared_ptr<Face>
   applyMcastConfigToNetif(const ndn::net::NetworkInterface& netif);
@@ -100,7 +104,8 @@ class EthernetFactory final : public ProtocolFactory
   applyConfig(const FaceSystem::ConfigContext& context);
 
 private:
-  std::map<std::string, shared_ptr<EthernetChannel>> m_channels; ///< ifname => channel
+  // ifname => channel
+  std::map<std::string, shared_ptr<EthernetChannel>> m_channels;
 
   struct UnicastConfig
   {

daemon/face/face-system.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2022,  Regents of the University of California,
+ * Copyright (c) 2014-2023,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -24,9 +24,8 @@
  */
 
 #include "face-system.hpp"
-#include "protocol-factory.hpp"
 #include "netdev-bound.hpp"
-#include "common/global.hpp"
+#include "protocol-factory.hpp"
 #include "fw/face-table.hpp"
 
 namespace nfd::face {
@@ -71,14 +70,14 @@ FaceSystem::listProtocolFactories() const
 }
 
 ProtocolFactory*
-FaceSystem::getFactoryById(const std::string& id)
+FaceSystem::getFactoryById(const std::string& id) const
 {
   auto found = m_factories.find(id);
   return found == m_factories.end() ? nullptr : found->second.get();
 }
 
 ProtocolFactory*
-FaceSystem::getFactoryByScheme(const std::string& scheme)
+FaceSystem::getFactoryByScheme(const std::string& scheme) const
 {
   auto found = m_factoryByScheme.find(scheme);
   return found == m_factoryByScheme.end() ? nullptr : found->second;

daemon/face/face-system.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2022,  Regents of the University of California,
+ * Copyright (c) 2014-2023,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -26,7 +26,6 @@
 #ifndef NFD_DAEMON_FACE_FACE_SYSTEM_HPP
 #define NFD_DAEMON_FACE_FACE_SYSTEM_HPP
 
-#include "network-predicate.hpp"
 #include "common/config-file.hpp"
 
 #include <ndn-cxx/net/network-address.hpp>
@@ -62,15 +61,17 @@ class FaceSystem : noncopyable
   [[nodiscard]] std::set<const ProtocolFactory*>
   listProtocolFactories() const;
 
-  /** \return ProtocolFactory for the specified registered factory id or nullptr if not found.
+  /**
+   * \return ProtocolFactory for the specified registered factory id or nullptr if not found.
    */
   ProtocolFactory*
-  getFactoryById(const std::string& id);
+  getFactoryById(const std::string& id) const;
 
-  /** \return ProtocolFactory for the specified FaceUri scheme or nullptr if not found.
+  /**
+   * \return ProtocolFactory for the specified FaceUri scheme or nullptr if not found.
    */
   ProtocolFactory*
-  getFactoryByScheme(const std::string& scheme);
+  getFactoryByScheme(const std::string& scheme) const;
 
   bool
   hasFactoryForScheme(const std::string& scheme) const;

daemon/face/netdev-bound.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2022,  Regents of the University of California,
+ * Copyright (c) 2014-2023,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -26,6 +26,7 @@
 #ifndef NFD_DAEMON_FACE_NETDEV_BOUND_HPP
 #define NFD_DAEMON_FACE_NETDEV_BOUND_HPP
 
+#include "network-predicate.hpp"
 #include "protocol-factory.hpp"
 
 namespace nfd::face {

daemon/face/tcp-factory.cpp

@@ -152,13 +152,13 @@ TcpFactory::doCreateFace(const CreateFaceRequest& req,
                          const FaceCreationFailedCallback& onFailure)
 {
   if (req.localUri) {
-    NFD_LOG_TRACE("Cannot create unicast TCP face with LocalUri");
+    NFD_LOG_TRACE("createFace: unsupported LocalUri");
     onFailure(406, "Unicast TCP faces cannot be created with a LocalUri");
     return;
   }
 
   if (req.params.persistency == ndn::nfd::FACE_PERSISTENCY_ON_DEMAND) {
-    NFD_LOG_TRACE("createFace does not support FACE_PERSISTENCY_ON_DEMAND");
+    NFD_LOG_TRACE("createFace: unsupported FacePersistency");
     onFailure(406, "Outgoing TCP faces do not support on-demand persistency");
     return;
   }
@@ -170,13 +170,13 @@ TcpFactory::doCreateFace(const CreateFaceRequest& req,
   BOOST_ASSERT(!endpoint.address().is_multicast());
 
   if (req.params.wantLocalFields && !endpoint.address().is_loopback()) {
-    NFD_LOG_TRACE("createFace cannot create non-local face with local fields enabled");
+    NFD_LOG_TRACE("createFace: cannot create non-local face with local fields enabled");
     onFailure(406, "Local fields can only be enabled on faces with local scope");
     return;
   }
 
   if (req.params.mtu) {
-    NFD_LOG_TRACE("createFace cannot create a TCP face with an overridden MTU");
+    NFD_LOG_TRACE("createFace: cannot create TCP face with overridden MTU");
     onFailure(406, "TCP faces do not support MTU overrides");
     return;
   }

daemon/face/tcp-factory.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2014-2022,  Regents of the University of California,
+ * Copyright (c) 2014-2023,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -27,11 +27,13 @@
 #define NFD_DAEMON_FACE_TCP_FACTORY_HPP
 
 #include "protocol-factory.hpp"
+#include "network-predicate.hpp"
 #include "tcp-channel.hpp"
 
 namespace nfd::face {
 
-/** \brief Protocol factory for TCP over IPv4 and IPv6.
+/**
+ * \brief Protocol factory for TCP over IPv4 and IPv6.
  */
 class TcpFactory final : public ProtocolFactory
 {
@@ -44,12 +46,12 @@ class TcpFactory final : public ProtocolFactory
   /**
    * \brief Create TCP-based channel using tcp::Endpoint.
    *
-   * tcp::Endpoint is really an alias for boost::asio::ip::tcp::endpoint.
+   * tcp::Endpoint is an alias for boost::asio::ip::tcp::endpoint.
    *
    * If this method is called twice with the same endpoint, only one channel
    * will be created. The second call will just return the existing channel.
    *
-   * \return always a valid pointer to a TcpChannel object, an exception
+   * \return Always a valid pointer to a TcpChannel object, an exception
    *         is thrown if it cannot be created.
    */
   shared_ptr<TcpChannel>

daemon/face/udp-channel.cpp

@@ -97,10 +97,9 @@ void
 UdpChannel::waitForNewPeer(const FaceCreatedCallback& onFaceCreated,
                            const FaceCreationFailedCallback& onReceiveFailed)
 {
-  m_socket.async_receive_from(boost::asio::buffer(m_receiveBuffer), m_remoteEndpoint,
-                              [=] (auto&&... args) {
-                                this->handleNewPeer(std::forward<decltype(args)>(args)..., onFaceCreated, onReceiveFailed);
-                              });
+  m_socket.async_receive_from(boost::asio::buffer(m_receiveBuffer), m_remoteEndpoint, [=] (auto&&... args) {
+    handleNewPeer(std::forward<decltype(args)>(args)..., onFaceCreated, onReceiveFailed);
+  });
 }
 
 void