NFD » ndn-cxx

I've started working on the code and have implemented the #2883 API. However, some of the unit tests are failing with the new API.

One question: How is lp::Packet supposed to work accept a bare network packet when decoding, as stated in the API description for Face::onReceiveElement?

Answer to note-5:

From NDNLPv2:

bare network packets SHOULD be interpreted as an LpPacket with the bare network packet as its Fragment

Junxiao Shi wrote:

Answer to note-5:

From NDNLPv2:

bare network packets SHOULD be interpreted as an LpPacket with the bare network packet as its Fragment

Does the current implementation of LpPacket work with this spec? I don't believe it does.

Please disregard note 7. I looked over the current implementation of the Packet API and it should work.

However, I also noticed that some changes will be necessary in DummyClientFace to get unit tests working again. This includes changing the DummyClientFace::construct to process LpPackets instead of bare network packets.

The first version of this feature is complete. I'm waiting on #2879 to merge before submitting it for review.

Implementation in http://gerrit.named-data.net/2296 is mostly correct.
However, it cannot be tested or merged until NFD side can recognize NDNLPv2.
This Change is going to drag for a long time.

To avoid further disputes on "merge vs rebase", I suggest:

1. Merge feature-ndnlp-rev-2 branch into master branch now. That branch completes the functionality of NDNLPv2 packet format, but does not use it in Face.
2. Retarget this Change to master branch, because it's the only commit needed to enable NACK.
3. When NFD can recognize NDNLPv2, test and merge.

I have no objections to retarget this change to the master branch.

Design question from gerrit review: When NACK is received by a node, how should it be matched against pending entries? Should it match exactly interest (same nonce, interest lifetime, link) or ignore some fields?

Alex Afanasyev wrote:

Design question from gerrit review: When NACK is received by a node, how should it be matched against pending entries? Should it match exactly interest (same nonce, interest lifetime, link) or ignore some fields?

I believe it should just be for the same Nonce. From what I understand, the Nonce should be unique to an Interest (within reasonable expectations).

20150810 conference call discussed note-13.

On a point-to-point link connecting two forwarders, the full Interest including the Nonce should match precisely.

Not comparing the Nonce causes undesired effect:

1. P has sent an Interest with Nonce n1 to Q.
2. Q is returning NACK to P for the Interest from step 1.
3. At the same time, P is retransmitting a similar Interest with Nonce n2 to Q.
4. When Q receives the Interest from step 3, it will attempt to forward it again.
5. When P receives the NACK from step 2, it would mistakenly believe that Q has given up.

The behavior of an app-forwarder link should be no different from a point-to-point link connecting two forwarders.
Therefore, the full Interest should be compared equal.

Minor reworking of the code is necessary to implement a design change described in #3178 note-3:

LpPacket should be encoded as bare Interest/Data, if no header field is present.

This should be implemented in LpPacket::wireEncode.

So now I have to modify the size in the unit-test?

With ndn-cxx:commit:8d2c2787048c4e4f96b43356d9f36d4d601b08fd (Nack in client Face) and NFD:commit:b03c3398acf430144506370b01036181af786cdc (including: Nack in best-route strategy), IntegrationTests test_interest_loop.test_best_route test case fails to terminate.

This is caused by:

1. ndn-traffic-generator client (application) expresses Interest with Face::expressInterest(Interest, OnData, OnTimeout).
2. When a forwarder detects a loop, a Nack is returned.
3. The Nack eventually goes to the ndn::Face used by application.
4. ndn::Face can understand the Nack, but it cannot inform the application because no NackCallback is provided.
5. Application is waiting for either OnData or OnTimeout to be invoked, but this never happens.

This problem isn't limited to ndn-traffic-generator.
Many other applications, including ndnping, also assume either OnData or OnTimeout would be invoked, and could deadlock if a Nack comes back.

Possible solution:
In Face::expressInterest(Interest, OnData, OnTimeout) deprecated overload, pass OnTimeout as the NackCallback to the non-deprecated overload.
Therefore, applications designed for pre-Nack era receives a timeout callback if Nack arrives.

The timeout callback can happen earlier than InterestLifetime expiry, but this shouldn't matter because timeout precision is never guaranteed.

note-21 solution is implemented in commit:bf15af04d5fea954762159e2e46fc0f5f10ba05b and the test_interest_loop.test_best_route test case is now passing.

Commit 83872fd229b8d87e8e2aac25ba1dac3a69740bfd has a weird piece of code in dummy-client-face.cpp. Basically a tlv::Error is caught and discarded, and a new exception of the same type, but with a different what string, is immediately thrown again from the catch block. This seems pointless, and loses the information contained in the original exception. What was the reason for this behavior?

Davide Pesavento wrote:

Commit 83872fd229b8d87e8e2aac25ba1dac3a69740bfd has a weird piece of code in dummy-client-face.cpp. Basically a tlv::Error is caught and discarded, and a new exception of the same type, but with a different what string, is immediately thrown again from the catch block. This seems pointless, and loses the information contained in the original exception. What was the reason for this behavior?

I agree. This is probably a bad design on my part. I'll work on a fix.