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

The memo of 8th NDN hackathon: A list of TODOs #40

Open
Zhiyi-Zhang opened this issue Mar 11, 2019 · 11 comments
Open

The memo of 8th NDN hackathon: A list of TODOs #40

Zhiyi-Zhang opened this issue Mar 11, 2019 · 11 comments

Comments

@Zhiyi-Zhang
Copy link
Member

Zhiyi-Zhang commented Mar 11, 2019

The ndn-lite team has successfully finished the demo development in the 8th NDN Hackathon and won the second prize.
During the development, our team members have identified a number of issues that should be addressed in future work, including bug fixes, usability enhancement, code optimization, doc enhancement, etc.
Let's use this track as a memo so that we won't forget to address these issues in the near future.

@peurpdapeurp
Copy link
Collaborator

Need to flesh out schematized trust implementation more; the current implementation was enough for the goal of the hackathon because the schematized trust rule checking was very simple, but need to add more library level support for more complicated things (i.e., library right now supports rules, but still need to add library api's to create actual schemas, groupings of rules)

@peurpdapeurp
Copy link
Collaborator

One of the general things needed was to make the library more user friendly; this is something I was also reminded of even before the hackathon, as one of Yanbiao's students tried using the sign-on protocol and ran into trouble setting it up, generating credentials for it, etc; I will update the official NDN-IoT Android / nRF52840 examples to be more well documented and easy to use, and try to create programs such as a credential generator to make using the sign-on protocol / examples easier.

@zjkmxy
Copy link
Collaborator

zjkmxy commented Mar 11, 2019

There's something wrong with forwarder and encode module. We need further test. Make sure everything goes well, compatible with NFD.

@zjkmxy
Copy link
Collaborator

zjkmxy commented Mar 11, 2019

We needs more document. Both Wiki tutorials and Doxygen documentations. Current Doxygen pages seldom help.

@Zhiyi-Zhang
Copy link
Member Author

Need to update Encrypted Content API for key name learning.
Need key fetch.
Better to put all init functions into one init function.

@tianyuan129
Copy link
Member

Memory Usage in Encoding/Decoding

Naturally, we declare strings and then construct a ndn_name_t by memcpy from string bytes. Then we TLV encode ndn_name_t by reading memory buffer it holds. A typical ndn_name_t's size is 384B, as macro NDN_NAME_MAX_BLOCK_SIZE indicates.

Actually, there is a memory waste here. Since from string to ndn_name_t and from ndn_name_t to TLV encoded block are usually consecutive operations, having a copy for string bytes is unnecessary, because encoding functions can directly access strings' raw bytes, besides copied bytes in name_component_t. In name_component_t, pointers to these certain bytes would be enough. So as Interest and Data encoding.

The same consideration applies to decoding as well. In on_data or on_interest callbacks, decoding and analyzing (e.g., service info update) are also usually consecutive operations. The latter one can access the raw bytes by const uint8_t* interest/const uint8_t* data, besides the copied version in ndn_interest_t/ndn_data_t. Maybe we don't need to require two structs hold large memory buffer.

Therefore I'm thinking about the possibility of replacing the some memory holding uint8_t arrays with pointers. That means, let encoding/decoding structs only serve as helpers to understand and organize raw bytes and no memory holding responsibility. For doing so, some structs should change to look like this.

Name Component

/**
 * The structure to represent the Name Component.
 */
typedef struct name_component {
  /**
   * The component type.
   */
  uint32_t type;
  /**
   * The value which name component holds. (not include T and L)
   */
  uint8_t* value;
  /**
   * The size of component value buffer.
   */
  uint32_t size;
} name_component_t;

Interest Parameters

/**
 * The structure to represent the Interest parameters element.
 */
typedef struct interest_params {
  uint8_t* value;
  uint32_t size;
} interest_params_t;

Data

/**
 * The structure to represent an NDN Data packet
 * The best practice of using ndn_data_t is to first declare a ndn_data_t object
 * and init each of its component to save memory
 */
typedef struct ndn_data {
  /**
   * Data Name Value (not including T and L)
   */
  ndn_name_t name;
  /**
   * Data MetaInfo Value (not including T and L)
   */
  ndn_metainfo_t metainfo;
  /**
   * Data Content Value (not including T and L)
   */
  uint8_t* content_value;
  /**
   * Data MetaInfo Content Value Size
   */
  uint32_t content_size;
  /**
   * Data Signature.
   * This attribute should not be manually modified.
   * Use ndn_data_tlv_encode_*_sign functions to generate signature.
   */
  ndn_signature_t signature;
} ndn_data_t;

Signature still have their memory buffer, because we don't know signature value until we start encoding. And I suggest when we have to hold Data/Interest/Name, we hold them in TLV encoded format, like what forwarder does in riot branch.

@zjkmxy
Copy link
Collaborator

zjkmxy commented Mar 11, 2019

Memory Usage in Encoding/Decoding

You said what I wanted to say for a long time, though I think we may not take your codes.
1.
Actually I once proposed a structure like this:

struct Block{
  unit32_t m_type;
  uint32_t m_size;
  Block *m_next; //The right brother of it in a TLV tree.
  union{
    Buffer *m_value;  //Pointer to the value, if it's encoded, m_size > 0
    Block *m_firstChild; //Pointer to the first child, if it's not encoded.
  };
};

But it doesn't fit ndn-lite scenario due to the absence of dynamic memory management.
And this also applies to the codes you gave. The member uint8_t* content_value; is a pointer to somewhere else, which is hard to manage.

The problem is there is a conflict between two purposes: efficiency and making users happy.
Our forwarder needs to minimize memory cost first, and then to improve the performance, we knows our system very well. We uses some information a packet carries but not modifies it.
Users, on the other hand, doesn't know NDN very well. They also need to friendly interfaces to construct and modify a packet. They don't care performance that much.
So my suggestion is to make 2 encoding systems, one for "beginners" which are easy to use, the other one is for "experts" which are well-tuned on performance.

Actually what may bother is not only the signature but also all the various sizes. Even the length of "length" in TLV is variable: 1, 2, 4 or 8 bytes.
But on the other hand, if the value is immutable, the length is fixed.

Suddenly I find something interesting:

Interest interest(Name("/example/testApp/randomData"));
interest.setInterestLifetime(2_s); // 2 seconds
interest.setMustBeFresh(true);
m_face.expressInterest(interest, .... //Codes from ndn-cxx examples

When we construct interest, actually everything (LifeTime, MustBeFresh, etc) is ready.
Why we first construct an empty Interest and then modify it?

@tianyuan129
Copy link
Member

So my suggestion is to make 2 encoding systems, one for "beginners" which are easy to use, the other one is for "experts" which are well-tuned on performance.

I agree with two encoding systems. Maybe the well-tuned encoding system should directly use URI + Lifetime + Parameters + Sth else as inputs, and no intermediate state like ndn_interest_t.

Why we first construct an empty Interest and then modify it?

I think it's because C don't have constructors so we must declare an empty one first?

@tianyuan129
Copy link
Member

I think it's because C don't have constructors so we must declare an empty one first?

Of course we can also use method like struct A a={.b = 1,.c = 2};

@zjkmxy
Copy link
Collaborator

zjkmxy commented Mar 12, 2019

I think it's because C don't have constructors so we must declare an empty one first?

OO concepts are semantic not syntax things, so C has "constructors".
And actually, those codes are C++ codes.
I mean, as everything is ready, why don't we just supply a function called "make-interest" and ask the user give ALL things at once, and the result is IMMUTABLE?
The user don't need to know what's inside since it's immutable.

@tianyuan129
Copy link
Member

I mean, as everything is ready, why don't we just supply a function called "make-interest" and ask the user give ALL things at once, and the result is IMMUTABLE?
The user don't need to know what's inside since it's immutable.

How about we added a light folder in current encode module, and put these "all in one" encoding functions in it? We need these memory efficient encoding/decoding functions, otherwise NDN Lite will be hard to use on devices who run RIOT, which limits main thread stack size, around 1.5KB.

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