Task #4391
closedFeature #1624: Design and Implement Congestion Control
Congestion Control: Test Local Link Loss Detection
100%
Description
Updated by Anonymous about 7 years ago
- Related to Feature #3823: Congestion Control: design Local Link Loss Detection added
Updated by Anonymous about 7 years ago
Here's the relevant earlier discussion:
Eric Newberry wrote:
I ran a test, but it appears that
onLostPacket
is being called for every frame, even if it has been successfully acknowledged on its first transmission.
Klaus Schneider wrote:
What might cause this bug?
Eric Newberry wrote:
No idea. The timeout calling
onLpLostPacket
should be cancelled when its frame goes out of scope (when theUnackedFrag
object containing it is deleted). Given that the initial RTO is 1 second and ndnping is reporting an RTT in the 10s of milliseconds on the link, I am at a loss.
Davide Pesavento wrote:
Klaus Schneider wrote:
Looks like we need a more formal integration test for this?
Not necessarily an integration test. A unit test should be able to catch this kind of bugs.
Updated by Eric Newberry about 7 years ago
- Tracker changed from Task to Bug
- Status changed from New to In Progress
- Priority changed from Normal to High
- Target version set to v0.7
Updated by Eric Newberry about 7 years ago
- Status changed from In Progress to Feedback
It looks like this may not actually be an issue. I forgot that reliability must be manually enabled on both ends of the link for the system to function. It looks like the packets were being received, but not acknowledged, leading to onLpPacketLost
being called due to the expiration of the timeout.
I ran the test again, using tc
to drop approximately 10% of packets on the receiver and it appears that the strategy was notified whenever onLpPacketLost
was called (with retx=0
, so this is expected upon every call to onLpPacketLost
).
Updated by Anonymous about 7 years ago
Can you give a specific, step-by-step instruction of your testing? (i.e. list the terminal commands and files that need editing).
Updated by Davide Pesavento about 7 years ago
- Tracker changed from Bug to Task
- Priority changed from High to Normal
So there's no bug here, correct?
Updated by Eric Newberry about 7 years ago
Davide Pesavento wrote:
So there's no bug here, correct?
Not to my knowledge. Here's how I performed my testing (if my memory serves right):
- Set up a VirtualBox VM with a host-only network adapter, allowing it to be reached over the network from the host machine. Install an unmodified version of NFD on the VM.
- On the host machine, modify, compile, and install NFD to include a message to
cout
inBestRouteStrategy2::onDroppedInterest
(creating this function in the process), as well as inLpReliability::onLpPacketLost
to print beforeonDroppedInterest
is signaled. - Start NFD both on the host machine and the VM.
- Create a UDP face on the host machine, with the VM as the remote endpoint, enabling reliability:
nfdc face create udp4://[remote-ip-addr] reliability on
- Run the corresponding nfdc command on the VM to enable reliability from that end of the connection.
- Use tc on the VM to simulate 10% packet loss on the host-only interface:
sudo tc qdisc add dev enp3s0 root netem loss 10%
- Start ndnpingserver on the VM to serve a particular prefix.
- Start ndnping on the host machine for the prefix mentioned above.
- Examine the output of NFD on the host machine to ensure that both messages are being printed to
cout
.
Updated by Davide Pesavento about 7 years ago
Great. I suggest rejecting this task unless someone can describe an integration test scenario that tests something not already covered by unit tests.
Updated by Davide Pesavento about 7 years ago
- Description updated (diff)
Oh wait, this task is about unit tests, not integration tests. LpReliability
already has numerous unit tests though, is there something that is not covered by them? Please explain in the description exactly what tests are needed, if any, otherwise reject.
Updated by Eric Newberry about 7 years ago
Davide Pesavento wrote:
Oh wait, this task is about unit tests, not integration tests.
LpReliability
already has numerous unit tests though, is there something that is not covered by them? Please explain in the description exactly what tests are needed, if any, otherwise reject.
We could add a unit test to ensure that the loss timeout is cancelled when an Ack in received for the packet. I already have this test case written, so I'll go ahead and push it for code review.
Updated by Anonymous about 7 years ago
Eric Newberry wrote:
Davide Pesavento wrote:
Oh wait, this task is about unit tests, not integration tests.
LpReliability
already has numerous unit tests though, is there something that is not covered by them? Please explain in the description exactly what tests are needed, if any, otherwise reject.We could add a unit test to ensure that the loss timeout is cancelled when an Ack in received for the packet. I already have this test case written, so I'll go ahead and push it for code review.
Sounds good.
Updated by Davide Pesavento about 7 years ago
- Status changed from Feedback to In Progress
You can get a rough idea of what is not covered here: http://jenkins.named-data.net/job/NFD/5879/OS=code-coverage/LCOV_Coverage_Report/daemon/face/lp-reliability.cpp.gcov.frameset.html
For example, receiving Acks for unknown TxSequence is not tested, the nRetransmitted
counter is not tested, and so on.
Updated by Eric Newberry about 7 years ago
- Status changed from In Progress to Code review
- % Done changed from 0 to 100
Updated by Eric Newberry about 7 years ago
- Status changed from Code review to Closed