Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for NDNLP packets #20

Open
JonnyKong opened this issue Nov 6, 2020 · 8 comments
Open

Add support for NDNLP packets #20

JonnyKong opened this issue Nov 6, 2020 · 8 comments

Comments

@JonnyKong
Copy link
Contributor

JonnyKong commented Nov 6, 2020

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
Copy link
Member

zjkmxy commented Nov 6, 2020

Will fix this ASAP.

@yoursunny
Copy link
Member

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
Copy link

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

@zjkmxy
Copy link
Member

zjkmxy commented Nov 13, 2020

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 added a commit to zjkmxy/python-ndn that referenced this issue Nov 15, 2020
@zjkmxy
Copy link
Member

zjkmxy commented Nov 15, 2020

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
Copy link
Member

As of b5e60a6, 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
Copy link
Member

zjkmxy commented Nov 15, 2020

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

@yoursunny
Copy link
Member

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.

zjkmxy added a commit that referenced this issue Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants