face: move packet encoding to Impl class
Practical limit of packet size is now enforced on encoded
NDNLPv2 packets instead of network-layer packets.
If a packet exceeds size limit, the face throws a
Face::OversizedPacketError exception. Since packet encoding may
be asynchronous, this exception could be thrown by
Face::processEvents instead of Face::expressInterest and
Face::put.
refs #4228
Change-Id: Ib68cf80b3b8967fdd0ba040bd8ee595a0eff740e
diff --git a/src/detail/face-impl.hpp b/src/detail/face-impl.hpp
index 027ae26..09c481a 100644
--- a/src/detail/face-impl.hpp
+++ b/src/detail/face-impl.hpp
@@ -24,6 +24,7 @@
#include "../face.hpp"
#include "container-with-on-empty-signal.hpp"
+#include "lp-field-tag.hpp"
#include "pending-interest.hpp"
#include "registered-prefix.hpp"
#include "../lp/packet.hpp"
@@ -53,15 +54,14 @@
namespace ndn {
-/**
- * @brief implementation detail of Face
+/** @brief implementation detail of Face
*/
class Face::Impl : noncopyable
{
public:
- typedef ContainerWithOnEmptySignal<shared_ptr<PendingInterest>> PendingInterestTable;
- typedef std::list<shared_ptr<InterestFilterRecord>> InterestFilterTable;
- typedef ContainerWithOnEmptySignal<shared_ptr<RegisteredPrefix>> RegisteredPrefixTable;
+ using PendingInterestTable = ContainerWithOnEmptySignal<shared_ptr<PendingInterest>>;
+ using InterestFilterTable = std::list<shared_ptr<InterestFilterRecord>>;
+ using RegisteredPrefixTable = ContainerWithOnEmptySignal<shared_ptr<RegisteredPrefix>>;
explicit
Impl(Face& face)
@@ -94,22 +94,12 @@
interest, afterSatisfied, afterNacked, afterTimeout, ref(m_scheduler))).first;
(*entry)->setDeleter([this, entry] { m_pendingInterestTable.erase(entry); });
- lp::Packet packet;
+ lp::Packet lpPacket;
+ addFieldFromTag<lp::NextHopFaceIdField, lp::NextHopFaceIdTag>(lpPacket, *interest);
+ addFieldFromTag<lp::CongestionMarkField, lp::CongestionMarkTag>(lpPacket, *interest);
- shared_ptr<lp::NextHopFaceIdTag> nextHopFaceIdTag = interest->getTag<lp::NextHopFaceIdTag>();
- if (nextHopFaceIdTag != nullptr) {
- packet.add<lp::NextHopFaceIdField>(*nextHopFaceIdTag);
- }
-
- shared_ptr<lp::CongestionMarkTag> congestionMarkTag = interest->getTag<lp::CongestionMarkTag>();
- if (congestionMarkTag != nullptr) {
- packet.add<lp::CongestionMarkField>(*congestionMarkTag);
- }
-
- packet.add<lp::FragmentField>(std::make_pair(interest->wireEncode().begin(),
- interest->wireEncode().end()));
-
- m_face.m_transport->send(packet.wireEncode());
+ m_face.m_transport->send(finishEncoding(std::move(lpPacket), interest->wireEncode(),
+ 'I', interest->getName()));
}
void
@@ -189,10 +179,30 @@
}
void
- asyncSend(const Block& wire)
+ asyncPutData(const Data& data)
{
this->ensureConnected(true);
- m_face.m_transport->send(wire);
+
+ lp::Packet lpPacket;
+ addFieldFromTag<lp::CachePolicyField, lp::CachePolicyTag>(lpPacket, data);
+ addFieldFromTag<lp::CongestionMarkField, lp::CongestionMarkTag>(lpPacket, data);
+
+ m_face.m_transport->send(finishEncoding(std::move(lpPacket), data.wireEncode(),
+ 'D', data.getName()));
+ }
+
+ void
+ asyncPutNack(const lp::Nack& nack)
+ {
+ this->ensureConnected(true);
+
+ lp::Packet lpPacket;
+ lpPacket.add<lp::NackField>(nack.getHeader());
+ addFieldFromTag<lp::CongestionMarkField, lp::CongestionMarkTag>(lpPacket, nack);
+
+ const Interest& interest = nack.getInterest();
+ m_face.m_transport->send(finishEncoding(std::move(lpPacket), interest.wireEncode(),
+ 'N', interest.getName()));
}
public: // prefix registration
@@ -315,6 +325,30 @@
}
private:
+ /** @brief Finish packet encoding
+ * @param lpPacket NDNLP packet without FragmentField
+ * @param wire wire encoding of Interest or Data
+ * @param pktType packet type, 'I' for Interest, 'D' for Data, 'N' for Nack
+ * @param name packet name
+ * @return wire encoding of either NDNLP or bare network packet
+ * @throw Face::OversizedPacketError wire encoding exceeds limit
+ */
+ Block
+ finishEncoding(lp::Packet&& lpPacket, Block wire, char pktType, const Name& name)
+ {
+ if (!lpPacket.empty()) {
+ lpPacket.add<lp::FragmentField>(std::make_pair(wire.begin(), wire.end()));
+ wire = lpPacket.wireEncode();
+ }
+
+ if (wire.size() > MAX_NDN_PACKET_SIZE) {
+ BOOST_THROW_EXCEPTION(Face::OversizedPacketError(pktType, name, wire.size()));
+ }
+
+ return wire;
+ }
+
+private:
Face& m_face;
util::Scheduler m_scheduler;
util::scheduler::ScopedEventId m_processEventsTimeoutEvent;
diff --git a/src/detail/lp-field-tag.hpp b/src/detail/lp-field-tag.hpp
new file mode 100644
index 0000000..f8b2aa5
--- /dev/null
+++ b/src/detail/lp-field-tag.hpp
@@ -0,0 +1,42 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/*
+ * Copyright (c) 2013-2017 Regents of the University of California.
+ *
+ * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
+ *
+ * ndn-cxx library is free software: you can redistribute it and/or modify it under the
+ * terms of the GNU Lesser General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later version.
+ *
+ * ndn-cxx library is distributed in the hope that it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ * PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details.
+ *
+ * You should have received copies of the GNU General Public License and GNU Lesser
+ * General Public License along with ndn-cxx, e.g., in COPYING.md file. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * See AUTHORS.md for complete list of ndn-cxx authors and contributors.
+ */
+
+#ifndef NDN_DETAIL_LP_FIELD_TAG_HPP
+#define NDN_DETAIL_LP_FIELD_TAG_HPP
+
+#include "../lp/packet.hpp"
+#include "../lp/tags.hpp"
+
+namespace ndn {
+
+template<typename Field, typename Tag, typename Packet>
+void
+addFieldFromTag(lp::Packet& lpPacket, const Packet& packet)
+{
+ shared_ptr<Tag> tag = static_cast<const TagHost&>(packet).getTag<Tag>();
+ if (tag != nullptr) {
+ lpPacket.add<Field>(*tag);
+ }
+}
+
+} // namespace ndn
+
+#endif // NDN_DETAIL_LP_FIELD_TAG_HPP
diff --git a/src/face.cpp b/src/face.cpp
index 1add5db..8e59ce9 100644
--- a/src/face.cpp
+++ b/src/face.cpp
@@ -30,9 +30,9 @@
// NDN_LOG_INIT(ndn.Face) is declared in face-impl.hpp
-// A callback scheduled through io.post and io.dispatch may be invoked after the face
-// is destructed. To prevent this situation, these macros captures Face::m_impl as weak_ptr,
-// and skips callback execution if the face has been destructed.
+// A callback scheduled through io.post and io.dispatch may be invoked after the face is destructed.
+// To prevent this situation, use these macros to capture Face::m_impl as weak_ptr and skip callback
+// execution if the face has been destructed.
#define IO_CAPTURE_WEAK_IMPL(OP) \
{ \
weak_ptr<Impl> implWeak(m_impl); \
@@ -46,53 +46,63 @@
namespace ndn {
+Face::OversizedPacketError::OversizedPacketError(char pktType, const Name& name, size_t wireSize)
+ : Error((pktType == 'I' ? "Interest " : pktType == 'D' ? "Data " : "Nack ") +
+ name.toUri() + " encodes into " + to_string(wireSize) + " octets, "
+ "exceeding the implementation limit of " + to_string(MAX_NDN_PACKET_SIZE) + " octets")
+ , pktType(pktType)
+ , name(name)
+ , wireSize(wireSize)
+{
+}
+
Face::Face(shared_ptr<Transport> transport)
- : m_internalIoService(new boost::asio::io_service())
+ : m_internalIoService(make_unique<boost::asio::io_service>())
, m_ioService(*m_internalIoService)
- , m_internalKeyChain(new KeyChain())
+ , m_internalKeyChain(make_unique<KeyChain>())
, m_impl(make_shared<Impl>(*this))
{
- construct(transport, *m_internalKeyChain);
+ construct(std::move(transport), *m_internalKeyChain);
}
Face::Face(boost::asio::io_service& ioService)
: m_ioService(ioService)
- , m_internalKeyChain(new KeyChain())
+ , m_internalKeyChain(make_unique<KeyChain>())
, m_impl(make_shared<Impl>(*this))
{
construct(nullptr, *m_internalKeyChain);
}
Face::Face(const std::string& host, const std::string& port)
- : m_internalIoService(new boost::asio::io_service())
+ : m_internalIoService(make_unique<boost::asio::io_service>())
, m_ioService(*m_internalIoService)
- , m_internalKeyChain(new KeyChain())
+ , m_internalKeyChain(make_unique<KeyChain>())
, m_impl(make_shared<Impl>(*this))
{
construct(make_shared<TcpTransport>(host, port), *m_internalKeyChain);
}
Face::Face(shared_ptr<Transport> transport, KeyChain& keyChain)
- : m_internalIoService(new boost::asio::io_service())
+ : m_internalIoService(make_unique<boost::asio::io_service>())
, m_ioService(*m_internalIoService)
, m_impl(make_shared<Impl>(*this))
{
- construct(transport, keyChain);
+ construct(std::move(transport), keyChain);
}
Face::Face(shared_ptr<Transport> transport, boost::asio::io_service& ioService)
: m_ioService(ioService)
- , m_internalKeyChain(new KeyChain())
+ , m_internalKeyChain(make_unique<KeyChain>())
, m_impl(make_shared<Impl>(*this))
{
- construct(transport, *m_internalKeyChain);
+ construct(std::move(transport), *m_internalKeyChain);
}
Face::Face(shared_ptr<Transport> transport, boost::asio::io_service& ioService, KeyChain& keyChain)
: m_ioService(ioService)
, m_impl(make_shared<Impl>(*this))
{
- construct(transport, keyChain);
+ construct(std::move(transport), keyChain);
}
shared_ptr<Transport>
@@ -147,9 +157,9 @@
transport = makeDefaultTransport();
}
BOOST_ASSERT(transport != nullptr);
- m_transport = transport;
+ m_transport = std::move(transport);
- m_nfdController.reset(new nfd::Controller(*this, keyChain));
+ m_nfdController = make_unique<nfd::Controller>(*this, keyChain);
IO_CAPTURE_WEAK_IMPL(post) {
impl->ensureConnected(false);
@@ -170,20 +180,15 @@
const NackCallback& afterNacked,
const TimeoutCallback& afterTimeout)
{
- shared_ptr<Interest> interestToExpress = make_shared<Interest>(interest);
+ shared_ptr<Interest> interest2 = make_shared<Interest>(interest);
+ interest2->getNonce();
+ NDN_LOG_DEBUG("<I " << *interest2);
- // Use `interestToExpress` to avoid wire format creation for the original Interest
- if (interestToExpress->wireEncode().size() > MAX_NDN_PACKET_SIZE) {
- BOOST_THROW_EXCEPTION(Error("Interest size exceeds maximum limit"));
- }
- NDN_LOG_DEBUG("<I " << *interestToExpress); // interestToExpress is guaranteed to have nonce
-
- // If the same ioService thread, dispatch directly calls the method
IO_CAPTURE_WEAK_IMPL(dispatch) {
- impl->asyncExpressInterest(interestToExpress, afterSatisfied, afterNacked, afterTimeout);
+ impl->asyncExpressInterest(interest2, afterSatisfied, afterNacked, afterTimeout);
} IO_CAPTURE_WEAK_IMPL_END
- return reinterpret_cast<const PendingInterestId*>(interestToExpress.get());
+ return reinterpret_cast<const PendingInterestId*>(interest2.get());
}
void
@@ -209,60 +214,20 @@
}
void
-Face::put(const Data& data)
+Face::put(Data data)
{
- Block wire = data.wireEncode();
-
- lp::Packet packet;
- bool hasLpFields = false;
-
- shared_ptr<lp::CachePolicyTag> cachePolicyTag = data.getTag<lp::CachePolicyTag>();
- if (cachePolicyTag != nullptr) {
- packet.add<lp::CachePolicyField>(*cachePolicyTag);
- hasLpFields = true;
- }
-
- shared_ptr<lp::CongestionMarkTag> congestionMarkTag = data.getTag<lp::CongestionMarkTag>();
- if (congestionMarkTag != nullptr) {
- packet.add<lp::CongestionMarkField>(*congestionMarkTag);
- hasLpFields = true;
- }
-
- if (hasLpFields) {
- packet.add<lp::FragmentField>(std::make_pair(wire.begin(), wire.end()));
- wire = packet.wireEncode();
- }
-
- if (wire.size() > MAX_NDN_PACKET_SIZE)
- BOOST_THROW_EXCEPTION(Error("Data size exceeds maximum limit"));
-
NDN_LOG_DEBUG("<D " << data.getName());
IO_CAPTURE_WEAK_IMPL(dispatch) {
- impl->asyncSend(wire);
+ impl->asyncPutData(data);
} IO_CAPTURE_WEAK_IMPL_END
}
void
-Face::put(const lp::Nack& nack)
+Face::put(lp::Nack nack)
{
- lp::Packet packet;
- packet.add<lp::NackField>(nack.getHeader());
- const Block& interestWire = nack.getInterest().wireEncode();
- packet.add<lp::FragmentField>(std::make_pair(interestWire.begin(), interestWire.end()));
-
- shared_ptr<lp::CongestionMarkTag> congestionMarkTag = nack.getTag<lp::CongestionMarkTag>();
- if (congestionMarkTag != nullptr) {
- packet.add<lp::CongestionMarkField>(*congestionMarkTag);
- }
-
- Block wire = packet.wireEncode();
-
- if (wire.size() > MAX_NDN_PACKET_SIZE)
- BOOST_THROW_EXCEPTION(Error("Nack size exceeds maximum limit"));
-
NDN_LOG_DEBUG("<N " << nack.getInterest() << '~' << nack.getHeader().getReason());
IO_CAPTURE_WEAK_IMPL(dispatch) {
- impl->asyncSend(wire);
+ impl->asyncPutNack(nack);
} IO_CAPTURE_WEAK_IMPL_END
}
diff --git a/src/face.hpp b/src/face.hpp
index b375e93..236b296 100644
--- a/src/face.hpp
+++ b/src/face.hpp
@@ -22,16 +22,14 @@
#ifndef NDN_FACE_HPP
#define NDN_FACE_HPP
-#include "common.hpp"
-
+#include "data.hpp"
#include "name.hpp"
#include "interest.hpp"
#include "interest-filter.hpp"
-#include "data.hpp"
#include "encoding/nfd-constants.hpp"
#include "lp/nack.hpp"
-#include "security/signing-info.hpp"
#include "security/key-chain.hpp"
+#include "security/signing-info.hpp"
namespace boost {
namespace asio {
@@ -107,6 +105,26 @@
}
};
+ /**
+ * @brief Exception thrown when attempting to send a packet over size limit
+ */
+ class OversizedPacketError : public Error
+ {
+ public:
+ /**
+ * @brief Constructor
+ * @param pktType packet type, 'I' for Interest, 'D' for Data, 'N' for Nack
+ * @param name packet name
+ * @param wireSize wire encoding size
+ */
+ OversizedPacketError(char pktType, const Name& name, size_t wireSize);
+
+ public:
+ const char pktType;
+ const Name name;
+ const size_t wireSize;
+ };
+
public: // constructors
/**
* @brief Create Face using given transport (or default transport if omitted)
@@ -115,7 +133,7 @@
* determined from a FaceUri in NDN_CLIENT_TRANSPORT environ,
* a FaceUri in configuration file 'transport' key, or UnixTransport.
*
- * @throw ConfigFile::Error if @p transport is nullptr, and the configuration file cannot be
+ * @throw ConfigFile::Error @p transport is nullptr, and the configuration file cannot be
* parsed or specifies an unsupported protocol
* @note shared_ptr is passed by value because ownership is shared with this class
*/
@@ -170,7 +188,7 @@
*
* @sa Face(shared_ptr<Transport>)
*
- * @throw ConfigFile::Error if @p transport is nullptr, and the configuration file cannot be
+ * @throw ConfigFile::Error @p transport is nullptr, and the configuration file cannot be
* parsed or specifies an unsupported protocol
* @note shared_ptr is passed by value because ownership is shared with this class
*/
@@ -185,7 +203,7 @@
* @sa Face(boost::asio::io_service&)
* @sa Face(shared_ptr<Transport>)
*
- * @throw ConfigFile::Error if @p transport is nullptr, and the configuration file cannot be
+ * @throw ConfigFile::Error @p transport is nullptr, and the configuration file cannot be
* parsed or specifies an unsupported protocol
* @note shared_ptr is passed by value because ownership is shared with this class
*/
@@ -201,7 +219,7 @@
* @sa Face(boost::asio::io_service&)
* @sa Face(shared_ptr<Transport>, KeyChain&)
*
- * @throw ConfigFile::Error if @p transport is nullptr, and the configuration file cannot be
+ * @throw ConfigFile::Error @p transport is nullptr, and the configuration file cannot be
* parsed or specifies an unsupported protocol
* @note shared_ptr is passed by value because ownership is shared with this class
*/
@@ -219,6 +237,7 @@
* @param afterNacked function to be invoked if Network NACK is returned
* @param afterTimeout function to be invoked if neither Data nor Network NACK
* is returned within InterestLifetime
+ * @throw OversizedPacketError encoded Interest size exceeds MAX_NDN_PACKET_SIZE
*/
const PendingInterestId*
expressInterest(const Interest& interest,
@@ -387,29 +406,27 @@
const UnregisterPrefixSuccessCallback& onSuccess,
const UnregisterPrefixFailureCallback& onFailure);
- /**
+ /**
* @brief Publish data packet
+ * @param data the Data; a copy will be made, so that the caller is not required to
+ * maintain the argument unchanged
*
- * This method can be called to satisfy the incoming Interest or to put Data packet into
- * the cache of the local NDN forwarder.
+ * This method can be called to satisfy incoming Interests, or to add Data packet into the cache
+ * of the local NDN forwarder if forwarder is configured to accept unsolicited Data.
*
- * @param data Data packet to publish. It is highly recommended to use Data packet that
- * was created using make_shared<Data>(...). Otherwise, put() will make an
- * extra copy of the Data packet to ensure validity of published Data until
- * asynchronous put() operation finishes.
- *
- * @throw Error when Data size exceeds maximum limit (MAX_NDN_PACKET_SIZE)
+ * @throw OversizedPacketError encoded Data size exceeds MAX_NDN_PACKET_SIZE
*/
void
- put(const Data& data);
+ put(Data data);
/**
- * @brief sends a Network NACK
+ * @brief Send a network NACK
* @param nack the Nack; a copy will be made, so that the caller is not required to
* maintain the argument unchanged
+ * @throw OversizedPacketError encoded Nack size exceeds MAX_NDN_PACKET_SIZE
*/
void
- put(const lp::Nack& nack);
+ put(lp::Nack nack);
public: // IO routine
/**
@@ -429,9 +446,11 @@
* @param keepThread Keep thread in a blocked state (in event processing), even when
* there are no outstanding events (e.g., no Interest/Data is expected)
*
- * @throw This may throw an exception for reading data or in the callback for processing
+ * @note This may throw an exception for reading data or in the callback for processing
* the data. If you call this from an main event loop, you may want to catch and
* log/disregard all exceptions.
+ *
+ * @throw OversizedPacketError encoded packet size exceeds MAX_NDN_PACKET_SIZE
*/
void
processEvents(time::milliseconds timeout = time::milliseconds::zero(),
diff --git a/src/lp/packet.hpp b/src/lp/packet.hpp
index b2e003e..eb30485 100644
--- a/src/lp/packet.hpp
+++ b/src/lp/packet.hpp
@@ -1,5 +1,5 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
* Copyright (c) 2013-2017 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -58,6 +58,16 @@
void
wireDecode(const Block& wire);
+ /**
+ * \retval true packet has no field
+ * \retval false packet has one or more fields
+ */
+ bool
+ empty() const
+ {
+ return m_wire.elements_size() == 0;
+ }
+
public: // field access
/**
* \return true if FIELD occurs one or more times
diff --git a/src/util/dummy-client-face.cpp b/src/util/dummy-client-face.cpp
index 24b05a3..57a43b7 100644
--- a/src/util/dummy-client-face.cpp
+++ b/src/util/dummy-client-face.cpp
@@ -20,6 +20,7 @@
*/
#include "dummy-client-face.hpp"
+#include "../detail/lp-field-tag.hpp"
#include "../lp/packet.hpp"
#include "../lp/tags.hpp"
#include "../mgmt/nfd/controller.hpp"
@@ -217,16 +218,6 @@
});
}
-template<typename Field, typename Tag, typename Packet>
-static void
-addFieldFromTag(lp::Packet& lpPacket, const Packet& packet)
-{
- shared_ptr<Tag> tag = static_cast<const TagHost&>(packet).getTag<Tag>();
- if (tag != nullptr) {
- lpPacket.add<Field>(*tag);
- }
-}
-
void
DummyClientFace::receive(const Interest& interest)
{