Project

General

Profile

Actions
Copy link

Bug #5128

closed

Forwarder's HopLimit handling breaks strategy assumptions

Added by Davide Pesavento about 4 years ago. Updated about 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Eric Newberry
Category:
Forwarding
Target version:
22.02
Start date:
Due date:
% Done:

100%

Estimated time:
Tags:
fuzzing

Description

Commit 9d283adac450171c65bb020ebb55df838609b6d7 added HopLimit processing to Forwarder::onOutgoingInterest. With this change, it is no longer guaranteed that every invocation of Strategy::sendInterest will create a PIT out-record for the egress face. Unfortunately, some strategies were relying on the previous behavior that the out-record is always created, so now they may misbehave or crash.

From a quick look I found the following, but there may be others and all strategies must be carefully audited to be sure.

I found this problem while analyzing a crash (stack trace below) found by NFDFuzz, an experimental fuzzer for NFD and ndn-cxx.

==86==ERROR: AddressSanitizer: SEGV on unknown address 0x000000002088 (pc 0x00000093768b bp 0x7f4279a99c00 sp 0x7f4279a99bd0 T4)
==86==The signal is caused by a READ memory access.
    #0 0x93768b in std::_Hashtable<int, std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > >, std::allocator<std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > > >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node(unsigned long, int const&, unsigned long) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/hashtable.h:1538:31
    #1 0x936ea8 in std::_Hashtable<int, std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > >, std::allocator<std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > > >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_node(unsigned long, int const&, unsigned long) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/hashtable.h:655:28
    #2 0x936c6b in std::__detail::_Map_base<int, std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > >, std::allocator<std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > > >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true>, true>::operator[](int&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/hashtable_policy.h:722:31
    #3 0x936783 in std::unordered_map<int, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> >, std::hash<int>, std::equal_to<int>, std::allocator<std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > > > >::operator[](int&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unordered_map.h:989:16
    #4 0x9f8a5f in std::pair<nfd::fw::SelfLearningStrategy::OutRecordInfo*, bool> nfd::StrategyInfoHost::insertStrategyInfo<nfd::fw::SelfLearningStrategy::OutRecordInfo>() /home/gtorresz/nfdfuzzer/NFD/build/../daemon/table/strategy-info-host.hpp:68:18
    #5 0x9ea8ed in nfd::fw::SelfLearningStrategy::broadcastInterest(ndn::Interest const&, nfd::face::Face const&, std::shared_ptr<nfd::pit::Entry> const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fw/self-learning-strategy.cpp:156:38
    #6 0x9e9859 in nfd::fw::SelfLearningStrategy::afterReceiveInterest(nfd::FaceEndpoint const&, ndn::Interest const&, std::shared_ptr<nfd::pit::Entry> const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fw/self-learning-strategy.cpp:93:7
    #7 0x99d6bb in nfd::Forwarder::onContentStoreMiss(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&)::$_5::operator()(nfd::fw::Strategy&) const /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fw/forwarder.cpp:217:16
    #8 0x991f54 in void nfd::Forwarder::dispatchToStrategy<nfd::Forwarder::onContentStoreMiss(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&)::$_5>(nfd::pit::Entry&, nfd::Forwarder::onContentStoreMiss(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&)::$_5) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fw/forwarder.hpp:258:5
    #9 0x990b3e in nfd::Forwarder::onContentStoreMiss(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fw/forwarder.cpp:215:9
    #10 0x9b5c9b in void std::__invoke_impl<void, void (nfd::Forwarder::*&)(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&), nfd::Forwarder*&, nfd::FaceEndpoint&, std::shared_ptr<nfd::pit::Entry>&, ndn::Interest const&>(std::__invoke_memfun_deref, void (nfd::Forwarder::*&)(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&), nfd::Forwarder*&, nfd::FaceEndpoint&, std::shared_ptr<nfd::pit::Entry>&, ndn::Interest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/invoke.h:73:14
    #11 0x9b59f8 in std::__invoke_result<void (nfd::Forwarder::*&)(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&), nfd::Forwarder*&, nfd::FaceEndpoint&, std::shared_ptr<nfd::pit::Entry>&, ndn::Interest const&>::type std::__invoke<void (nfd::Forwarder::*&)(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&), nfd::Forwarder*&, nfd::FaceEndpoint&, std::shared_ptr<nfd::pit::Entry>&, ndn::Interest const&>(void (nfd::Forwarder::*&)(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&), nfd::Forwarder*&, nfd::FaceEndpoint&, std::shared_ptr<nfd::pit::Entry>&, ndn::Interest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/invoke.h:95:14
    #12 0x9b5897 in void std::_Bind<void (nfd::Forwarder::* (nfd::Forwarder*, nfd::FaceEndpoint, std::shared_ptr<nfd::pit::Entry>, std::_Placeholder<1>))(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&)>::__call<void, ndn::Interest const&, 0ul, 1ul, 2ul, 3ul>(std::tuple<ndn::Interest const&>&&, std::_Index_tuple<0ul, 1ul, 2ul, 3ul>) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/functional:400:11
    #13 0x9b53c3 in void std::_Bind<void (nfd::Forwarder::* (nfd::Forwarder*, nfd::FaceEndpoint, std::shared_ptr<nfd::pit::Entry>, std::_Placeholder<1>))(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&)>::operator()<ndn::Interest const&, void>(ndn::Interest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/functional:482:17
    #14 0x9a4a7a in void nfd::cs::Cs::find<std::_Bind<void (nfd::Forwarder::* (nfd::Forwarder*, nfd::FaceEndpoint, std::shared_ptr<nfd::pit::Entry>, std::_Placeholder<1>, std::_Placeholder<2>))(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&, ndn::Data const&)>, std::_Bind<void (nfd::Forwarder::* (nfd::Forwarder*, nfd::FaceEndpoint, std::shared_ptr<nfd::pit::Entry>, std::_Placeholder<1>))(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&)> >(ndn::Interest const&, std::_Bind<void (nfd::Forwarder::* (nfd::Forwarder*, nfd::FaceEndpoint, std::shared_ptr<nfd::pit::Entry>, std::_Placeholder<1>, std::_Placeholder<2>))(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&, ndn::Data const&)>&&, std::_Bind<void (nfd::Forwarder::* (nfd::Forwarder*, nfd::FaceEndpoint, std::shared_ptr<nfd::pit::Entry>, std::_Placeholder<1>))(nfd::FaceEndpoint const&, std::shared_ptr<nfd::pit::Entry> const&, ndn::Interest const&>&&) const /home/gtorresz/nfdfuzzer/NFD/build/../daemon/table/cs.hpp:85:7
    #15 0x98e8e2 in nfd::Forwarder::onIncomingInterest(nfd::FaceEndpoint const&, ndn::Interest const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fw/forwarder.cpp:152:10
    #16 0x9ac270 in nfd::Forwarder::startProcessInterest(nfd::FaceEndpoint const&, ndn::Interest const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fw/forwarder.hpp:87:11
    #17 0x99ba43 in nfd::Forwarder::Forwarder(nfd::FaceTable&)::$_2::operator()(nfd::face::Face const&) const::'lambda'(ndn::Interest const&, unsigned long const&)::operator()(ndn::Interest const&, unsigned long const&) const /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fw/forwarder.cpp:59:15
    #18 0x99b740 in std::_Function_handler<void (ndn::Interest const&, unsigned long const&), nfd::Forwarder::Forwarder(nfd::FaceTable&)::$_2::operator()(nfd::face::Face const&) const::'lambda'(ndn::Interest const&, unsigned long const&)>::_M_invoke(std::_Any_data const&, ndn::Interest const&, unsigned long const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2
    #19 0x789653 in std::function<void (ndn::Interest const&, unsigned long const&)>::operator()(ndn::Interest const&, unsigned long const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
    #20 0x786aaf in ndn::util::signal::Signal<nfd::face::LinkService, ndn::Interest, unsigned long>::operator()(ndn::Interest const&, unsigned long const&) /usr/local/include/ndn-cxx/util/signal/signal.hpp:232:7
    #21 0x782c01 in nfd::face::LinkService::receiveInterest(ndn::Interest const&, unsigned long const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/face/link-service.cpp:94:3
    #22 0x72c859 in nfd::face::GenericLinkService::decodeInterest(ndn::Block const&, ndn::lp::Packet const&, unsigned long const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/face/generic-link-service.cpp:411:9
    #23 0x729159 in nfd::face::GenericLinkService::decodeNetPacket(ndn::Block const&, ndn::lp::Packet const&, unsigned long const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/face/generic-link-service.cpp:340:17
    #24 0x727f9b in nfd::face::GenericLinkService::doReceivePacket(ndn::Block const&, unsigned long const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/face/generic-link-service.cpp:320:13
    #25 0x8b4414 in nfd::face::LinkService::receivePacket(ndn::Block const&, unsigned long const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/face/link-service.hpp:240:3
    #26 0x8af5ea in nfd::face::Transport::receive(ndn::Block const&, unsigned long const&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/face/transport.cpp:122:14
    #27 0x8a31c1 in nfd::face::StreamTransport<boost::asio::ip::tcp>::handleReceive(boost::system::error_code const&, unsigned long) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/face/stream-transport.hpp:258:11
    #28 0x8a2b18 in auto nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(auto&&...)::operator()<boost::system::error_code const&, unsigned long const&>(auto&&...) const /home/gtorresz/nfdfuzzer/NFD/build/../daemon/face/stream-transport.hpp:233:58
    #29 0x8a2aa8 in boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>::operator()() /usr/include/boost/asio/detail/bind_handler.hpp:164:5
    #30 0x8a2a70 in void boost::asio::asio_handler_invoke<boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long> >(boost::asio::ip::tcp&, ...) /usr/include/boost/asio/handler_invoke_hook.hpp:69:3
    #31 0x8a2a45 in void boost_asio_handler_invoke_helpers::invoke<boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>, nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...)>(boost::asio::ip::tcp&, boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>&) /usr/include/boost/asio/detail/handler_invoke_helpers.hpp:37:3
    #32 0x8a29d0 in void boost::asio::detail::asio_handler_invoke<boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>, nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>(boost::asio::ip::tcp&, boost::asio::detail::binder2<boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>, nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code>*) /usr/include/boost/asio/detail/bind_handler.hpp:207:3
    #33 0x8a26e3 in void boost_asio_handler_invoke_helpers::invoke<boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>, boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long> >(boost::asio::ip::tcp&, boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>&) /usr/include/boost/asio/detail/handler_invoke_helpers.hpp:37:3
    #34 0x8a2657 in void boost::asio::detail::io_object_executor<boost::asio::executor>::dispatch<boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>, std::allocator<void> >(boost::asio::ip::tcp&&, boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long> const&) const /usr/include/boost/asio/detail/io_object_executor.hpp:119:9
    #35 0x8a23d6 in void boost::asio::detail::handler_work<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::asio::detail::io_object_executor<boost::asio::executor>, boost::asio::detail::io_object_executor<boost::asio::executor> >::complete<boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long> >(boost::asio::detail::binder2<nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::system::error_code, unsigned long>&, nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...)&) /usr/include/boost/asio/detail/handler_work.hpp:72:15
    #36 0x8a1b3d in boost::asio::detail::reactive_socket_recv_op<boost::asio::mutable_buffers_1, nfd::face::StreamTransport<boost::asio::ip::tcp>::startReceive()::'lambda'(boost::asio::ip::tcp&&...), boost::asio::detail::io_object_executor<boost::asio::executor> >::do_complete(void*, boost::asio::detail::scheduler_operation*, boost::system::error_code const&, unsigned long) /usr/include/boost/asio/detail/reactive_socket_recv_op.hpp:123:9
    #37 0x62afc7 in boost::asio::detail::scheduler_operation::complete(void*, boost::system::error_code const&, unsigned long) /usr/include/boost/asio/detail/scheduler_operation.hpp:40:5
    #38 0x664e9f in boost::asio::detail::epoll_reactor::descriptor_state::do_complete(void*, boost::asio::detail::scheduler_operation*, boost::system::error_code const&, unsigned long) /usr/include/boost/asio/detail/impl/epoll_reactor.ipp:776:11
    #39 0x62afc7 in boost::asio::detail::scheduler_operation::complete(void*, boost::system::error_code const&, unsigned long) /usr/include/boost/asio/detail/scheduler_operation.hpp:40:5
    #40 0x62898e in boost::asio::detail::scheduler::do_run_one(boost::asio::detail::conditionally_enabled_mutex::scoped_lock&, boost::asio::detail::scheduler_thread_info&, boost::system::error_code const&) /usr/include/boost/asio/detail/impl/scheduler.ipp:447:12
    #41 0x627d89 in boost::asio::detail::scheduler::run(boost::system::error_code&) /usr/include/boost/asio/detail/impl/scheduler.ipp:200:10
    #42 0x67468d in boost::asio::io_context::run() /usr/include/boost/asio/impl/io_context.ipp:63:24
    #43 0x65ec9d in nfd::NfdRunner::run(std::mutex&, std::condition_variable&, bool&) /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fuzzer/nfd_runner.hpp:201:15
    #44 0x65dfdc in SetUp::$_3::operator()() const /home/gtorresz/nfdfuzzer/NFD/build/../daemon/fuzzer/fuzzer.cpp:171:20
    #45 0x65de60 in int std::__invoke_impl<int, SetUp::$_3>(std::__invoke_other, SetUp::$_3&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/invoke.h:60:14
    #46 0x65dd90 in std::__invoke_result<SetUp::$_3>::type std::__invoke<SetUp::$_3>(SetUp::$_3&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/invoke.h:95:14
    #47 0x65dd48 in int std::thread::_Invoker<std::tuple<SetUp::$_3> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/thread:244:13
    #48 0x65dcf8 in std::thread::_Invoker<std::tuple<SetUp::$_3> >::operator()() /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/thread:251:11
    #49 0x65db0c in std::thread::_State_impl<std::thread::_Invoker<std::tuple<SetUp::$_3> > >::_M_run() /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/thread:195:13
    #50 0x7f428a826cb3  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xd6cb3)
    #51 0x7f428a996608 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x9608)
    #52 0x7f428a4e2102 in clone (/lib/x86_64-linux-gnu/libc.so.6+0x122102)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/hashtable.h:1538:31 in std::_Hashtable<int, std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > >, std::allocator<std::pair<int const, std::unique_ptr<nfd::fw::StrategyInfo, std::default_delete<nfd::fw::StrategyInfo> > > >, std::__detail::_Select1st, std::equal_to<int>, std::hash<int>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node(unsigned long, int const&, unsigned long) const
Actions
Copy link
 #1

Updated by Davide Pesavento about 4 years ago

Given that sendInterest may or may not send (or attempt to send) the Interest, I'd also recommend revisiting the API. Maybe we can return a bool, or a pointer or iterator to the out-record, from sendInterest?

Actions
Copy link
 #2

Updated by Eric Newberry about 4 years ago

Davide Pesavento wrote in #note-1:

Given that sendInterest may or may not send (or attempt to send) the Interest, I'd also recommend revisiting the API. Maybe we can return a bool, or a pointer or iterator to the out-record, from sendInterest?

I this this would be a good solution. Is there a use case for returning the out record instead of just whether the Interest was sent?

Actions
Copy link
 #3

Updated by Eric Newberry about 4 years ago

It looks like AsfStrategy is also be affected by this because it calls findEligibleNextHopWithEarliestOutRecord in algorithm.cpp which asserts that out record in the PIT entry exists.

Actions
Copy link
 #4

Updated by Eric Newberry about 4 years ago

  • Status changed from New to In Progress
Actions
Copy link
 #5

Updated by Eric Newberry about 4 years ago

Eric Newberry wrote in #note-3:

It looks like AsfStrategy is also be affected by this because it calls findEligibleNextHopWithEarliestOutRecord in algorithm.cpp which asserts that out record in the PIT entry exists.

Nevermind, upon closer inspection, findEligibleNextHopWithEarliestOutRecord will skip a nexthop if isNextHopEligible is false for that face. isNextHopEligible will return false if there isn't a matching out record for the face, which means findEligibleNextHopWithEarliestOutRecord won't try to get a non-existent out record.

Actions
Copy link
 #6

Updated by Eric Newberry about 4 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

I looked at all the other strategies besides the three mentions in the issues description. It seems these are the only three affected by this issue.

Actions
Copy link
 #7

Updated by Davide Pesavento about 4 years ago

  • Status changed from Closed to Code review

Not so fast :)

Actions
Copy link
 #8

Updated by Eric Newberry about 4 years ago

Davide Pesavento wrote in #note-7:

Not so fast :)

Jumped ahead one too many states I see.

Actions
Copy link
 #9

Updated by Davide Pesavento about 4 years ago

Eric Newberry wrote in #note-2:

Davide Pesavento wrote in #note-1:

Given that sendInterest may or may not send (or attempt to send) the Interest, I'd also recommend revisiting the API. Maybe we can return a bool, or a pointer or iterator to the out-record, from sendInterest?

I this this would be a good solution. Is there a use case for returning the out record instead of just whether the Interest was sent?

Well, I believe Forwarder::onOutgoingInterest already has a handle to the created out-record, so it would be cheap to return it and it would save an extra lookup if the strategy needs it. Not a major difference but why not? I would slightly prefer a pointer instead of an iterator because comparing against out_end() (or whatever it's called) feels a bit weird to me.

Actions
Copy link
 #10

Updated by Eric Newberry about 4 years ago

Davide Pesavento wrote in #note-9:

Eric Newberry wrote in #note-2:

Davide Pesavento wrote in #note-1:

Given that sendInterest may or may not send (or attempt to send) the Interest, I'd also recommend revisiting the API. Maybe we can return a bool, or a pointer or iterator to the out-record, from sendInterest?

I this this would be a good solution. Is there a use case for returning the out record instead of just whether the Interest was sent?

Well, I believe Forwarder::onOutgoingInterest already has a handle to the created out-record, so it would be cheap to return it and it would save an extra lookup if the strategy needs it. Not a major difference but why not? I would slightly prefer a pointer instead of an iterator because comparing against out_end() (or whatever it's called) feels a bit weird to me.

Is there an easy way to convert an iterator to a pointer? I can't find a straightforward way apart from &*, which would fail for a past-the-end iterator.

Actions
Copy link
 #11

Updated by Davide Pesavento about 4 years ago

The end iterator gets mapped to nullptr obviously.

Actions
Copy link
 #12

Updated by Eric Newberry about 4 years ago

  • Status changed from Code review to Closed
Actions
Copy link

Also available in: Atom PDF