Re: [Cbor] I-D Action: draft-ietf-cbor-time-tag-04.txt

[Laurence Lundblade <[e-mail]>]

> On Jan 16, 2023, at 10:14 AM, Carsten Bormann <[e-mail]> wrote:
> 
> Hi Laurence,
> 
> Thank you for these comments.
> 
>> On 2023-01-15, at 01:16, lgl island-resort.com <[e-mail]> wrote:
>> 
>> Attached is an annotated PDF with my comments.
> 
> Let me take these items first:
> 
>> * It does a lot more than these two — time zones, clock quality.
> 
> 
> The spec itself is not concerned with timezone (the SEDATA additions may provide some additions).  But the paragraph you made this comment on was about timescales, so I’m confused.

This section is a list of objectives. Only two are listed when there while the document achieves more.  The thought was to add a few more bullet-paragraphs listing clock quality and such as objectives to make the document more consistent.


>> * Why not bignum? Bignum is much simpler to implement than bigfloat of decimal fraction. Maybe because -3, -6 and such give the equivalent of big nums?
> 
> 
> We are not aware of an application that would need a bignum value for the *seconds* that 1, 4, or 5 need.  But it would of course be possible to use a bignum value for milliseconds, picoseconds, 2^{-64} seconds, …  Keys for this can be registered later, or can be defined now.  Vadim made us aware of SQLite, which among other variants uses a milliseconds-based integer time — but here a basic CBOR int (-2^{64}..2^64-1) suffices for the next 500 Gigayears to so (the earth is scheduled to be absorbed by the sun in 7.5 Gigayears).

Understood. We don’t need bignums for the calendar app where the sun's schedule is tracked. :-)


>> * Seems like this CDDL could be much improved by listing out all map items like ClockQuality-group and providing a default rule for items not defined in this document. This would help people validating with the CDDL tool a lot.
> 
> 
> Well, when including future registrations, `any` is as specific as we can get here.  We could of course include the known registrations in an extended CDDL spec of the map.

Yes, I was thinking that CDDL for defined registrations is included and the future extensions have a .feature so you can know in CDDL validation when you are in the bounds of the specification.


>> * New time formats must be Posix-based (because the base time keys 1, 4 and 5 are TIME_T) (If others are allowed, then require definition of new base time types for them
> 
> Can you please elaborate on this concern?
> It is true that the tag so far assumes time formats and timescales mix freely; that may not be always true.  But a new (base) time format could come with its own timescales and require their use, so I’m not sure that actually is a problem.

I see.  A new base-time with key 11 or such could be defined that is not Posix/TIME_T based.

Some care is needed by implemtors to coordinate the timescale key (-1) with what base time is used in the base time. It’s counter-intuitive to me that each base-time has a default timescale that is then refined by the timescale key. Some text pointing this out would be helpful.

A more intuitive definition for me would be that the timescale of any base-time is determined solely by the timescale key, perhaps with the default timescale for all base times being TIME_T, but with some explanation the current seems workable.


>> Most are requests for clarifications and improvements. Here’s some, but not all:
>>  - Clearly state this is only for times with 1970 as origin — Posix-based time
> 
> “This”?  The two timescales registered so far have this property, but this is not intrinsic to the time tag.  (Of course, it would be nice if future timescales were to have this property, but that may be more complicated to achieve than it's worth.)

“This” is the draft-ietf-cbor-time-tag, I didn’t understand at first reading that base times could be other time than Posix.


>>  - Request for more examples
> 
> Examples are always good, but too many examples can bloat a specification.
> Do you have examples [sic] where an example might help?

One or two examples for section 3.3 is the top of my list. Would help to understand intent much more quickly.


>> Here’s one show stopper for me. I think the intent is that a decoder should error out if it encounters an item with an unsigned key that it can’t decode. For example a decoder that only supports base time “1” should be allowed and it should error out if it encounters a big float base time as input. But I think as written the text kind of says that all decoders must implement all base times and critical items.
> 
> Where does it say this?
> Nothing is mandatory to implement.
> (You need to have at least one base-time per time-tag.
> We might want to add a tag that identifies just additional keys such as clock quality; this needs to be a different tag as it is not by itself a timestamp.)

This sentence 
    Not understanding key/value for unsigned integer keys is an error
says to me that all decoders MUST implement all unsigned integer keys.

> Maybe clarifying this some more:
> https://github.com/cbor-wg/time-tag/pull/13

This helps and now I know what is intended, but I think it could be phrased a little more clearly. I think I would say somewhere that the only requirement of an implementation is that it support at least one base time. Then I’d say that decoders that encounters a base time type that they have not implemented should return a fatal error and cease decoding.



>> Here’s the other one (which you’ve heard from me before). I dislike the requirement to implement floating-point a lot. I would like to see an integer-only base time that doesn’t require floating-point. Perhaps key 1 is integer only and key 7 is float only. Or just say float is optional with key “1”.  This does break some compatibility with CBOR tag 1, but I still think this change should be made. 
> 
> It is evident whether your key 1 carries a floating point value or an integer value.

As an implementor of a decoder of key 1, I believe the text implies I MUST be able to decode floats. It seems like the encoder is free to choose int of float so there is only guaranteed interoperability of key 1 if the decoder supports float.

This is why I went to the trouble of explicitly excluding float for tag 1 in EAT. It prevents the sender from ever sending float so the decoder is clearly relieved from implementing float.

> Why do you need a different key as well?

Yes it could be either a different key or clearly state float is optional for key 1.

>> As this document stands all decoders of it MUST support one of IEEE 754 float, decimal fractions or big numbers.
> 
> I don’t agree.

OK, perhaps "strongly implied" because the definition of key 1 doesn’t explicitly say float is optional. 

> The point of key -3 etc. is that you can combine an integer base time with fractional seconds that are again specified in an integer key.
> 
>> There’s no simple integer-only option. I can’t do a simple thing like indicate an integer time is TAI without implementing IEEE 754 float. OK. Well I could and probably it would mostly be OK in practice, but it would be out of compliance with the specification. We’re serious about interop in the IETF, right?
>> 
>> Evidence I’d offer for this is multiple serious requests that I’ve had and fulfilled to optionally remove all use of floating point from my QCBOR library.
> 
> This is totally within the domain of application of this tag!

TLDR for the following: should the document have a profiles section?

The reality of this tag is that it must be profiled to guarantee interoperability. For example a protocol incorporating this needs to say which base-time and which fractions the sender can send and which the receiver must decode. There’s quite a lot to profile here. A full profile should probably list every key the sender can send and which the receiver must decode. Even within some keys (e.g., timescale) profiling is needed. Float optionality is just another thing to profile.

EAT takes this issue head-on admitting up front it is a framework, not a protocol and defines a profiling method. COSE also has some paragraphs that say it must be profiled for interoperability.

It was fairly strongly suggested that EAT was taking the easy way out and not doing it’s job by not locking down for full interoperability. It seems to me that those that said that about EAT might say that of this tag. However, I strongly disagree with that suggestion. The variability in EAT and here are present for a very good reason and should not be removed.

I kind of think there should be a paragraph that explains that this tag needs to be profiled for interoperability.

To go on a little further — the FIDO standard has a full protocol compliance validation/logo program that was used to validate that end consumer client products interoperate fully with commercial server products. This helped make the FIDO standard much tighter. There are only a few implementation options and they are managed. Probably WiFi Alliance, W3C, BlueTooth… do similar. This is what I think of for interoperability. A lot of IETF protocols, like COSE and this draft, are building blocks or tool kits, not end commercial interoperable protocols. That’s fine. The question is do we be up front about this the way EAT is.

LL