NFD

I could not update figure 19 (app-work-flow.pdf) as there does not seem to be a source file for the figure.

I have requested the original author to upload the source file into the repository slides/ folder.

Junxiao Shi wrote:

Review on nfd-docs:commit:8d6264984a423c6835f99d78e1f4b5bd02ea80c9

• section overview paragraphs

  • Spell out "route inheritance flags", not just "flags".
  • It's unclear what "common abstraction" means in "the RIB manager also provides a common abstraction".

• Initializing the RIB manager section

  • Registering /localhost/nfd/rib prefix has nothing to do with receiving notifications.
  • LocalControlHeader has been replaced with NDNLPv2. In particular, IncomingFaceId is needed by RIB manager.
  • Initialization of auto prefix propagator is missing.

• RIB Entry section

  • expires should be the absolute expiry time, not a duration.

• Prefix Registeration Flags section

  • It's called "route inheritance flags" per RibMgmt.

• On Request section

  • Rename the section as "Command Processing".
  • A command can't (un)register an entry, but can (un)register a route. An entry contains multiple routes of the same name prefix.

I've pushed a new commit to address these comments.

Review of RIB section at nfd-docs:commit:f936e1c8fa281ba395972a861a4ad3e90c797307.

Section overview paragraphs:

• Define what is "routing information base" upfront, and indicate its relation to and difference from FIB. Introduce concepts such as Route vs RIB entry.
• "RIB .. communicates with NFD through Face abstraction only": what does "abstraction" mean? It's just "face".

"Initializing the RIB manager" section:

• What does "localhost and localhop validation rules" mean?
• What is "Control Header" in the timing diagram?
• "set Interest filter" is a concept within a client library. Its effect is sending a prefix registration command to NFD-RIB.
• There's no RIB entry "corresponds to" a face being created or destroyed. A face can, however, appear as the nexthop of a route.

"Communication with the RIB Manager", "RIB Entry", "Route Inheritance Flags" sections:

• It's unnecessary to repeat the details of RibMgmt protocol. Add a link or citation instead. The devguide should focus on how they are implemented.
• If you want to list the features of RibMgmt protocol, don't forget the StatusDataset portion.

"Command Processing" section:

• This section should be arranged before FIB updater and automatic prefix propagation sections. In this section, introduce the entire workflow of processing a register/unregister command, and when FIB updater and automatic prefix propagation would be invoked.
• How does the RIB manager validate a request? Relate this to "localhost and localhop validation rules" introduced in initialization section.
• What is "Control Header"?

"Auto Prefix Propagator" section:

• Define the concept of "local" and "connected gateway router" with a topology figure.
• "When to Propagate" section first paragraph third line goes off page boundary.
• What is "revocation"?
• In "some of which .. are actually set or modified in this method", what does "this method" refer to?

"Extending RIB Manager" section:

• ".. supports only two commands" is inaccurate. We have a dataset, and automatic prefix propagator.

I've pushed a new commit to address the comments in #3392-7.

Junxiao Shi wrote:

• ".. supports only two commands" is inaccurate. We have a dataset, and automatic prefix propagator.

I agree with the list command for the dataset, but aren't automatic prefix propagation commands covered by register and unregister?

Review of RIB section at nfd-docs:commit:8c2ceec3492e9f7105569cd0e7dbb036885992a1.

Section overview paragraphs:

• It's unclear what's the difference between "nexthop" and "outgoing face".
• "Face" should not be capitalized in most places.

"Initializing the RIB manager" section:

• What does "localhost and localhop validation rules" mean? The concept should be introduced upfront, not in parenthesis. Suggestion: introduce the major components of RIB management in introduction paragraphs, mention that commands need to be validated for authentication and authorization; use the term "command validation rules" rather than "localhost and localhop validation rules".
• How is the automatic prefix propagator initialized?

"Communication with the RIB Manager", "RIB Entry", "Route Inheritance Flags" sections:

• It's unnecessary to repeat the details of RibMgmt protocol. Add a link or citation instead. The devguide should focus on how they are implemented.
• If you want to list the features of RibMgmt protocol, don't forget the StatusDataset portion.

"Command Processing" section:

• In "register route" procedure, after the FIB update process is successful, it's too late to look at the FaceId in the request.

"Auto Prefix Propagator" section:

• How does auto prefix propagator detect there's active connectivity to a gateway router?
• Can auto prefix propagator deal with multiple connected gateway routers?

"Extending RIB Manager" section:

• list is not a command. Only register and unregister are commands.
• In addition to two commands, there are more features: the RIB dataset, and the auto prefix propagator.

I've pushed a new commit to address the comments in #3392-9.

Review of RIB section at nfd-docs:commit:02f00bb05a7723c305f93723bfd5b456fc30467a.

Section overview paragraphs:

• It's unobvious why "a RIB entry can contain multiple Routes toward the same outgoing face". Explain the reason.

"Initializing the RIB manager" section:

• The initialization steps should be represented as an ordered list (numbered list), instead of an unordered list.
• "the prefix registration/unregisteration commands" should omit "the", because it's not referring to one particular command.
• Why does RibManager need to know the FaceId from which a request to RIB dataset is received?
• "the destroyed face" should be "a destroyed face", because it's not referring to one particular face.
• It's inaccurate to say "RIB entries which contain Routes with a destroyed face can be updated". It's better to say ".. so that when a face is destroyed, Routes associated with that face can be deleted".

"Communication with the RIB Manager", "RIB Entry", "Route Inheritance Flags", "RIB Status Dataset" sections:

• Do not create a citation for every section in RibMgmt protocol. It's enough to use only one citation.

"Communication with the RIB Manager", "RIB Entry" sections:

• Information contained in these section should be merged into RIB section overview paragraphs, because they are general concepts introduced over there.

"Route Inheritance Flags" section:

• Information contained in this section should be merged into FIB Updater section, because it's FibUpdater's responsibility to compute FIB entries using route inheritance flags and determine the cost of nexthop records.

"Command Processing" section:

• "if the Interest is valid" is inaccurate, because that means the Interest has valid TLV structure which should be true for every Interest. It's more accurate to say "if the signature on the command request is valid".

• After "If the FaceId is 0 ... (self-registration)", "if the command is a self-registration" should not be repeated.

  If the FaceId is 0 or omitted in the command parameters, it means the requester wants to register a route to itself. This is called a self-registration. In this case, the RIB manager creates the RIB update with the requester's FaceId from the IncomingFaceId field.

"Auto Prefix Propagator" section:

• How does auto prefix propagator detect there's active connectivity to a gateway router?
• Can auto prefix propagator deal with multiple connected gateway routers?

"Extending RIB Manager" section:

• "auto prefix propagator" is a component; the corresponding feature is called "automatic prefix propagation".

I've pushed a new commit to address the comments in #3392-11.

Junxiao Shi wrote:

"Auto Prefix Propagator" section:

• How does auto prefix propagator detect there's active connectivity to a gateway router?
• Can auto prefix propagator deal with multiple connected gateway routers?

In nfd-docs:commit:02f00bb05a7723c305f93723bfd5b456fc30467a, I addressed these questions in the following ways:

The Auto Prefix Propagator considers the connection to the router as active if the local RIB has the link local NFD prefix registered and inactive if the local RIB does not have the link local NFD prefix registered.

The Auto Prefix Propagator can be enabled by the RIB manager to propagate necessary knowledge of local prefix registrations to one or many connected gateway routers.

Please let me know if there is additional information I should provide.

Review of RIB section at nfd-docs:commit:d77bef6faf22845d1dbbfafc20dd878f040ce5de.

"Command Processing" section:

• How is expiration period processed?

"FIB Updater" section:

• "in the case of" => "in case of"

"Auto Prefix Propagator" section:

• Introduce "connectivity detection" positively, not through a negative sentence like "does not indicate a connectivity to the router".
• Introduce the requirement of connectivity positively, not through a negative sentence after "Further,".
• "can not" => "cannot"
• As far as I know, auto prefix propagator cannot deal with multiple connected gateway routers.

I've pushed a new commit to address the comments in #3392-13.

Review of RIB section at nfd-docs:commit:f02cfaa57caa2883da696ae5b9f03ce0ce17ba9d.

"Command Processing" section:

• Semantically, both "omitted ExpirationPeriod" and "ExpirationPeriod set to maximum" mean ExpirationPeriod is infinity. This is defined in RibMgmt protocol and there's no need to repeat it. Just say, "If ExpirationPeriod is not infinity, ...".
• How is "route scheduled to expire" performed? Indicate which action (or function) would happen at expiration.

"Auto Prefix Propagator" section:

• It's inaccurate to equal afterInsertRibEntry to "attempt for propagation". The auto prefix propagator first looks at the prefix: if it's /localhop/nfd, this indicates a connection to HUB is established; otherwise, it's an attempt for propagation.
• Explain why only one connect gateway router can be supported by the current protocol. Hint: #3142.

I've pushed a new commit to address the comments in #3392-15.

Review of RIB section at nfd-docs:commit:04ba9452ef177bf57ec809048adbf1f36dc4a2ab.

"Command Processing" section:

• The discussion about route expiration appears in the middle of the registration procedure, which is very confusing. It's better to pull it out to a separate paragraph, after both registration and unregistration are discussed.

I pushed the update addressing note 17.