face: allow GenericLinkService to override MTU
refs #5056
Change-Id: I8eb2dd732b1431e0e809deb53792a14be1698ff1
diff --git a/daemon/face/ethernet-channel.cpp b/daemon/face/ethernet-channel.cpp
index 0104b19..725bbfd 100644
--- a/daemon/face/ethernet-channel.cpp
+++ b/daemon/face/ethernet-channel.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -201,11 +201,13 @@
options.allowFragmentation = true;
options.allowReassembly = true;
options.reliabilityOptions.isEnabled = params.wantLpReliability;
+ if (params.mtu) {
+ options.overrideMtu = *params.mtu;
+ }
auto linkService = make_unique<GenericLinkService>(options);
auto transport = make_unique<UnicastEthernetTransport>(*m_localEndpoint, remoteEndpoint,
- params.persistency, m_idleFaceTimeout,
- params.mtu);
+ params.persistency, m_idleFaceTimeout);
auto face = make_shared<Face>(std::move(linkService), std::move(transport));
m_channelFaces[remoteEndpoint] = face;
diff --git a/daemon/face/ethernet-factory.cpp b/daemon/face/ethernet-factory.cpp
index 4d4d295..9659985 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-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -197,10 +197,10 @@
return;
}
- if (req.params.mtu && *req.params.mtu < Transport::MIN_MTU) {
+ if (req.params.mtu && *req.params.mtu < MIN_MTU) {
// The specified MTU must be greater than the minimum possible
- NFD_LOG_TRACE("createFace cannot create a face with an MTU less than " << Transport::MIN_MTU);
- onFailure(406, "MTU cannot be less than " + to_string(Transport::MIN_MTU));
+ NFD_LOG_TRACE("createFace: override MTU cannot be less than " << MIN_MTU);
+ onFailure(406, "Override MTU cannot be less than " + to_string(MIN_MTU));
return;
}
diff --git a/daemon/face/face-common.hpp b/daemon/face/face-common.hpp
index f233309..e2a0572 100644
--- a/daemon/face/face-common.hpp
+++ b/daemon/face/face-common.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -54,6 +54,12 @@
/// upper bound of reserved FaceIds
const FaceId FACEID_RESERVED_MAX = 255;
+/** \brief Minimum MTU that may be set
+ *
+ * This is done to ensure the NDNLPv2 fragmentation feature functions properly.
+ */
+const ssize_t MIN_MTU = 64;
+
/** \brief Identifies a remote endpoint on the link.
*
* This ID is only meaningful in the context of the same Transport.
diff --git a/daemon/face/face.hpp b/daemon/face/face.hpp
index dc8c20d..4a5948b 100644
--- a/daemon/face/face.hpp
+++ b/daemon/face/face.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -60,6 +60,18 @@
Transport*
getTransport() const;
+ /** \brief Request that the face be closed
+ *
+ * This operation is effective only if face is in the UP or DOWN state; otherwise, it has no effect.
+ * The face will change state to CLOSING, and then perform a cleanup procedure.
+ * When the cleanup is complete, the state will be changed to CLOSED, which may happen
+ * synchronously or asynchronously.
+ *
+ * \warning The face must not be deallocated until its state changes to CLOSED.
+ */
+ void
+ close();
+
public: // upper interface connected to forwarding
/** \brief send Interest to \p endpointId
*/
@@ -92,7 +104,7 @@
*/
signal::Signal<LinkService, Interest>& onDroppedInterest;
-public: // static properties
+public: // properties
/** \return face ID
*/
FaceId
@@ -134,7 +146,13 @@
ndn::nfd::LinkType
getLinkType() const;
-public: // dynamic properties
+ /** \brief Returns face effective MTU
+ *
+ * This function is a wrapper. The effective MTU of a face is determined by the link service.
+ */
+ ssize_t
+ getMtu() const;
+
/** \return face state
*/
FaceState
@@ -150,19 +168,6 @@
time::steady_clock::TimePoint
getExpirationTime() const;
- /** \brief request the face to be closed
- *
- * This operation is effective only if face is in UP or DOWN state,
- * otherwise it has no effect.
- * The face changes state to CLOSING, and performs cleanup procedure.
- * The state will be changed to CLOSED when cleanup is complete, which may
- * happen synchronously or asynchronously.
- *
- * \warning the face must not be deallocated until its state changes to CLOSED
- */
- void
- close();
-
const FaceCounters&
getCounters() const;
@@ -186,6 +191,12 @@
}
inline void
+Face::close()
+{
+ m_transport->close();
+}
+
+inline void
Face::sendInterest(const Interest& interest, const EndpointId& endpointId)
{
m_service->sendInterest(interest, endpointId);
@@ -251,6 +262,12 @@
return m_transport->getLinkType();
}
+inline ssize_t
+Face::getMtu() const
+{
+ return m_service->getEffectiveMtu();
+}
+
inline FaceState
Face::getState() const
{
@@ -263,12 +280,6 @@
return m_transport->getExpirationTime();
}
-inline void
-Face::close()
-{
- m_transport->close();
-}
-
inline const FaceCounters&
Face::getCounters() const
{
diff --git a/daemon/face/generic-link-service.cpp b/daemon/face/generic-link-service.cpp
index 7e32fc7..af03ffd 100644
--- a/daemon/face/generic-link-service.cpp
+++ b/daemon/face/generic-link-service.cpp
@@ -62,6 +62,25 @@
m_reliability.setOptions(m_options.reliabilityOptions);
}
+ssize_t
+GenericLinkService::getEffectiveMtu() const
+{
+ // Since MTU_UNLIMITED is negative, it will implicitly override any finite override MTU
+ return std::min(m_options.overrideMtu, getTransport()->getMtu());
+}
+
+bool
+GenericLinkService::canOverrideMtuTo(ssize_t mtu) const
+{
+ // Not allowed to override unlimited transport MTU
+ if (getTransport()->getMtu() == MTU_UNLIMITED) {
+ return false;
+ }
+
+ // Override MTU must be at least MIN_MTU (also implicitly forbids MTU_UNLIMITED and MTU_INVALID)
+ return mtu >= MIN_MTU;
+}
+
void
GenericLinkService::requestIdlePacket(const EndpointId& endpointId)
{
@@ -74,7 +93,7 @@
void
GenericLinkService::sendLpPacket(lp::Packet&& pkt, const EndpointId& endpointId)
{
- const ssize_t mtu = this->getTransport()->getMtu();
+ const ssize_t mtu = getEffectiveMtu();
if (m_options.reliabilityOptions.isEnabled) {
m_reliability.piggyback(pkt, mtu);
@@ -169,7 +188,7 @@
GenericLinkService::sendNetPacket(lp::Packet&& pkt, const EndpointId& endpointId, bool isInterest)
{
std::vector<lp::Packet> frags;
- ssize_t mtu = this->getTransport()->getMtu();
+ ssize_t mtu = getEffectiveMtu();
// Make space for feature fields in fragments
if (m_options.reliabilityOptions.isEnabled && mtu != MTU_UNLIMITED) {
diff --git a/daemon/face/generic-link-service.hpp b/daemon/face/generic-link-service.hpp
index e30a3e6..38473f5 100644
--- a/daemon/face/generic-link-service.hpp
+++ b/daemon/face/generic-link-service.hpp
@@ -102,7 +102,6 @@
class Options
{
public:
- constexpr
Options() noexcept
{
}
@@ -155,6 +154,16 @@
/** \brief enables self-learning forwarding support
*/
bool allowSelfLearning = true;
+
+ /** \brief overrides MTU provided by Transport
+ *
+ * This MTU value will be used instead of the MTU provided by the transport if it is less than
+ * the transport MTU. However, it will not be utilized when the transport MTU is unlimited.
+ *
+ * Acceptable values for the override MTU are values >= MIN_MTU, which can be validated before
+ * being set with canOverrideMtuTo().
+ */
+ ssize_t overrideMtu = std::numeric_limits<ssize_t>::max();
};
/** \brief counters provided by GenericLinkService
@@ -177,6 +186,16 @@
const Counters&
getCounters() const OVERRIDE_WITH_TESTS_ELSE_FINAL;
+ ssize_t
+ getEffectiveMtu() const OVERRIDE_WITH_TESTS_ELSE_FINAL;
+
+ /** \brief Whether MTU can be overridden to the specified value
+ *
+ * If the transport MTU is unlimited, then this will always return false.
+ */
+ bool
+ canOverrideMtuTo(ssize_t mtu) const;
+
PROTECTED_WITH_TESTS_ELSE_PRIVATE: // send path
/** \brief request an IDLE packet to transmit pending service fields
*/
diff --git a/daemon/face/link-service.hpp b/daemon/face/link-service.hpp
index 415c185..fd35723 100644
--- a/daemon/face/link-service.hpp
+++ b/daemon/face/link-service.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -109,6 +109,9 @@
virtual const Counters&
getCounters() const;
+ virtual ssize_t
+ getEffectiveMtu() const;
+
public: // upper interface to be used by forwarding
/** \brief Send Interest to \p endpoint
* \pre setTransport has been called
@@ -225,6 +228,12 @@
return *this;
}
+inline ssize_t
+LinkService::getEffectiveMtu() const
+{
+ return m_transport->getMtu();
+}
+
inline void
LinkService::receivePacket(const Block& packet, const EndpointId& endpoint)
{
diff --git a/daemon/face/transport.cpp b/daemon/face/transport.cpp
index 18d8608..6b235bc 100644
--- a/daemon/face/transport.cpp
+++ b/daemon/face/transport.cpp
@@ -31,8 +31,6 @@
NFD_LOG_INIT(Transport);
-const ssize_t Transport::MIN_MTU;
-
std::ostream&
operator<<(std::ostream& os, TransportState state)
{
diff --git a/daemon/face/transport.hpp b/daemon/face/transport.hpp
index 211fefc..d3840bc 100644
--- a/daemon/face/transport.hpp
+++ b/daemon/face/transport.hpp
@@ -339,13 +339,6 @@
virtual void
doSend(const Block& packet, const EndpointId& endpoint) = 0;
-public:
- /** \brief minimum MTU that may be set on a transport
- *
- * This is done to ensure the NDNLPv2 fragmentation feature functions properly.
- */
- static constexpr ssize_t MIN_MTU = 64;
-
private:
Face* m_face;
LinkService* m_service;
diff --git a/daemon/face/udp-channel.cpp b/daemon/face/udp-channel.cpp
index 5a23758..4776661 100644
--- a/daemon/face/udp-channel.cpp
+++ b/daemon/face/udp-channel.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -180,9 +180,13 @@
options.defaultCongestionThreshold = *params.defaultCongestionThreshold;
}
+ if (params.mtu) {
+ options.overrideMtu = *params.mtu;
+ }
+
auto linkService = make_unique<GenericLinkService>(options);
auto transport = make_unique<UnicastUdpTransport>(std::move(socket), params.persistency,
- m_idleFaceTimeout, params.mtu);
+ m_idleFaceTimeout);
auto face = make_shared<Face>(std::move(linkService), std::move(transport));
m_channelFaces[remoteEndpoint] = face;
diff --git a/daemon/face/udp-factory.cpp b/daemon/face/udp-factory.cpp
index 54b00b1..b15b762 100644
--- a/daemon/face/udp-factory.cpp
+++ b/daemon/face/udp-factory.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -268,10 +268,10 @@
return;
}
- if (req.params.mtu && *req.params.mtu < Transport::MIN_MTU) {
+ if (req.params.mtu && *req.params.mtu < MIN_MTU) {
// The specified MTU must be greater than the minimum possible
- NFD_LOG_TRACE("createFace cannot create a face with MTU less than " << Transport::MIN_MTU);
- onFailure(406, "MTU cannot be less than " + to_string(Transport::MIN_MTU));
+ NFD_LOG_TRACE("createFace: override MTU cannot be less than " << MIN_MTU);
+ onFailure(406, "Override MTU cannot be less than " + to_string(MIN_MTU));
return;
}
diff --git a/daemon/face/unicast-ethernet-transport.cpp b/daemon/face/unicast-ethernet-transport.cpp
index 97e5c4e..33e80bd 100644
--- a/daemon/face/unicast-ethernet-transport.cpp
+++ b/daemon/face/unicast-ethernet-transport.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -36,8 +36,7 @@
UnicastEthernetTransport::UnicastEthernetTransport(const ndn::net::NetworkInterface& localEndpoint,
const ethernet::Address& remoteEndpoint,
ndn::nfd::FacePersistency persistency,
- time::nanoseconds idleTimeout,
- optional<ssize_t> overrideMtu)
+ time::nanoseconds idleTimeout)
: EthernetTransport(localEndpoint, remoteEndpoint)
, m_idleTimeout(idleTimeout)
{
@@ -46,13 +45,7 @@
this->setScope(ndn::nfd::FACE_SCOPE_NON_LOCAL);
this->setPersistency(persistency);
this->setLinkType(ndn::nfd::LINK_TYPE_POINT_TO_POINT);
-
- if (overrideMtu) {
- this->setMtu(std::min<ssize_t>(localEndpoint.getMtu(), *overrideMtu));
- }
- else {
- this->setMtu(localEndpoint.getMtu());
- }
+ this->setMtu(localEndpoint.getMtu());
NFD_LOG_FACE_DEBUG("Creating transport");
diff --git a/daemon/face/unicast-ethernet-transport.hpp b/daemon/face/unicast-ethernet-transport.hpp
index 63d74e9..49c9d48 100644
--- a/daemon/face/unicast-ethernet-transport.hpp
+++ b/daemon/face/unicast-ethernet-transport.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -43,8 +43,7 @@
UnicastEthernetTransport(const ndn::net::NetworkInterface& localEndpoint,
const ethernet::Address& remoteEndpoint,
ndn::nfd::FacePersistency persistency,
- time::nanoseconds idleTimeout,
- optional<ssize_t> overrideMtu = {});
+ time::nanoseconds idleTimeout);
protected:
bool
diff --git a/daemon/face/unicast-udp-transport.cpp b/daemon/face/unicast-udp-transport.cpp
index a81e1d0..140f691 100644
--- a/daemon/face/unicast-udp-transport.cpp
+++ b/daemon/face/unicast-udp-transport.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -41,8 +41,7 @@
UnicastUdpTransport::UnicastUdpTransport(protocol::socket&& socket,
ndn::nfd::FacePersistency persistency,
- time::nanoseconds idleTimeout,
- optional<ssize_t> overrideMtu)
+ time::nanoseconds idleTimeout)
: DatagramTransport(std::move(socket))
, m_idleTimeout(idleTimeout)
{
@@ -51,14 +50,7 @@
this->setScope(ndn::nfd::FACE_SCOPE_NON_LOCAL);
this->setPersistency(persistency);
this->setLinkType(ndn::nfd::LINK_TYPE_POINT_TO_POINT);
-
- if (overrideMtu) {
- this->setMtu(std::min(udp::computeMtu(m_socket.local_endpoint()), *overrideMtu));
- }
- else {
- this->setMtu(udp::computeMtu(m_socket.local_endpoint()));
- }
- BOOST_ASSERT(this->getMtu() >= MIN_MTU);
+ this->setMtu(udp::computeMtu(m_socket.local_endpoint()));
NFD_LOG_FACE_DEBUG("Creating transport");
diff --git a/daemon/face/unicast-udp-transport.hpp b/daemon/face/unicast-udp-transport.hpp
index fdadfb9..991d561 100644
--- a/daemon/face/unicast-udp-transport.hpp
+++ b/daemon/face/unicast-udp-transport.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -41,8 +41,7 @@
public:
UnicastUdpTransport(protocol::socket&& socket,
ndn::nfd::FacePersistency persistency,
- time::nanoseconds idleTimeout,
- optional<ssize_t> overrideMtu = {});
+ time::nanoseconds idleTimeout);
protected:
bool
diff --git a/daemon/mgmt/face-manager.cpp b/daemon/mgmt/face-manager.cpp
index 9b36410..fd5f3aa 100644
--- a/daemon/mgmt/face-manager.cpp
+++ b/daemon/mgmt/face-manager.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2014-2019, Regents of the University of California,
+ * Copyright (c) 2014-2020, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -117,7 +117,8 @@
faceParams.defaultCongestionThreshold = parameters.getDefaultCongestionThreshold();
}
if (parameters.hasMtu()) {
- faceParams.mtu = parameters.getMtu();
+ // Cap this value at the maximum representable value in an ssize_t
+ faceParams.mtu = std::min<uint64_t>(std::numeric_limits<ssize_t>::max(), parameters.getMtu());
}
faceParams.wantLocalFields = parameters.hasFlagBit(ndn::nfd::BIT_LOCAL_FIELDS_ENABLED) &&
parameters.getFlagBit(ndn::nfd::BIT_LOCAL_FIELDS_ENABLED);
@@ -152,13 +153,10 @@
static void
copyMtu(const Face& face, T& to)
{
- auto transport = face.getTransport();
- BOOST_ASSERT(transport != nullptr);
-
- if (transport->getMtu() > 0) {
- to.setMtu(std::min(static_cast<size_t>(transport->getMtu()), ndn::MAX_NDN_PACKET_SIZE));
+ if (face.getMtu() >= 0) {
+ to.setMtu(std::min<size_t>(face.getMtu(), ndn::MAX_NDN_PACKET_SIZE));
}
- else if (transport->getMtu() == face::MTU_UNLIMITED) {
+ else if (face.getMtu() == face::MTU_UNLIMITED) {
to.setMtu(ndn::MAX_NDN_PACKET_SIZE);
}
}