NFD

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

"Unix Stream Transport" section:

• "when a UnixStreamTransport receives a packet that .. is incomplete, the Transport enters a failed state" is wrong. If a packet is incomplete (the remaining buffer is less than TLV-LENGTH), StreamTransport would wait for additional input.
• Describe how buffer is managed in StreamTransport.
• A decoding failure (e.g. Interest has no Name) would fail the transport.

"Ethernet Transport" section:

• What's the behavior of a decoding error?

• Design choice of not supporting unicast:

  Each Ethernet link is a broadcast media which naturally supports multicast communication.
  Although it's possible to use the broadcast media as many point-to-point links, doing so loses NDN's native multicast benefits, and increases the workload of NFD hosts.
  Therefore, we decide it's best to use Ethernet link as multi-access only, and not support Ethernet unicast.

"UDP unicast Transport" section:

• UdpChannel should create UdpUnicastTransport not TcpTransport.
• Describe what's the "MTU" passed to LinkService.

"UDP multicast Transport" section:

• I don't know exactly the difficulty of IPv6 multicast. It has something to do with FreeBSD or the like. There's simply no use case, because almost all kernels support IPv4 multicast, and we are communicating on a single hop only.

"TCP Transport" section:

• It's sufficient to say TcpTransport is a specialization of StreamTransport, and then refer to Unix stream transport section for other details.
• If there's different between TCP and Unix, they should be described in this section.

"Developing a New Transport" section:

• StreamTransport and DatagramTransport are *template*s, not *superclass*es. They can be *specialize*d, not *inherit*ed.

Review of Face section at nfd-docs:commit:e5421bb278f4fa6c5ed628b584fe64697ce7f9e9.

General:

• The term "Transport" shouldn't be capitalized everywhere. It should be capitalized when referring to the Transport base class. It should stay lower-case when referring to the specific type of transport of the subsection.
• Likewise, "Face" shouldn't be capitalized everywhere.
• "non-valid" => "invalid"

"Unix Stream Transport" section:

• "the Transport copies any remaining non-valid packets to the beginning of the buffer" conflicts with "when a UnixStreamTransport receives a packet that .. is not valid, the Transport enters a failed state and closes". What remains in buffer after valid packets are processed is just "remaining octets"; they are not "non-valid packets" and not even "packets".

"Ethernet Transport" section:

• Why does Ethernet MTU has anything to do with UDP?

"UDP unicast Transport" section:

• Describe the behavior of each persistency setting. Such as, ICMP errors are ignored with permanent persistency.

"Developing a New Transport" section:

• Indicate that static properties shall be set in the constructor.

Revisions based upon note 7 have been pushed.

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

"UDP unicast Transport" section:

• The description about disabling PMTU discovery should follow the motivation of relying on IP fragmention.
  The logic is: we want to rely on IP fragmentation => IP packets should not have DF flag => we need to disable PMTU discovery

Revisions based upon note 9 have been pushed.

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

"UDP unicast Transport" section:

• The description about disabling PMTU discovery should follow the motivation of relying on IP fragmention.
  
  The logic is: we want to rely on IP fragmentation => IP packets should not have DF flag => we need to disable PMTU discovery

  UDP unicast transport does not attempt to fit packets to the MTU of the directly connected link, but relies on IP fragmentation, so that intermediate IP routers can further fragment the packet as it goes into a link with lower MTU.
  Therefore, UDP unicast transport does not set the Don't Fragment (DF) flag on outgoing packets; on Linux, this is achieved by disabling PMTU discovery on the socket.

Revisions based upon note 11 have been pushed.

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

"UDP unicast Transport" section:

• The description about disabling PMTU discovery should follow the motivation of relying on IP fragmention.
  
  The logic is: we want to rely on IP fragmentation => IP packets should not have DF flag => we need to disable PMTU discovery

  UDP unicast transport does not attempt to fit packets to the MTU of the directly connected link, but relies on IP fragmentation, so that intermediate IP routers can further fragment the packet as it goes into a link with lower MTU.
  Therefore, UDP unicast transport does not set the Don't Fragment (DF) flag on outgoing packets; on Linux, this is achieved by disabling PMTU discovery on the socket.

Again, why is IP fragmentation discussed in both the second and the fifth paragraph under the static attribute list?
Delete both, and add the text above as the second paragraph.

Revisions based upon note 13 have been pushed.

I believe the second paragraph now sums up the point you want it to make about intermediate IP fragmentation.