named-data / python-ndn

An NDN client library with AsyncIO support in Python 3
https://python-ndn.readthedocs.io/en/latest
Apache License 2.0
24 stars 17 forks source link

Add support for NDNLP packets #20

Open JonnyKong opened 3 years ago

JonnyKong commented 3 years ago

Currently, python-ndn does not handle NDN link protocol packets, except NACK.

For example, when the network gets congested, NFD will wrap Interest/Data packets in NDNLP packet with congestion markings. The python-ndn library currently discards such packets, and shows a warning message as follows:

WARNING:Unable to decode received packet

I have attached an example NDNLP packet here: packet.zip

zjkmxy commented 3 years ago

Will fix this ASAP.

yoursunny commented 3 years ago

NDNLP has a "critical vs non-critical TLV-TYPE" distinction. If you don't want to handle congestion mark, you can ignore it since it has a non-critical TLV-TYPE number. On the other hand, if you are unable to handle PIT token or fragmentation, you have to drop the packet as those have critical TLV-TYPE numbers.

susmit85 commented 3 years ago

Hi, can we please fix this? We don't want to rely on application retransmission since that adds to congestion.

zjkmxy commented 3 years ago

Hi, can we please fix this? We don't want to rely on application retransmission since that adds to congestion.

I'm currently working on this. I'm sorry that this may take some time, but you will see it in several weeks.

zjkmxy commented 3 years ago

Hello. I just pushed a patch. Sorry for being late. After I upgraded my Mac, everything was broken ... I have only tested it in the integration test, but not with NFD yet. (I need to reinstall NFD and fix possible problems which may take more time.) Please check if it solves the problem or brings more bugs. Thanks

yoursunny commented 3 years ago

As of b5e60a67ee8b72582b25bb1262e913c09f0cfbe5, the implementation is incorrect. For example, if a fragmented packet arrives, this implementation would incorrectly accept this packet, despite that it lacks a reassembler.

Please see NDNLPv2 protocol for requirements on processing critical vs non-critical TLV-TYPE numbers. It differs from evolvability rules of NDN network layer packets.

zjkmxy commented 3 years ago

This implementation does not handle fragmentation yet. Will be added later.

yoursunny commented 3 years ago

This implementation does not handle fragmentation yet. Will be added later.

You missed the point. It's OK to not handle a certain NDNLPv2 field that has a critical TLV-TYPE number, but then the decoder MUST drop any NDNLPv2 frame that uses such field.