NFD » ndn-cxx

To make sure I'm understanding this correctly, does this issue include building the Parameters class and its encoding? Based on the current wireEncode, it looks like I would need the Parameters implementation before I can check for the Parameters element and encode it. If so, is there any more information about what type of information can/should be contained in Parameters?

does this issue include building the Parameters class and its encoding?

No. There would be no Parameters class. It's a Block same as Data packet Content element.

Apologies for a late response on this, but the API has to include updating of the name with a hash of the parameters (to ensure that the name is unique). The API can stay the same, just action for setParameters should include updating of the name; not quite sure what to do with unsetParameters.

In the NFD call that greenlit the current design (as implemented by Arthi), the consensus was that it's the application's responsibility to ensure that the name is unique. I don't have a strong opinion either way, but:

1. with Data packets and content, the uniqueness of the name is left to the application
2. hashing the parameters means agreeing on a hash function
3. what if the application knows better and wants to deal with name uniqueness on its own?
4. as you said, what component do we remove on unsetParameters?

If we go this way, my suggestion is to use a typed name component for the Parameters hash.

At the very least, we need to have a helper and a quick "standard" way to add such a component. Ideally, as part of this commit. I still have a tiny preference to force this inside setParameters, but can also be flexible.

Sure, maybe we can discuss at Monday's NFD call? And can we do this in a separate commit? Parameters aren't currently being used, though I can add a Redmine issue/comment if we're concerned about people trying to use it immediately

I disagree with setParameters automatically modifying Name. It would be surprising for a setter to change another field.

I'm fine with creating a helper function appendParameterDigestToName, but this requires defining a naming convention first, and should belong to a separate issue.

It could be even more surpising to have two interests with the same name be aggregated or retrieve wrong data from caches (just because there is no suggestion/enforcement for name uniqueness). In any case, we shall proceed with the helper function (including the definition of the convention) ASAP, before we start using the parameters.

Alex Afanasyev wrote:

In any case, we shall proceed with the helper function (including the definition of the convention) ASAP, before we start using the parameters.

Fair enough.

Logically, it should have its own name (the number is a possible URI representation). I was trying to add, but couldn't come up with a good name.

sha256params= ?

sha256params=

Yes this is fine.

To all: I'll take care of adding the name component type to ndn-cxx under #4570.
After that, Arthi can write the helper function for creating InterestParametersSha256DigestComponent from Parameters.

Yes, I'm still planning to but can only start next week

Davide Pesavento wrote:

This seems to have stalled. Is anyone still working on this? If not, we need to reassign again.

I'm still working on this. I will commit a new version soon.

I was wondering why Interest::getParameters() returns a Block (which includes the Parameters T and L)... why should an application care about how the element is encoded? This effectively means that the vast majority of apps will have to manually extract the TLV value of the returned Block, which leads to more boilerplate in the app's code. It's also inconsistent with the other getters... we don't include the T and L in the return value of getInterestLifetime or getNonce, for example.

My suggestion is to return a Buffer or ConstBufferPtr from getParameters().

Isn’t Data::getContent returning Block too?
A major benefit of Block is that its API allows easy decoding into a sequence of sub elements.

Junxiao Shi wrote:

Isn’t Data::getContent returning Block too?

Yes, I don't like that either, but it's too late to change it.

A major benefit of Block is that its API allows easy decoding into a sequence of sub elements.

It's only a "benefit" if you assume that apps will use a Block-based encoding (i.e. NDN-TLV) for their parameters. Still, returning a ConstBufferPtr from getParameters doesn't prevent this use case, since you can efficiently construct a Block from a ConstBufferPtr without copying the underlying buffer.

To ensure applications do not forget to check ParametersSha256DigestComponent, Interest::wireDecode should perform the check automatically.
To avoid slowing down the forwarder, this check can be turned off via Interest::setAutoCheckParametersDigest(false).
When an application leaves it at default, Interest::wireDecode throws on bad ParametersSha256DigestComponent.
Forwarder can turn it off.

On gerrit, Alex says:

I'm still unease that we modify name during encoding (which is const operation). No objection, but this is the root cause of this question.

Yeah I don't like that either, so I'm all ears if you have better suggestions for the API. I can think of two alternatives:

1. In wireEncode, only check that the digest component is present (and unique) if the Interest carries parameters, and that it's absent if the Interest does not carry parameters, throw an exception otherwise. Require the user to call a new updateParametersSha256Digest function at the right time (after adding/changing/removing parameters but prior to encoding/sending). One shortcoming of this alternative is that the checks in wireEncode won't detect mistakes such as modifying the parameters after calling updateParametersSha256Digest.

2. Automatically update (or add/remove) the digest component from setApplicationParameters and unsetApplicationParameters. Possibly add a boolean flag to these functions to disable the automatic update of the digest (will be useful when we implement the new signed Interest format). No extra work to do in wireEncode, or we could do the same checks as in alternative (1) just to be on the safe side. One drawback of this alternative is that it's not very future-proof if we add more Interest elements covered by the ParametersSha256Digest (i.e. elements after ApplicationParameters).

I'm still unease that we modify name during encoding

No, Interest type should not modify name during encoding. Instead:

1. In constructor or setName:

   • If the Interest is expected to contain parameter elements, the name should contain exactly one ParametersSha256DigestComponent. Even if the Interest does not contain parameter elements at the moment, this operation does not throw.
   • If the Interest is expected to not contain parameter elements, the name should contain zero ParametersSha256DigestComponent. Even if the Interest contains parameter elements at the moment, this operation does not throw.
   • This operation throws if the name contains more than one ParametersSha256DigestComponent.

2. In getName:

   • If the Interest contains parameter elements, and the name contains a ParametersSha256DigestComponent, the return value should reflect an updated digest.
   • If the Interest does not contain parameter elements, and the name does not contain ParametersSha256DigestComponent, the return value does not contain a digest.
   • In all other cases, the return value is the current name unchanged.

3. In wireEncode:

   • If the Interest contains parameter elements, and the name contains a ParametersSha256DigestComponent, the encoding result should reflect an updated digest.
   • If the Interest does not contain parameter elements, and the name does not contain ParametersSha256DigestComponent, the encoding result does not contain a digest.
   • In all other cases, this operation throws because ParametersSha256DigestComponent mismatches parameter elements.

Effectively,

• setName and any operation that adds or removes parameter elements can recompute the digest, iff the presence of ParametersSha256DigestComponent matches the presence of parameter elements.
• wireEncode fails if the presence of ParametersSha256DigestComponent mismatches the presence of parameter elements.

Notably, there's no automatic insertion or deletion of ParametersSha256DigestComponent in Name, because application should maintain control over the name structure.
The Interest type can update the digest in an existing ParametersSha256DigestComponent, but cannot insert or delete a ParametersSha256DigestComponent.

At callsite, Interest type is used as follows:

Interest iA("/A");
iA.setCanBePrefix(false);
iA.setMustBeFresh(false);
iA.wireEncode();

Interest iB("/B/params-sha256=0000000000000000000000000000000000000000000000000000000000000000");
iB.setCanBePrefix(false);
iB.setMustBeFresh(false);
iB.wireEncode(); // throws
iB.setApplicationParameters("240100"_block);
iB.getName(); // reflects updated digest
iB.wireEncode(); // reflects updated digest

Internally, Interest type may employ lazy computation to reduce overhead when multiple operations that adds or removes parameter elements are invoked in sequence. This could mean modifying name in getName and wireEncode functions, but the semantic remains the same: setName and any operation that adds or removes parameter elements can recompute the digest.

Junxiao Shi wrote:

Notably, there's no automatic insertion or deletion of ParametersSha256DigestComponent in Name, because application should maintain control over the name structure.
The Interest type can update the digest in an existing ParametersSha256DigestComponent, but cannot insert or delete a ParametersSha256DigestComponent.

Not sure if I agree with the above.

Internally, Interest type may employ lazy computation to reduce overhead when multiple operations that adds or removes parameter elements are invoked in sequence. This could mean modifying name in getName and wireEncode functions, but the semantic remains the same: setName and any operation that adds or removes parameter elements can recompute the digest.

This is exactly what Alex doesn't like (unless I misunderstood his comment). And you're contradicting yourself by first agreeing with him ("should not modify name during encoding") and later saying that wireEncode can modify the name...

you're contradicting yourself by first agreeing with him ("should not modify name during encoding") and later saying that wireEncode can modify the name...

wireEncode cannot modify the name in an externally visible way. Every name modification appears as if it's performed by setName or a method that affects an element covered by the digest, not wireEncode.
What wireEncode does internally is an implementation detail.

Let me suggest a combination approach, based on what Junxiao suggested, but without modifications within const functions:

• In constructor or setName (same as in comment #35). With the exception that we make a requirement/assumption that not having parameters implies params-sha256=0000000000000000000000000000000000000000000000000000000000000000.

• getName does not do any modification of the name ever

• wireEncode does not do any modification of the name ever

• Any method that sets or updates parameters (AppParameters or any field afterward) triggers updating of the digest name component. We may do it multiple times, but I prefer paying this price compared to modifying memory in const methods. These methods should do throwing similar to what described in comment #35)

With the exception that we make a requirement/assumption that not having parameters implies params-sha256=0000000000000000000000000000000000000000000000000000000000000000

I don't understand this sentence. Do you mean that if there are parameters (or will be added later), the user can set a name without params-sha256= component and we automatically append one? (that would be my preference I think)

We may do it multiple times, but I prefer paying this price compared to modifying memory in const methods.

Yes, this is the intrinsic tradeoff of eager vs lazy computation of the digest. I have no preference either way. With the current Interest format, recalculation would only happen in rare or contrived cases, so I'm not worried. I will adopt your approach.

Davide Pesavento wrote:

With the exception that we make a requirement/assumption that not having parameters implies params-sha256=0000000000000000000000000000000000000000000000000000000000000000

I don't understand this sentence. Do you mean that if there are parameters (or will be added later), the user can set a name without params-sha256= component and we automatically append one? (that would be my preference I think)

No. I meant that if app plans parameters, it must add a placeholder params component (with params-sha256=0000... value).

Oh. I kinda disagree with that actually. It seems slightly simpler, from the app point of view, to have the Interest class append a ParametersSha256DigestComponent to the name as needed. Of course if the app protocol requires the digest component to be in a specific position within the name, the app can put a placeholder component at the required position. Same for removing the component automatically in unsetApplicationParameters(). Otherwise the app has to do the following, which is unnecessarily verbose and repetitive:

interest.unsetApplicationParameters();
auto name = interest.getName();
// find and erase the ParametersSha256DigestComponent from name (we don't even have the API for this in Name)
interest.setName(name);

Moreover, an all-zero SHA256 hash value is not special, it's just one of many possible outputs. So it would be quite ugly to designate params-sha256=00000... as a special placeholder component.

Davide Pesavento wrote:

Of course if the app protocol requires the digest component to be in a specific position within the name, the app can put a placeholder component at the required position.

And it would be good to have a nicer API for this, instead of writing long and ugly names with .../params-sha256=... in the code. But this is minor.