Early Review of draft-ietf-taps-impl-12
review-ietf-taps-impl-12-artart-early-gondwana-2022-05-30-00

Apologies for the really delayed review - I clicked "accept" on this thinking
I'd get to it immediately, and then was unable to before I went on vacation.
Thanks Barry for prodding me to get back to it.

Overall, this document looks fine.  I reviewed it in isolation, without reading
all the specs to which it refers.

Thanks for the detailed work.  If I was to criticize the structure at all, I'd
say it's a bit repetative in some places - but I'm also aware that as a
reference, it's necessary to repeat the same information!

Having said that - he's some notes of things I noticed or that confused me on
the read through - take what you will if it's useful to you!

Cheers,

Bron.

-----

   [...] During the process of
   establishment (Section 4), the Connection will not be bound to a
   specific transport protocol instance, since multiple candidate
   Protocol Stacks might be raced.

It took me a while to understand "raced" in this context - but I guess it's
well known to anyone in the field.

There's a couple of English language things:

   Once the Connection is established, Transport Services implementation
   maps actions and events to the details of the chosen Protocol Stack.

"the Transport Services"?

   [...] The properties held by a Connection or Listener is
   independent of other connections that are not part of the same
   Connection Group.

"the properties held by ... are"?

   Connection establishment is only a local operation for a Datagram
   transport (e.g., UDP(-Lite)), which serves to simplify the local
   send/receive functions and to filter the traffic for the specified
   addresses and ports [RFC8085].

That's a mess.  I rewrote it as:

"For a Datagram transport (e.g. UDP(-Lite)), connection establishment is only a
local operation, where it serves to simplify the local send/receive functions
and ..."

   The implementation stores these properties as a part of the
   Preconnection object for use during connection establishment.  For
   Selection Properties that are not provided by the application, the
   implementation must use the default values specified in the Transport
   Services API ([I-D.ietf-taps-interface]).

Normative MUST?  I guess this is just a guidance document so doesn't need
anything normative.  There's some later "should" as well which I would
generally ask the same question for.

   This document structures the candidates for racing as a tree as
   terminological convention.  While a a tree structure is not the only
   way in which racing can be implemented, it does ease the illustration
   of how racing works.

Apart from the 'a a', the first sentence is also clumsy.  Maybe:

"For ease of illustration, this document structures the candidates for racing
as a tree, however a tree structure is not the only way in which racing can be
implemented".

   *  "Interface Instance or Type": If the application specifies an
      interface type to be preferred or avoided, implementations should
      accordingly rank the paths.  If the application specifies an
      interface type to be required or prohibited, an implementation is
      expeceted to not include the non-conforming paths.

Typo "expected".

   The set of possible Local Endpoints is gathered.  In the simple case,
   this merely enumerates the local interfaces and protocols, and
   allocates ephemeral source ports.  For example, a system that has
   WiFi and Ethernet and supports IPv4 and IPv6 might gather four
   candidate Local Endpoints (IPv4 on Ethernet, IPv6 on Ethernet, IPv4
   on WiFi, and IPv6 on WiFi) that can form the source for a transient.

I don't recognise the term "transient" as a noun in this context.  Maybe this
is lack of familiarity.

   The timing algorithms for racing should remain independent across
   branches of the tree.  Any timers or racing logic is isolated to a
   given parent node, and is not ordered precisely with regards to other
   children of other nodes.

"Any timers or racing logic are"?  Or "Any timer or racing logic is". 
Something to align the plurals!  Probably the later since the rest of the
paragraph remains in the singular.

4.7.2.  Implementing listeners for Connectionless Protocols

   Connectionless protocols such as UDP and UDP-lite generally do not
   provide the same mechanisms that connected protocols do to offer
   Connection objects.  Implementations should wait for incoming packets
   for connectionless protocols on a listening port and should perform
   four-tuple matching of packets to either existing Connection objects
   or the creation of new Connection objects.  On platforms with
   facilities to create a "virtual connection" for connectionless
   protocols implementations should use these mechanisms to minimise the
   handling of datagrams intended for already created Connection
   objects.

This is really fuzzy.  There's either a missing comma or some missing words.

   The amount of data that can be sent as 0-RTT data varies by protocol
   and can be queried by the application using the Maximum Message Size
   Concurrent with Connection Establishment Connection Property.  An
   implementation can set this property according to the protocols that
   it will race based on the given Selection Properties when the
   application requests to establish a connection.

The capitalisation in there is very confusing to me - I would expect "With" to
also be capitalised if the whole thing is the name of something, and
"Concurrent" not to be capitalised otherwise.

   Upon receiving this event, a framer implementation is responsible for
   performing any necessary transformations and sending the resulting
   data back to the Message Framer, which will in turn send it to the
   next protocol.  Implementations SHOULD ensure that there is a way to
   pass the original data through without copying to improve
   performance.

but... here we have some NORMATIVE language.  That SHOULD should probably be
non-capitalised to align with earlier usage.  (and so on later in the document,
e.g. in the SCTP description)

----

That's all the notes I took.