Re: [Taps] Artart early review of draft-ietf-taps-interface-13

[Michael Welzl <[e-mail]>]

> On 17 Sep 2021, at 22:57, Robert Sparks via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Robert Sparks
> Review result: On the Right Track
> 
> This is an art-art early review of draft-ietf-taps-interface
> 
> This document reads fairly well. I have some structural concerns (see the early
> review of the architecture document). In particular I think this document
> contains the actual definition of the architecture and some of that is
> inferential.
> 
> There are several places where the language in this document, once published as
> an RFC, will not age well. Search for occurrences of the word "modern" and
> consider writing less to motivate and more to define. Reconsider the use of
> terms like "traditional".
> 
> Reconsider words like "atop" and "underneath" when talking about interface
> boundaries. (Also, I don't know what it means to be atop an architecture.)
> 
> I cringe at systems that define Integers to be integers and some other
> enumerated things ("special values"). Is it necessary to define things this
> way? I understand it's currently typical to have this behavior in the APIs of
> the protocols this system is attempting to abstract, but is it impossible to
> stop propagating that kind of type confusion? This API could move the things
> special values are being used for into separate arguments for example.
> 
> The definition of Numeric in 1.1 is circular.
> 
> The concept of cloning is used in the overview without introduction. I've
> suggested an earlier discussion of it in the architecture document, which would
> help, but even in this document, there should be an introduction or forward
> reference.
> 
> As you can tell, I don't think the documents do what the first sentence of the
> API summary claims. This document would not lose anything if you deleted the
> sentence.
> 
> At the end of the API summary, there's a paragraph that looks like a "Structure
> of this document" discussion - should it be in a different place? The list in
> the paragraph is odd - why is Section 8, for instance, not in the list?
> 
> Consider showing an explicit context in the Send in the example in 3.1.1 - it
> would make the reader guess less to wait to introduce implicit contexts later.
> 
> In the text version of the document, the last paragraph before 3.1.3 has some
> markup (~~~) that has leaked through.
> 
> 3.1.3 has a comment that's too long for the text version
> 
> The first sentence-paragraph of section 4 is complex. Please break it apart.
> 
> At the discussion of Transport Property Names at 4.1, I think you need to be
> more specific than "alphanumeric". Explicitly list what characters are allowed.
> 
> The "reformatted where necessary" part of the requirement in the last paragraph
> of 4.1 makes the MUST NOT almost impossible to conform to. I think I understand
> what the paragraph is trying to say, but the current formulation leaves the
> reader guessing. I think you are trying to say "Avoid using any of the terms
> listed as keywords in the protocol numbers registry as any part of a vendor or
> implementation-specific property name".
> 
> Do you really mean "host" at "(endpoints) SHOULD represent the same host" in
> section 6, or do you mean something more like service, given that an endpoint
> may configured with a DNS name?
> 
> I think there's a need for more discussion and clarity around the restriction
> that "An Endpoint cannot have multiple identifiers of a same type set." Why
> not? You effectively are letting the endpoint have (potentially) multiple IP
> addresses when you create it with an identifier that is a DNS name. This feels
> like an artificial, inconsistent, restriction given the rest of the document
> (look at the 3rd paragraph of 6.1).
> 
> Would there be any benefit of having something like "WithServiceName" to
> qualify a specifier for use with NAPTR/SRV? Or is the thinking that the
> resolution of the thing handed down through WithHostname is restricted to
> A/AAAA?
> 
> Why does the API have both the ability to say LocalSpecifier.WithInterface and
> to set interface requirements on TransportProperties? Why not just keep the
> latter and remove the former?
> 
> I would really like to see an example of how an application that relies on ICE
> learns about new candidates as they are discovered - the callback pattern is
> not clear to me yet.
> 
> There are several places where the document says "Changes to a <foo> after
> func() has been called do not have any connection on existing <bar>s". This
> feels like an opportunity for more clarity about what a <foo> really is. The
> document uses the word Object, and maybe there's an assumption that there's a
> shared understanding of what the properties of an Object are. Being explicit
> with something like "An Object is a collection of properties that
> implementations of the API read and use..." to avoid the "here's a chunk of
> memory that can get passed into the API that it can operate on, and expect the
> caller to see the effects of the operation" interpretation. I _think_ what the
> current text is trying to say is that the user of the API should assume the
> implementation of the API made its own private copy of the Object at the point
> of the given func() call and that it will not see any changes to the copy the
> user has at any later point. But then there's some text that looks like "Once a
> <foo> has been used with a func(), it is invalid to modify any of its
> properties." So...
> 
> I'll have to read the document yet another time, perhaps, but after the few
> times I went through it for this review, I'm still not clear on how/whether
> connections are grouped because of actions from the remote end(s). If so, how
> does the user learn about the grouping? Does it have to poll
> Connection.GroupedConnections() every time its told of a new Connection?
> 
> In section 8's third paragraph, the MUST NOT is not a good use of 2119/8174.
> Are you trying to say protocol specific property _names_ must not be reused
> across different protocols?
> 
> At the last bullet before section 8.1, are you RFC7556 being pointed to as an
> example way one might derive properties? If so, adjust the language to make it
> clear that its an example.
> 
> In 8.1.1 the second sentence is convoluted. Is there a way to restate it
> without as many nots, nons, an different zeros?
> 
> At 8.1.2 you say lower values have higher priorities for connPrio. But later,
> at msgPrio, higher values have higher priority, and in the examples where you
> speak of the interaction of connPrio and msgPrio, you imply that higher means
> higher for _both_ properties. Please check for consistency and clarify.
> 
> Is the "scope" used for message contexts (9.1.1) ever anything but a framer? If
> not, could this section just say framer?
> 
> At the discussion of Send Events, I worry that the API is closing a door on
> allowing the api to channel information about what's happening because of a
> Send that may be available from future transports. Maybe such things would be
> communicated as events from Connection instead (like Connection->SoftError),
> even if they happened to be Send specific?


I added this as a set of issues in github, where we can also discuss whether/how to resolve these:

https://github.com/ietf-tapswg/api-drafts/issues

Michael