NFD

What if the field is missing from a packet - is the packet treated as discovery or non-discovery in that case? Also, I assume this field can be successfully ignored (given the two LSBs are "00")?

Also, given that this field may appear on every Interest in a LAN using self-learning, should we move the field into the 1-octet TLV-TYPE encoding range so that it takes up less space in the LpPacket?

Junxiao Shi wrote:

What if the field is missing from a packet - is the packet treated as discovery or non-discovery in that case?

It should be treated as "discovery", so that a consumer application isn't required to set this field.

If a FIB entry exists, it will be treated as "non-discovery"; otherwise, it will be treated as "discovery". I assume the self-learning forwarding strategy can add a NDNLPv2 tag. Does it make sense?

Also, given that this field may appear on every Interest in a LAN using self-learning, should we move the field into the 1-octet TLV-TYPE encoding range so that it takes up less space in the LpPacket?

No, because this field should be ignorable.

I was thinking the best use case of self-learning is some kind application discovery mechanism. E.g., apply SL for the prefix /discover, once the path is established, there is no need to use SL, so we could save this field; the assumption is that the LAN is stable such as a department network. For a more flexible network, SL may always be used.

Why does a strategy need to know whether it's a "discovery" interest or not? Can't it act based on the presence or absence of a (possibly strategy-private) FIB entry that matches the interest name?

Junxiao Shi wrote:

What if the field is missing from a packet - is the packet treated as discovery or non-discovery in that case?

It should be treated as "discovery", so that a consumer application isn't required to set this field.

This should be stated in the protocol description above and the NDNLPv2 design document.

Also, given that this field may appear on every Interest in a LAN using self-learning, should we move the field into the 1-octet TLV-TYPE encoding range so that it takes up less space in the LpPacket?

No, because this field should be ignorable.

Makes sense, no argument.

Davide Pesavento wrote:

Why does a strategy need to know whether it's a "discovery" interest or not? Can't it act based on the presence or absence of a (possibly strategy-private) FIB entry that matches the interest name?

That's a very good question. I think the answer is covered by 4305#note-8. In brief, a strategy treats "discovery" and "non-discovery" Interests differently when sending data back.

A "discovery" Interest retrieves data with a prefix announcement, but a "non-discovery" Interest retrieves data without a prefix announcement. This design is to reduce overhead of carrying unnecessary prefix announcements. Then e.g., node A sends a "discovery" Interest to node B, but node B already has a FIB state for the Interest, so node B sends a "non-discovery" Interest out. A "non-discovery" Interest brings back a Data packet without a prefix announcement, but node B needs to attach a prefix announcement to the Data since node A has no FIB state (recognized from the "discovery" Interest).

Teng Liang wrote:

A "discovery" Interest retrieves data with a prefix announcement, but a "non-discovery" Interest retrieves data without a prefix announcement. This design is to reduce overhead of carrying unnecessary prefix announcements.

OK

Discovery ::= DISCOVERY-TYPE TLV-LENGTH nonNegativeInteger

If the value=0, it is a non-discovery Interest. If the value=1, it is a discovery Interest.

Given that there are only 2 possible states (either discovery or non-discovery), do we really need to have a value for this TLV? Are zero-length TLVs supported?

Davide Pesavento wrote:

Discovery ::= DISCOVERY-TYPE TLV-LENGTH nonNegativeInteger

If the value=0, it is a non-discovery Interest. If the value=1, it is a discovery Interest.

Given that there are only 2 possible states (either discovery or non-discovery), do we really need to have a value for this TLV? Are zero-length TLVs supported?

I don't understand zero-length TLVs, though I have read the TLV encoding doc.

Another idea: how about defining a type for Interest (i.e., InterestType) as a TLV-Type? Then "discovery" and "non-discovery" are just two values of InterestType. The benefit is that future designs that need new Interest types can easily extend this set.

Davide Pesavento wrote:

Given that there are only 2 possible states (either discovery or non-discovery), do we really need to have a value for this TLV? Are zero-length TLVs supported?

In general, I agree that this would be a better option, unless we plan to add further self-learning flags (which I don't believe we will in the foreseeable future). However, supporting zero-length fields will require some additional changes to the code, since it is currently only engineered to support fields with a value. Perhaps we could add an EncodeHelper and DecodeHelper for an optional bool or int that would simply encode the TLV with an empty value if the optional value were not set?

Teng Liang wrote:

Another idea: how about defining a type for Interest (i.e., InterestType) as a TLV-Type? Then "discovery" and "non-discovery" are just two values of InterestType. The benefit is that future designs that need new Interest types can easily extend this set.

Are you talking about adding a new element nested inside the existing Interest TLV, or adding a new type of packet identical to an Interest except for a different TLV-TYPE?

In either case, I suppose changing the NDN packet format requires wider discussion... we've been using NDNLP header fields to experiment with new features, so that seems to be the way to go for now.

Davide Pesavento wrote:

Teng Liang wrote:

Another idea: how about defining a type for Interest (i.e., InterestType) as a TLV-Type? Then "discovery" and "non-discovery" are just two values of InterestType. The benefit is that future designs that need new Interest types can easily extend this set.

Are you talking about adding a new element nested inside the existing Interest TLV, or adding a new type of packet identical to an Interest except for a different TLV-TYPE?

In either case, I suppose changing the NDN packet format requires wider discussion... we've been using NDNLP header fields to experiment with new features, so that seems to be the way to go for now.

As Eric mentioned in the note-12, we don't support zero-length fields and it is currently only engineered to support fields with a value. Thus to better use this value (not only two of them), we could name the "discovery" field as "InterestType" field, so that a new forwarding strategy can use it with a new value. It is just a LpHeaderField.

We can change the header name from

Header definition:

LpHeaderField ::= .. | Discovery
Discovery ::= DISCOVERY-TYPE TLV-LENGTH
                     nonNegativeInteger

to

LpHeaderField ::= .. | InterestType
InterestType ::= INTERESTTYPE-TYPE TLV-LENGTH
                         nonNegativeInteger

We can still take the following design:

If the value=0, it is a non-discovery Interest. If the value=1, it is a discovery Interest.

TLV-TYPE Number Assignments:

Type: Disocvery
number(decimal):844
number(hex):0x034C

I see no issue with adding an InterestType field whose contents have a different meaning depending upon the strategy in use. However, would any other strategies actually make use of it?

I would be semi ok adding Discovery field, but not ok with "interest type". Davide, we don't have issues with zero-length TLVs. We have them as part of several definitions and I'm not sure what you're referring to.

Alex Afanasyev wrote:

I would be semi ok adding Discovery field, but not ok with "interest type". Davide, we don't have issues with zero-length TLVs. We have them as part of several definitions and I'm not sure what you're referring to.

Zero-length TLVs are currently not implemented in NDNLPv2 (to my knowledge). However, I don't think they would be too difficult to implement.

Eric Newberry wrote:

Zero-length TLVs are currently not implemented in NDNLPv2 (to my knowledge). However, I don't think they would be too difficult to implement.

So let's do it.

A zero-length field would naturally be declared as typedef of FieldDecl<..., void, tlv::Whatever>, where void becomes the ValueType. Then, it seems we need to specialize DecodeHelper and EncodeHelper for the new type (void). The complication here is that a zero-length TLV doesn't have a value, but FieldDecl::encode() and friends expect to be able to pass a value. Similarly, functions such as Packet::set<>() and get<>() don't make sense for void, but in that case we don't really have to do anything, because if someone tries to instantiate those templates with void they would get a compile error. Packet::has<>() and count<>() should work as-is. Packet::add<>() is problematic though, I think we need to add an explicit specialization for void. There could be other obstacles I haven't noticed. Overall, not so straightforward.

Another possibility is using bool as ValueType instead of void. The encode function for bool would simply skip the field if it's set to false. Everything should work as-is. The downside is that the user-facing API becomes a bit weird to use, e.g. what's the semantics of Packet::has<>() vs. Packet::get<>()?

Davide Pesavento wrote:

Another possibility is using bool as ValueType instead of void. The encode function for bool would simply skip the field if it's set to false. Everything should work as-is. The downside is that the user-facing API becomes a bit weird to use, e.g. what's the semantics of Packet::has<>() vs. Packet::get<>()?

I would prefer the bool approach. Then Packet::has<>() always returns true for this value, and Packet::get<>() returns the its value (True when exists and false when omitted).

Teng Liang wrote:

Then Packet::has<>() always returns true for this value, and Packet::get<>() returns the its value (True when exists and false when omitted).

Interesting approach. But I'm afraid it requires special-casing has(), count(), and get() for bool.

How about using only has() for zero-length fields? The generic implementation already does what we need (true if the field is there, false otherwise). get() would be useless (always returns true), but who cares?

Davide Pesavento wrote:

How about using only has() for zero-length fields? The generic implementation already does what we need (true if the field is there, false otherwise). get() would be useless (always returns true), but who cares?

Sounds good to me. I will submit a patch on this.

Though I am not sure if get() would always return true; if no target TlvType is found, will it throw an exception?

How about std::nullptr_t? This ensures only has() and add(nullptr) can be used, while avoiding the non-sense with void.

Teng Liang wrote:

Though I am not sure if get() would always return true; if no target TlvType is found, will it throw an exception?

get() will throw an exception if the field is not found. Therefore, we shouldn't use it for zero-length fields.

Based on our discussion, for zero-length Tlvs, I think two basic operations are needed: get and set the value. To get the value, The current has() is able to do that (i.e., if the field is present, return true; otherwise false). To set the value, it seems trickier (some specializations are needed) and I am not sure if set() or add() is a better place to be specialized.

Junxiao Shi wrote:

How about std::nullptr_t? This ensures only has() and add(nullptr) can be used, while avoiding the non-sense with void.

How to do add(nullptr)? To me, zero-length Tlvs face the same issues, no matter it is nullptr, void or bool.

Teng Liang wrote:

Though I am not sure if get() would always return true; if no target TlvType is found, will it throw an exception?

get() doxygen says "\throw std::out_of_range if index>=count()", so throwing when the field isn't present is correct (does not violate the contract).

Junxiao Shi wrote:

How about std::nullptr_t? This ensures only has() and add(nullptr) can be used, while avoiding the non-sense with void.

I don't like this. It just feels weird to have a pointer as argument, it's unintuitive... Moreover, you'd still need some code changes because you can't have a const reference to nullptr.

Teng Liang wrote:

Based on our discussion, for zero-length Tlvs, I think two basic operations are needed: get and set the value. To get the value, The current has() is able to do that (i.e., if the field is present, return true; otherwise false). To set the value, it seems trickier (some specializations are needed) and I am not sure if set() or add() is a better place to be specialized.

To set the value, it would be better to use set(), so that we just need to modify remove() for the bool value type.

After briefly discussing with Junxiao, I think the best solution for ValueType is to use a dedicated empty type that can only have one value. This is ultimately what nullptr_t achieves, but without the "pointer concept" that I think is slightly ugly. We can add a new type, e.g. tlv::EmptyValue, or use nullopt_t.

Btw this issue should be moved from NFD to ndn-cxx project.

Davide Pesavento wrote:

After briefly discussing with Junxiao, I think the best solution for ValueType is to use a dedicated empty type that can only have one value. This is ultimately what nullptr_t achieves, but without the "pointer concept" that I think is slightly ugly. We can add a new type, e.g. tlv::EmptyValue, or use nullopt_t.

The sematics of tlv::EmptyValue, or use nullopt_t is the same to the current implementation (bool), right? So I just need to change the value type. Does Junxiao agree with tlv::EmptyValue? I am ok with either one.

Btw this issue should be moved from NFD to ndn-cxx project.

Is there a way to port the whole issue with notes to ndn-cxx project, or do I simply create another one in ndn-cxx project?

this issue should be moved from NFD to ndn-cxx project.

No. Every field addition involves:

1. Define the field in NDNLPv2 page.
2. Declare the field in ndn-cxx.
3. Encode and decode the field in ndn::Face if needed.
4. Encode and decode the field in a LinkService.

At least half of the steps are in NFD.

Junxiao Shi wrote:

this issue should be moved from NFD to ndn-cxx project.

No. Every field addition involves:

1. Define the field in NDNLPv2 page.
2. Declare the field in ndn-cxx.
3. Encode and decode the field in ndn::Face if needed.
4. Encode and decode the field in a LinkService.

At least half of the steps are in NFD.

Then Status shouldn't be "code review" if additional steps are needed.

Junxiao Shi wrote:

this issue should be moved from NFD to ndn-cxx project.

No. Every field addition involves:

1. Define the field in NDNLPv2 page.
2. Declare the field in ndn-cxx.
3. Encode and decode the field in ndn::Face if needed.
4. Encode and decode the field in a LinkService.

Once we reach an agreement, I can finish the first two steps. Which option do you prefer, "tlv::EmptyValue" or "nullopt_t"? (note-28)

Davide Pesavento wrote:

Then Status shouldn't be "code review" if additional steps are needed.

I changed the status because I pushed a commit to gerrit. The current status is not code review, but it only has two options now 'code review' or 'closed'. There is not much I can do now.

In NDNLPv2 rev18, it is unclear what "discovery purpose" means.
Also, how to distinguish between "Interest is non-discovery" and "Discovery flag is unset"?

In self-learning publication and its initial implementation, an Interest with unset Discovery flag is implicitly an discovery Interest. This allows maximum compatible because not every application is aware of self-learning. If a discovery Interest (either tagged as such or untagged) hits a FIB entry, the strategy would retag it as non-discovery. In this sense, you could define a NonDiscovery header field to make an Interest non-discovery, and all Interests without this field are discovery Interests.

Junxiao Shi wrote:

In NDNLPv2 rev18, it is unclear what "discovery purpose" means.
Also, how to distinguish between "Interest is non-discovery" and "Discovery flag is unset"?

In self-learning publication and its initial implementation, an Interest with unset Discovery flag is implicitly an discovery Interest. This allows maximum compatible because not every application is aware of self-learning. If a discovery Interest (either tagged as such or untagged) hits a FIB entry, the strategy would retag it as non-discovery. In this sense, you could define a NonDiscovery header field to make an Interest non-discovery, and all Interests without this field are discovery Interests.

Sounds good to me. I made an update to the docs accordingly.

Davide Pesavento wrote:

After briefly discussing with Junxiao, I think the best solution for ValueType is to use a dedicated empty type that can only have one value. This is ultimately what nullptr_t achieves, but without the "pointer concept" that I think is slightly ugly. We can add a new type, e.g. tlv::EmptyValue, or use nullopt_t.

nullopt_t is a member of std since C++17, but we are using C++11, so does it mean using nullopt_t has a higher requirement? For tlv::EmptyValue, is it better to assign a number reserved for NDNLP or not?

Teng Liang wrote:

Davide Pesavento wrote:

After briefly discussing with Junxiao, I think the best solution for ValueType is to use a dedicated empty type that can only have one value. This is ultimately what nullptr_t achieves, but without the "pointer concept" that I think is slightly ugly. We can add a new type, e.g. tlv::EmptyValue, or use nullopt_t.

nullopt_t is a member of std since C++17, but we are using C++11, so does it mean using nullopt_t has a higher requirement?

No, we have backported nullopt and nullopt_t in namespace ndn for C++11.

For tlv::EmptyValue, is it better to assign a number reserved for NDNLP or not?

I don't understand this question. What number?

Davide Pesavento wrote:

Teng Liang wrote:

nullopt_t is a member of std since C++17, but we are using C++11, so does it mean using nullopt_t has a higher requirement?

No, we have backported nullopt and nullopt_t in namespace ndn for C++11.

Does it mean if I replace std::nullptr_t with ndn::nullopt_t and replace nullptr with ndn::nullopt. The code can work?

For tlv::EmptyValue, is it better to assign a number reserved for NDNLP or not?

I don't understand this question. What number?

If we use tlv::EmptyValue with the following defination

typedef FieldDecl<field_location_tags::Header,
                  tlv::EmptyValue,
                  tlv::NonDiscovery> NonDiscoveryField;

The ValueType is tlv::EmptyValue, should we assign a TLV-Type number to it and declare it somewhere?

Teng Liang wrote:

Does it mean if I replace std::nullptr_t with ndn::nullopt_t and replace nullptr with ndn::nullopt. The code can work?

Yes.

The ValueType is tlv::EmptyValue, should we assign a TLV-Type number to it and declare it somewhere?

No. EmptyValue is not a TLV-TYPE. It's simply an implementation detail, a type to represent zero-length TLVs.

Davide Pesavento wrote:

Teng Liang wrote:

Does it mean if I replace std::nullptr_t with ndn::nullopt_t and replace nullptr with ndn::nullopt. The code can work?

Yes.

I just tried it, but got this error message. Did I mess up anything?

usr/include/boost/concept_check.hpp:139:10: error: no matching function for call to ‘ndn::nullopt_t::nullopt_t()’
       TT a;               // require default constructor

The ValueType is tlv::EmptyValue, should we assign a TLV-Type number to it and declare it somewhere?

No. EmptyValue is not a TLV-TYPE. It's simply an implementation detail, a type to represent zero-length TLVs.

What is its instance value? E.g., for struct DecodeHelper<TlvType, tlv::EmptyValue>, what is the return value?

Teng Liang wrote:

Davide Pesavento wrote:

Teng Liang wrote:

Does it mean if I replace std::nullptr_t with ndn::nullopt_t and replace nullptr with ndn::nullopt. The code can work?

Yes.

I just tried it, but got this error message. Did I mess up anything?

usr/include/boost/concept_check.hpp:139:10: error: no matching function for call to ‘ndn::nullopt_t::nullopt_t()’
       TT a;               // require default constructor

Apparently we require that the encoded type is default constructible, but nullopt_t cannot be, per C++ standard. So we cannot use nullopt_t.

The ValueType is tlv::EmptyValue, should we assign a TLV-Type number to it and declare it somewhere?

No. EmptyValue is not a TLV-TYPE. It's simply an implementation detail, a type to represent zero-length TLVs.

What is its instance value? E.g., for struct DecodeHelper<TlvType, tlv::EmptyValue>, what is the return value?

It doesn't really matter... you can call the type EmptyValueType and then declare

constexpr EmptyValueType EmptyValue;

as the (only) value. Or simply use EmptyValueType{} when you need the value. I guess the former is slightly more user friendly.

Davide Pesavento wrote:

The ValueType is tlv::EmptyValue, should we assign a TLV-Type number to it and declare it somewhere?

No. EmptyValue is not a TLV-TYPE. It's simply an implementation detail, a type to represent zero-length TLVs.

What is its instance value? E.g., for struct DecodeHelper<TlvType, tlv::EmptyValue>, what is the return value?

It doesn't really matter... you can call the type EmptyValueType and then declare

constexpr EmptyValueType EmptyValue;

as the (only) value. Or simply use EmptyValueType{} when you need the value. I guess the former is slightly more user friendly.

I guess I should declare EmptyValueType first. Like defining a struct for EmptyValueType in tlv.hpp?

Teng Liang wrote:

I guess I should declare EmptyValueType first. Like defining a struct for EmptyValueType in tlv.hpp?

Obviously. I'm not sure if I'd use tlv.hpp though, as that file contains the TLV-TYPE declarations only. lp::Sequence is the same "kind" of type and is declared in a header on its own, lp/sequence.hpp.

Junxiao Shi wrote:

1. Define the field in NDNLPv2 page.
2. Declare the field in ndn-cxx.
3. Encode and decode the field in ndn::Face if needed.
4. Encode and decode the field in a LinkService.

I am implementing the related encoding and decoding in the generic link service (I don't think any coding is needed in "ndn::Face"). Should we implement an option to enable/disable the self-learning forwarding support? According to the NDNLPv2, we should, but I don't see a reason to disable the support (so why should we have the option).

I don't think any coding is needed in "ndn::Face"

Correct. You can assume all Interests from the consumer is discovery, and the producer can safely ignore discovery vs non-discovery.

I am implementing the related encoding and decoding in the generic link service. Should we implement an option to enable/disable the self-learning forwarding support?

No. The protocol is written such that an implementation can have such an option, but GenericLinkService does not need this option.