NFD

What's the application scenario, for each proposed feature?

Monitoring/measurement/experimentation.

I am also interested in this feature. I think the controller should have this feature and it can help with experimentation as well. NDN-CC should also provide this feature as well.

My main concern is on the scalability of the enumeration operation as part of the Management protocol.
On the NDN testbed, each router has 65536-entry CS. Suppose each record is 50 octets, and each Data packet in the dataset carries 8000 octets of payload, a full enumerate would produce 409 Data packets. Creating and signing 409 Data packets takes more than 1 second (according to #1589-1 benchmark on v1 KeyChain), which means NFD cannot process any new packets for more than 1 second, which is unacceptable.
Management thread (#3565) can eliminate the signing overhead, but it still requires the CS to be locked for some time in order to copy the packet names.

Monitoring/measurement/experimentation.

These purposes can be achieved with ndnSIM, where the scenario code has direct access to the nfd::Cs object. We just need to add methods to flush and enable/disable the CS.

Junxiao Shi wrote:

My main concern is on the scalability of the enumeration operation as part of the Management protocol.

Yes, that is my concern as well. However:

1. The management protocol should not be constrained by current implementation limitations or inefficiencies, a future version of NFD may remove these limitations or a different forwarder may not have them in the first place.
2. Enumeration can be a privileged operation.
3. I'm not sure we actually need full CS enumeration. In fact, it would be enough as a first step to have a "query" mgmt interface that given a name + selectors returns any matching entry, basically wrapping Cs::find().
4. Additionally, in the case of filtered enumeration, the CS manager can return only the first N matching entries, where N is a configuration parameter.

Monitoring/measurement/experimentation.

These purposes can be achieved with ndnSIM

I'm not interested in simulations.

it would be enough as a first step to have a "query" mgmt interface that given a name + selectors returns any matching entry, basically wrapping Cs::find().

You already have it (untested snippet):

auto interest = make_shared<Interest>("/A");
interest->setTag(make_shared<lp::NextHopFaceId>(254));
face.expressInterest(*interest, nullptr, nullptr, nullptr);

This Interest can only be forwarded to the special "content store" face, which means it will retrieve any Data present in the CS, but will be Nacked (with standards-conforming forwarding plane implementation) if the Data is missing.

Junxiao Shi wrote:

interest->setTag(make_shared<lp::NextHopFaceId>(254));

My understanding is that: (1) there's no guarantee that NextHopFaceId will be respected, and (2) everything concerning FaceId 254 is an internal implementation detail and must not be relied upon.

Another question is whether we need a new "cs" privilege for this manager or not.

Also, can we add the "max number of entries" to the GeneralStatus dataset, or do we need to create a separate CsStatus dataset for just one number?

can we add the "max number of entries" to the GeneralStatus dataset, or do we need to create a separate CsStatus dataset for just one number?

This can belong to status/general, if there's no plan for more fields.

The only 2 things I can think of right now are:

1. Obtain the current state of the "cache" and "serve" flags (the "get" counterpart of the "set" described above).
2. Get statistics on the CS, e.g. hit/miss count. However, I would expect monitoring software to poll these counters much more frequently than other relatively static values, so keeping them separate might not be a bad idea.

Finally, status/general already provides NCsEntries, so it seems natural to add a MaxCsEntries there.

Draft of the specification:
https://gist.github.com/pchan1/29d603fb490ac3fc706ce3caba6eafeb

Suggestions are welcomed

I've reviewed design as of revision 797a7998bac6dc65ddeb9eb1c4bde9bd7c76f372.

• I do not understand this sentence:

  This command does not have a hard limit and does not account for limits that exceed memory space

• What's a use case for a CS with "cache" enabled but "serve" disabled?

• What's a use case for a CS with "serve" enabled but "cache" disabled?

• Why are set-limit and set-flags commands needed?
  Similar functionality is already supported as (1) modify nfd.conf (2) send SIGHUP to trigger config reload.
  What's a use case that cannot be supported through existing config reload mechanism?
  Hint: a valid use case should require runtime changes of each setting, and the justification needs to state why modifying nfd.conf is impractical.

• Assuming there is a compelling use case that requires commands, why not defining a single command that controls all attributes of the CS including capacity limit, serve/cache flags, and eviction policy?

• Should flush command support flushing only Data packets under a certain prefix?
  If so, erase is a better name.

• list dataset could be huge.
  Suppose the CS is at default capacity of 65536 packets and is completely full, and the average entry length is 32 octets, the dataset would have 263 segments each with 8000 octets of payload.
  Generating and signing so many Data packets would cause a 664ms delay (according to #1589-1 benchmark), which is unacceptable.
  However, this is not a protocol problem.

Junxiao Shi wrote:

• I do not understand this sentence:

  This command does not have a hard limit and does not account for limits that exceed memory space

It means that we make no attempt to limit the CS limit based on currently available RAM or other metrics. So if you ask for a 50GB limit on a 1GB machine, you'll probably trigger an OOM condition.

• What's a use case for a CS with "cache" enabled but "serve" disabled?

For testing/experimentation, it can simplify or accelerate cache warm-up in some wireless scenarios. Also for demonstration purposes (I saw a CICN demo where they turned off "serving data from the CS").

• What's a use case for a CS with "serve" enabled but "cache" disabled?

We don't really have a strong use case in mind at the moment.
However, if the cache-on serve-off use case above is approved, then we still need at least 2 bits to represent all 3 possible states, therefore adding the cache-off serve-on combination comes basically for free.

• Why are set-limit and set-flags commands needed? Similar functionality is already supported as (1) modify nfd.conf (2) send SIGHUP to trigger config reload. What's a use case that cannot be supported through existing config reload mechanism? Hint: a valid use case should require runtime changes of each setting, and the justification needs to state why modifying nfd.conf is impractical.

Changing nfd.conf requires root privileges.
Also, I could ask the same thing about StrategyChoice management...

• Assuming there is a compelling use case that requires commands, why not defining a single command that controls all attributes of the CS including capacity limit, serve/cache flags, and eviction policy?

What's your idea, specifically? We're open to suggestions on this.

• Should flush command support flushing only Data packets under a certain prefix? If so, erase is a better name.

Maybe, but we didn't have a use case. However, I agree that the protocol can use "erase" as command verb, with one parameter (the prefix), and it doesn't necessarily have to match nfdc command line.

• list dataset could be huge. Suppose the CS is at default capacity of 65536 packets and is completely full, and the average entry length is 32 octets, the dataset would have 263 segments each with 8000 octets of payload. Generating and signing so many Data packets would cause a 664ms delay (according to #1589-1 benchmark), which is unacceptable. However, this is not a protocol problem.

We already discussed about this (see above). We'll probably make this a "query" operation rather than "list".
For "query", the question is whether it finds only a single match or multiple matches. In the latter case, one can probably construct a query that matches every packet in the CS, so the scaling problem is still there. However, a CS Manager implementation may decide to truncate the response if it's larger than a predefined threshold.

Changing nfd.conf requires root privileges.

This reason is invalid. The sysadmin can create a UNIX group as the owner of /etc/ndn/nfd.conf, and add users who are authorized to modify this file to the group.

Also, I could ask the same thing about StrategyChoice management...

Initially NFD management was designed to rely entirely on nfdc commands. table.strategy_choice config option was added later.

why not defining a single command that controls all attributes of the CS including capacity limit, serve/cache flags, and eviction policy?

What's your idea, specifically? We're open to suggestions on this.

I'd prefer a single set-config command for all these settings, because they can be changed together.
In particular, changing eviction policy requires clearing the entire CS. If both eviction policy and capacity limit are to be changed, it's more efficient to do them together. Otherwise, assuming there is a clear method on the existing eviction policy that is faster than repeatedly calling evict, in case the caller lowers capacity limit and then changes eviction policy, there is useless work during the first command for evicting excess entries first, given the second command would clear the CS anyways.

Symmetrically, there should be a config StatusDataset that reveals the current config.

The set of available eviction policies needs a separate cs/available-policies dataset, to support nfdc autocompletion.

I agree that the protocol can use "erase" as command verb, with one parameter (the prefix), and it doesn't necessarily have to match nfdc command line.

erase is more general and should be preferred. My use case is to clean up poisoned Data.

For "query", the question is whether it finds only a single match or multiple matches. In the latter case, one can probably construct a query that matches every packet in the CS, so the scaling problem is still there. However, a CS Manager implementation may decide to truncate the response if it's larger than a predefined threshold.

query definitely should support multiple matches. The desired number of entries should be provided as a parameter in the Interest, with an implementation-defined upper bound. Paging support is unnecessary.
Also, the Dispatcher's StatusDataset signing can be optimized: it can defer signing a segment until an Interest asking for this segment arrives. This still requires signing all the segments, but it does not block NFD for long time, and gives other faces a chance to compete for NFD forwarding processing.

Junxiao Shi wrote:

Changing nfd.conf requires root privileges.

This reason is invalid. The sysadmin can create a UNIX group as the owner of /etc/ndn/nfd.conf, and add users who are authorized to modify this file to the group.

Your solution is inferior, as it is too coarse-grained (all or nothing). With NFD mgmt, the admin can grant only the "cs" privilege to a user, disallowing all other changes.

I agree that the protocol can use "erase" as command verb, with one parameter (the prefix), and it doesn't necessarily have to match nfdc command line.

erase is more general and should be preferred. My use case is to clean up poisoned Data.

Sure. What I meant was that we can have a nfdc cs flush command (similar to ip address flush) in addition to nfdc cs erase <prefix>.

I agree with note-19.

I feel this scope of this issue is too board and thus it's going to take very long time (I'm estimating more than 6 months). I suggest splitting get/set config, enumeration, and deleting data as three issues. Then, we can start with get/set config which is the easiest.

Junxiao Shi wrote:

Protocol update for cs/config command and cs/info dataset extension are in CsMgmt rev5 and ControlCommand rev34.

LGTM.

I suggest adding NCsEntries to cs/info as well, so that we have both capacity and current # of entries in the same dataset.

I suggest adding NCsEntries to cs/info as well, so that we have both capacity and current # of entries in the same dataset.

Agreed. This is added in CsMgmt rev7.

why don't we require the client to set the mask appropriately, as in FaceUpdateCommand

Agreed.
ControlCommand rev35 and FaceMgmt rev73 and CsMgmt rev8 update the protocol. Definition of Mask field has been moved to ControlCommand page.

https://gerrit.named-data.net/4453 patchset3 implements the updated protocol.

https://gerrit.named-data.net/4468 adds Capacity and CS enablement flags to CsInfo struct.
It is a design choice to not have getFlags and to use the name getNStored instead of getNCsEntries. These function names are suitable for the semantics of this struct, instead of matching the encoding.

Junxiao Shi wrote:

It is a design choice [...] to use the name getNStored instead of getNCsEntries.

Why not getSize or getNEntries?

These function names are suitable for the semantics of this struct, instead of matching the encoding.

This sentence is completely meaningless.

Why not getSize or getNEntries?

getSize does not indicate whether the unit is “entries” or “octets”.
getNEntries is not better than getNStored, ie “number of stored entries”.

the field in the serialized/deserialized way is called NCsEntries (I thought it is the same until check). I think it should simply match that one and be called getNCsEntries / setNCsEntries

No.

• TLV-TYPE does not have a name. It’s just a number.
• The constant naming that number is indeed “NCsEntries”. This constant name is defined in the context of general status dataset. “CS” part would be duplicate in the context of CsInfo struct.
• Public API of CsInfo is designed to make sense to the caller, not to match the encoding.

Junxiao Shi wrote:

Why not getSize or getNEntries?

getSize does not indicate whether the unit is “entries” or “octets”.

Neither does getNStored.

getNEntries is not better than getNStored, ie “number of stored entries”.

getNEntries provides a more uniform naming since "entries" is used elsewhere in management with the same meaning. And see also the previous point.

the field in the serialized/deserialized way is called NCsEntries (I thought it is the same until check). I think it should simply match that one and be called getNCsEntries / setNCsEntries

No.

• TLV-TYPE does not have a name. It’s just a number.
• The constant naming that number is indeed “NCsEntries”. This constant name is defined in the context of general status dataset. “CS” part would be duplicate in the context of CsInfo struct.

True. But getNEntries doesn't contain this redundancy.

CsInfo::wireDecode has a bug: when it encounters Capacity element, the value is incorrectly assigned to m_nHits instead of m_capacity. This is fixed in https://gerrit.named-data.net/4512. This bug was not caught earlier because:

• The unit test passes the wire encoding to wireDecode, and then checks the decoded CsInfo equals the original.
• To check the decoded CsInfo equals the original, the unit test uses operator==.
• operator== does not invoke individual getters, but simply checks the encodings are equal.
• wireDecode caches the encoding, so operator== is comparing the cached encoding.
• As a result, there's no test coverage checking the correctness in decoded fields.

I have changed the test case to check the return value of each getter. To avoid the same mistake in the future, TLV struct decoding test cases should always check with individual getters.

Remaining part of this issue is adding nfdc commands. Design is below, but implementation needs to wait after https://gerrit.named-data.net/4489 to avoid conflicts.

nfdc cs info will switch to a multi-line format similar to nfdc status.

CS information:
  capacity=17274
     admit=on
     serve=off
  nEntries=4615
     nHits=8958
   nMisses=13540

There will be a new nfdc cs config command:

nfdc cs config [capacity <CAPACITY>] [admit on|off] [serve on|off]

Junxiao Shi wrote:

Remaining part of this issue is adding nfdc commands. Design is below, but implementation needs to wait after https://gerrit.named-data.net/4489 to avoid conflicts.

4489 has been merged.

I agree with the proposed syntax for config and output format for info.

Junxiao, do you think you can finish this ASAP so that we can have it in v0.6.1?