Project

General

Profile

Actions
Copy link

Feature #3174

closed

NACK counters

Added by Junxiao Shi over 8 years ago. Updated over 8 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Eric Newberry
Category:
Management
Target version:
v0.4
Start date:
Due date:
% Done:

100%

Estimated time:
1.50 h

Description

Add NInNacks and NOutNacks counters to ForwarderStatus and FaceStatus structs.

Related issues 1 (0 open — 1 closed)

Related to NFD - Feature #3177: LpFace countersClosedJunxiao Shi

Actions
Actions
Copy link
 #1

Updated by Junxiao Shi over 8 years ago

  • Blocks Task #3088: Refactor Face as LinkService+Transport added
Actions
Copy link
 #2

Updated by Junxiao Shi over 8 years ago

  • Assignee set to Eric Newberry

This shall solve NFD:commit:c2ee303880e6bfd79c107a4cda2cc28194356409 face-counters.hpp line328 problem.

Actions
Copy link
 #3

Updated by Eric Newberry over 8 years ago

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

Updated by Eric Newberry over 8 years ago

What TLV type values should I assign NInNacks and NOutNacks?

Actions
Copy link
 #5

Updated by Junxiao Shi over 8 years ago

  • Description updated (diff)

Answer to note-4: see updated spec with links to relevant wiki pages. TLV-TYPE codes are on the bottom of each page.

I notice that the TLV-TYPE codes I previously assigned in ForwarderStatus have conflicts in Face dataset. I have changed the codes to avoid this conflict.

Actions
Copy link
 #6

Updated by Eric Newberry over 8 years ago

It appears that LinkCounters also has NInPackets and NOutPackets counters, so ForwarderStatus and FaceStatus need those as well.

Actions
Copy link
 #7

Updated by Junxiao Shi over 8 years ago

Reply to note-6:

Yes, but not now.

In #3088, don't copy packet counters.

Actions
Copy link
 #8

Updated by Eric Newberry over 8 years ago

  • Status changed from In Progress to Code review
Actions
Copy link
 #9

Updated by Junxiao Shi over 8 years ago

  • Blocks deleted (Task #3088: Refactor Face as LinkService+Transport)
Actions
Copy link
 #10

Updated by Junxiao Shi over 8 years ago

Actions
Copy link
 #11

Updated by Junxiao Shi over 8 years ago

note-7 says packet counters need not be copied into FaceStatus.
Similarly, #3088 could skip copying Nack counters as well.

Therefore, this issue is no longer blocking #3088.

Actions
Copy link
 #12

Updated by Eric Newberry over 8 years ago

  • Status changed from Code review to Closed
  • % Done changed from 0 to 100
Actions
Copy link
 #13

Updated by Alex Afanasyev over 8 years ago

  • Status changed from Closed to Feedback

I have missed that originally, but my recent attempt to upgrade NFD version on android revealed that this is a breaking change for other libraries that implement ForwarderStatus and FaceStatus.

I would like to propose to make this change less intrusive: move all new fields to the end of the TLV. This way old libraries are not getting affected while parsing new status packet.

Actions
Copy link
 #14

Updated by Junxiao Shi over 8 years ago

I disagree with note-13.

A correct implementation of a decoding procedure should check nothing extra exists at the end of a parent element, so any changes of a required element would be a breaking change, regardless of where it's placed.

The ordering "NInInterests, NInDatas, NInNacks, NOutInterests, NOutDatas, NOutNacks, NInPackets, NInBytes, NOutPackets, NOutBytes" follows the logical order of "incoming network later counters, outgoing network layer counters, incoming link layer counters, outgoing link layer counters".
The alternate would be a nonsense ordering.

Recommendation for NFD-Android:

  • Do not upgrade NFD until the library is ready.
  • If the library must support both versions of NFD Management protocol, it may implement the additional field as if it's an optional field.
Actions
Copy link
 #15

Updated by Junxiao Shi over 8 years ago

  • Status changed from Feedback to Closed

Closing because there's no more comment in past 60 days.

Actions
Copy link

Also available in: Atom PDF