Re: [core] Robert Wilton's Discuss on draft-ietf-core-yang-cbor-16: (with DISCUSS and COMMENT)

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Carsten,

Thanks for the answers, please see further comments inline ...

> -----Original Message-----
> From: iesg <[e-mail]> On Behalf Of Carsten Bormann
> Sent: 05 January 2022 16:09
> To: Rob Wilton (rwilton) <[e-mail]>
> Cc: [e-mail]; [e-mail]; The IESG
> <[e-mail]>; [e-mail]; Marco Tiloca <[e-mail]>
> Subject: Re: Robert Wilton's Discuss on draft-ietf-core-yang-cbor-16: (with
> DISCUSS and COMMENT)
> 
> Hi Rob,
> 
> thank you for this quick response!
> 
> >
> > (1) Section 3.2:
> >
> >                                                 Where absolute SIDs are
> > 	   desired in map key positions where a bare integer implies a delta,
> 
> > 	   they may be encoded using CBOR tag 47 (as defined in Section 9.3).
> >
> > Just to check, is may rather than MUST correct here?  I.e., if you want to
> use an absolute SID where a delta is expected then it must use CBOR tag 47,
> or otherwise the receiver would not know it is an absolute SID?
> 
> The use of absolute SID values as map keys indeed requires the use of tag 47;
> the choice is only whether to actually opt for absolute SIDs or to keep with
> the (untagged) normal deltas.
> The sentence you cited was intended as text leading in to a cross-reference,
> not a normative statement.
> 
> However, that normative statement isn’t fully contained in Section 9.3, which
> just defines the CBOR tag, but somewhat smeared over 4.2.1 and the
> example in 4.5.1 (of all places).
> So it seems that some editorial surgery would be beneficial here, making sure
> that the definition how to use tag 47 and an example for that is in a form that
> can be referenced here (and, to reduce may/MAY confusion, we probably
> should substitute “can”).
> 
> Now: https://github.com/core-wg/yang-cbor/issues/137

Okay.

> 
> > (2)
> > 	  *  application/yang-data+cbor; id=sid -- for use by applications that
> 
> > 	      need to be frugal with encoding space and text string processing
> 
> > 	      (e.g., applications running on constrained nodes [RFC7228], or
> 
> > 	      applications with particular performance requirements);
> >
> > 	   *  application/yang-data+cbor; id=name -- for use by applications
> 
> > 	      that do not want to engage in SID management, and that have
> ample
> > 	      resources to manage text-string based item identifiers (e.g.,
> > 	      applications that directly want to substitute application/
> > 	      yang.data+json with a more efficient representation without any
> 
> > 	      other changes);
> >
> > I think that it is fairly obvious from their names, but the definitions don't
> technically restrict what type of ids are used in the content.  Should they be
> more explicit on this?
> 
> The intention was to say this in paragraph 2 of Section 7 — the cited text is
> just the summary of this section.  We could repeat the salient parts in the
> summary, but that has the usual risk of stating the same thing twice in subtly
> different ways.

Ah, that's fine.  I was only checking the diffs, so I had missed this.


> 
> > (3) Related to 2, I did wonder if the document could possible be clearer
> about what is allowed when mixing name and sids, perhaps as an extra
> sentence in the third paragraph of section 3?  As I understand it, barring the
> media-type, anything goes, e.g., a container or list entry could contain some
> entries encoded with name ids and some with sids, e.g., which could be
> required if handling an augmented module that doesn't have sids allocated.
> 
> Indeed, the intention is for the mechanism to be permissive — I think that
> styles/policies on how to best use the mechanism in hybrid scenarios will
> need to evolve before we can write them up (possibly becoming part of an
> RFC-8407-like document at some point).

My comments was really because I was wondering if this permissiveness is perhaps non-obvious (although the file definitions provide a counter example), hence I was questioning whether making the permissiveness explicitly clear would be beneficial to the reader.


> 
> [Skipping items where no further work seems to be necessary:]
> 
> >>> 4. How does the CBOR encoding of SIDs apply to YANG features?  This
> draft
> >>> references features and the SID draft allows SIDs for them, but I don't
> >>> understand how they are used in the encoding (since features don't
> appear
> >> in
> >>> the instance data, they are only at the schema level).
> >>
> >> YANG features can occur in the data of a YANG module, e.g., yang-library
> >> (RFC 8525).  Using a SID instead of an identifier for a feature allows a
> >> compact representation.
> >
> > I'm still not really understanding how this would work.
> >
> > The YANG library data carries the feature as a "yang:yang-identifier", which
> has a string as its base type.
> >
> > So, unless there was some special handling either for the 'feature' leaf-list
> in YANG library, or for the encoding of "yang:yang-identifier" to allow the use
> of SIDs, I think that this would naturally just end up using a string encoding of
> the feature name (along with all other yang:yang-identifiers).
> >
> > I can see that allocating SIDs to features does no harm, but I still don't
> currently see how they can be used.
> 
> We didn’t want to put in examples showing good ways of using these feature
> SIDs, again for mechanism/policy reasons.
> 
> However, an example usage can be found in Page 8 of core-yang-library:
> https://datatracker.ietf.org/doc/html/draft-ietf-core-yang-library-03#page-8
> 
> grouping implementation-parameters {
>     description
>       "Parameters for describing the implementation of a module.";
> 
>     leaf-list feature {
>       type sid:sid;
>       description
>         "List of all YANG feature names from this module that are
> […]
> 
> So a YANG module needs to make use of sid:sid explicitly in order to make
> use of SIDs for features.

Okay, I think that this was the bit that I was missing.


> 
> Enabling the use of SIDs automatically in the encoding of any SID-naive usage
> of yang:yang-identifier could widen the field of application.  However, as you
> note, at the base type level these are just YANG strings, so we would need a
> mechanism such as a CBOR tag that can be used in place of text strings
> anywhere in the encoding.  This would seem to be a rather invasive
> mechanism, for a somewhat limited gain.
> Simply allocating SIDs for YANG features costs nearly nothing, and enables
> the above optimization in a module that will often be used in an id=sid
> environment.

That's fine, I wasn't suggesting that - I was just not understanding how you were planning on using the allocated sids, and whether it needed any explicit text to describe how they are encoded, but I see that this just falls out because they are defined as YANG uint64s.

Note, possibly it would be 'nicer' if the YANG typedef for "sid" was defined as part of a separate typdefs YANG module (arguably ideally in this YANG CBOR draft) rather than the SID file draft, otherwise your CORE YANG library will end up with an import-only dependency on ietf-sid-file.yang.  Not the end of the world, but I thought that I would flag it, and leave it to you to decide whether you want to act on it (i.e., I won't hold my "discuss" for this point).  To give an example of where became a slight issue: OpenConfig wanted to reuse iana-types.yang, which had a dependency on a type defined in ietf-interfaces.yang but didn't want to use ietf-interfaces.yang.  In hindsight, having the type defined in a separate types YANG file would have been better.


> 
> >>> 5. I also support Ben's second discuss point.  I think that as written, this
> >>> draft needs a normative reference to the SID draft.
> >>
> >> I don’t necessarily agree, but this is an easy change without negative
> >> consequences.
> >
> > Okay, I can probably see where you are coming from, in that the intention
> is that the SID draft just defines a file format and allocation strategy, but the
> core definition of a SID is contained in the CBOR draft.
> >
> > The SID draft constraints SIDs to being 63 bits.  Should that text/constraint
> move to the CBOR draft instead?
> 
> Strictly speaking, the YANG-CBOR specification does not have to make this
> constraint; deltas from 64-bit unsigned integers do fit into CBOR 64-bit+sign
> integers.  However, given little compiler support for 64+sign, implementers
> are rather likely to get this wrong.  More so, as the usual source of SIDs to be
> used with YANG-CBOR is constrained to 63-bit integers, as you note, so there
> will be little testing of SID deltas that don’t fit into int64.
> We can defuse this situation, with no practical loss, by insisting on 63-bit
> unsigned integers (0..9223372036854775807) for the SIDs in the YANG-CBOR
> specification, as well.
> 
> Now https://github.com/core-wg/yang-cbor/issues/138

Thanks.

I'll clear my discuss.

Regards,
Rob


> 
> [Thank you for the affirmative comments on our responses to the
> COMMENTs.]
> 
> Grüße, Carsten