Subject: [Taps] Review of taps-interface

From: Martin Duke <[email-masked]>
Date: Thu, 6 Aug 2020 12:30:02 -0700

Here is my promised "no prior knowledge" review of taps-interface. I
haven't filed issues for these yet, as I suspect some of these have already
been well-litigated there.

The only other draft I've read is taps-arch.

*High level comments*:

(1) I think this conceptually headed in the right direction.

(2) I found the relationship between streams and connections to be
extremely murky throughout the draft, and left it not really sure how I
would control a streamed transport.

Section 6.4 hints suggests that opening additional streams is an
alternative implementation of Clone() for transports that can do so. Does
this imply that if the peer stream limit does not allow new streams, the
transport should open a connection instead? How does
SetNewConnectionLimit() in Sec 6.2 relate to streamed transport -- are
there two levels of listener: a stream listener and a connection listener?
How would opening "stream listeners" relate to the maximum number of
streams the peer could open (i.e. do I have to call listen() 8 times to
allow the client to open 8 streams)? Or are there no stream listeners and
these "connections" just sort of appear?

Skimming the -impl draft, I can see there's some text here but it is not
fitting together for me. There are at least three streamed transports and
it would be helpful to explain how the API conceptualizes this, perhaps in
a new section.

The next two major points are related to streams, so if I've missed
something important above they may be complete nonsequiturs.

(3) I'm not sure the priority framework really fits together.

Sec 6.4 says we 'should' do something that sounds like WFQ, but 7.1.5
allows the application to decide what happens. Meanwhile message priority
suggests a strict priority-based scheduler.

Meanwhile, in sec 8.1.3.2 "Note that this property is not a per-message
override of the  connection Priority - see Section 7.1.3.  Both Priority
properties
   may interact, but can be used independently and be realized by
   different mechanisms."

At the risk of overindexing on a partly-baked draft in httpbis, how would I
implement strict priority of HTTP/3 streams over a taps API that implements
QUIC? Would I need to request strict priority in *both* connection priority
and message priority? Or I guess connection priority would be sufficient?

(4) I found myself continually asking how I would implement foo-over-http/x
using this API, and very much stumbled over ambiguity regarding HTTP 1.1
pipelining. While it's certainly possible to order messages in the proper
way using this API, if I was writing this agnostically to HTTP version, it
appears it would gravitate to opening a pooled TCP connection for each
request/response if we ended up on HTTP/1 [because each request/response
would have a clone() call]. Maybe this is OK, but I found this conclusion
to be jarring.

*Specific comments*:

sec 4.2.2
"implementations MUST provide a
      method for the application to determine the entire set of possible
      values for each property."

Is this meant to be a requirement for a machine-readable API for learning
the enum, or merely a requirement for documentation? i.e. is it
satisfied if the values are in the man page or comments of the C header
file?

sec 5.
I found it curious that there is no API here for registering callbacks, or
mention that this is a precondition to connection. Surely event handlers
are a prerequisite?

sec 5.1
I found it odd that there didn't appear to be any sort of formal list of
attributes of an endpoint, just some examples.

sec 5.2.7
I am not sure what this sentence means: "Disabling this property may enable
to control checksum coverage later."

sec 5.2.9
It would be helpful to say "Applications that neither prohibit nor require
congestion should query the result and disable their own congestion control
if present in the transport"

sec 5.2.12
Using temporary local addresses may break various mechanisms that prove
address ownership (e.g QUIC resumption tokens) and therefore impair
performance as the client has to re-verify its address.

sec 6.2
"After Stop() is called, the Listener can be disposed of." I don't
understand the difference between calling listener.stop() and simply
disposing of the object. Does this imply that there is a listener.start()
that allows a configured listener to resume?

sec 7.1.4
It would be helpful to clarify the relevance of control vs application data
in the idle timeout -- for various reasons, the connection could stay alive
regardless of how active the connection is (in fact, perhaps "keepalive" is
a generally useful notion that can be exposed in the API?)

sec 7.1.8
This section is missing a reference to minSendRate and minRecvRate

sec 7.1.9.
We're going to need an event for MTU changes. These can't simply be Send
errors because the Sent event might go first, or the transport might be
sending a probe with padding that doesn't correspond to a message.

sec 7.2
perhaps I'm not thinking of this correctly, but should there be a way to
query the peer-offered user timeout?

sec 8.3.2.1
Reading the Sent event, I was pretty sure that reliable transports would
deliver the event until the ack arrived. But Sec 10 most definitely does
not say that. I would think that the ack would be a more useful signal, but
maybe I'm in the rough on this -- certainly many transport implementations
will not do this well.

-Martin

---

From: Michael Welzl <[email-masked]>
Date: Fri, 7 Aug 2020 10:21:33 +0200

Hi Martin,

Thanks much for this detailed review!   - and for the many interesting questions you’re raising; you discovered a few important holes there. E.g., I believe we just haven’t discussed stream limits; I would have thought that that’s an implementation matter, but the fact that you bring it up in a review of the interface draft is a reminder that applications may want to be aware of this behavior.
(as a quick answer to this particular item, in my opinion Clone() should fail if the stream limit is exceeded - getting a mix of Connections and streams, altogether grouped is surely not what anyone would want or expect; e.g. priorities within the group won’t work out anymore). Note that the idea is that an application can query for what it got if it’s interested in that level of detail.

Now, for addressing the comments, I think we should just split them up by topic into issues for the repository, and then address them one by one.

Thanks again!

Cheers,
Michael

> On Aug 6, 2020, at 9:30 PM, Martin Duke <[email-masked]> wrote:
> 
> Here is my promised "no prior knowledge" review of taps-interface. I haven't filed issues for these yet, as I suspect some of these have already been well-litigated there.
> 
> High level comments:
> (2) I found the relationship between streams and connections to be extremely murky throughout the draft, and left it not really sure how I would control a streamed transport.
> 
> Section 6.4 hints suggests that opening additional streams is an alternative implementation of Clone() for transports that can do so. Does this imply that if the peer stream limit does not allow new streams, the transport should open a connection instead? How does SetNewConnectionLimit() in Sec 6.2 relate to streamed transport -- are there two levels of listener: a stream listener and a connection listener? How would opening "stream listeners" relate to the maximum number of streams the peer could open (i.e. do I have to call listen() 8 times to allow the client to open 8 streams)? Or are there no stream listeners and these "connections" just sort of appear?
> 
> Skimming the -impl draft, I can see there's some text here but it is not fitting together for me. There are at least three streamed transports and it would be helpful to explain how the API conceptualizes this, perhaps in a new section.
> 
> The next two major points are related to streams, so if I've missed something important above they may be complete nonsequiturs.
> 
> Sec 6.4 says we 'should' do something that sounds like WFQ, but 7.1.5 allows the application to decide what happens. Meanwhile message priority suggests a strict priority-based scheduler.
> 
> Meanwhile, in sec 8.1.3.2 "Note that this property is not a per-message override of the  connection Priority - see Section 7.1.3.  Both Priority properties
> At the risk of overindexing on a partly-baked draft in httpbis, how would I implement strict priority of HTTP/3 streams over a taps API that implements QUIC? Would I need to request strict priority in *both* connection priority and message priority? Or I guess connection priority would be sufficient?
> 
> (4) I found myself continually asking how I would implement foo-over-http/x using this API, and very much stumbled over ambiguity regarding HTTP 1.1 pipelining. While it's certainly possible to order messages in the proper way using this API, if I was writing this agnostically to HTTP version, it appears it would gravitate to opening a pooled TCP connection for each request/response if we ended up on HTTP/1 [because each request/response would have a clone() call]. Maybe this is OK, but I found this conclusion to be jarring.
> 
> Specific comments:
> 
> sec 4.2.2
> Is this meant to be a requirement for a machine-readable API for learning the enum, or merely a requirement for documentation? i.e. is it satisfied if the values are in the man page or comments of the C header file?
> 
> sec 5.
> I found it curious that there is no API here for registering callbacks, or mention that this is a precondition to connection. Surely event handlers are a prerequisite?
> 
> sec 5.1
> I found it odd that there didn't appear to be any sort of formal list of attributes of an endpoint, just some examples.
> 
> sec 5.2.7
> I am not sure what this sentence means: "Disabling this property may enable to control checksum coverage later."
> 
> sec 5.2.9
> It would be helpful to say "Applications that neither prohibit nor require congestion should query the result and disable their own congestion control if present in the transport"
> 
> sec 5.2.12
> Using temporary local addresses may break various mechanisms that prove address ownership (e.g QUIC resumption tokens) and therefore impair performance as the client has to re-verify its address.
> 
> sec 6.2 
> "After Stop() is called, the Listener can be disposed of.." I don't understand the difference between calling listener.stop() and simply disposing of the object. Does this imply that there is a listener.start() that allows a configured listener to resume?
> 
> sec 7.1.4
> It would be helpful to clarify the relevance of control vs application data in the idle timeout -- for various reasons, the connection could stay alive regardless of how active the connection is (in fact, perhaps "keepalive" is a generally useful notion that can be exposed in the API?)
> 
> sec 7.1..8
> We're going to need an event for MTU changes. These can't simply be Send errors because the Sent event might go first, or the transport might be sending a probe with padding that doesn't correspond to a message.
> 
> sec 7.2
> perhaps I'm not thinking of this correctly, but should there be a way to query the peer-offered user timeout?
> 
> sec 8.3.2.1
> Reading the Sent event, I was pretty sure that reliable transports would deliver the event until the ack arrived. But Sec 10 most definitely does not say that. I would think that the ack would be a more useful signal, but maybe I'm in the rough on this -- certainly many transport implementations will not do this well.
> 
> -Martin
> _______________________________________________
> Taps mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/taps

---

From: Martin Duke <[email-masked]>
Date: Fri, 7 Aug 2020 09:25:11 -0700

OK, I'll file github issues

On Fri, Aug 7, 2020 at 1:21 AM Michael Welzl <[email-masked]> wrote:

> Hi Martin,
>
> Thanks much for this detailed review!   - and for the many interesting
> questions you’re raising; you discovered a few important holes there. E.g.,
> I believe we just haven’t discussed stream limits; I would have thought
> that that’s an implementation matter, but the fact that you bring it up in
> a review of the interface draft is a reminder that applications may want to
> be aware of this behavior.
> (as a quick answer to this particular item, in my opinion Clone() should
> fail if the stream limit is exceeded - getting a mix of Connections and
> streams, altogether grouped is surely not what anyone would want or expect;
> e.g. priorities within the group won’t work out anymore). Note that the
> idea is that an application can query for what it got if it’s interested in
> that level of detail.
>
> Now, for addressing the comments, I think we should just split them up by
> topic into issues for the repository, and then address them one by one.
>
> Thanks again!
>
> Cheers,
> Michael
>
>
> At the risk of overindexing on a partly-baked draft in httpbis, how would
> I implement strict priority of HTTP/3 streams over a taps API that
> implements QUIC? Would I need to request strict priority in *both*
> connection priority and message priority? Or I guess connection priority
> would be sufficient?
>
> (4) I found myself continually asking how I would implement
> foo-over-http/x using this API, and very much stumbled over ambiguity
> regarding HTTP 1.1 pipelining. While it's certainly possible to order
> messages in the proper way using this API, if I was writing this
> agnostically to HTTP version, it appears it would gravitate to opening a
> pooled TCP connection for each request/response if we ended up on HTTP/1
> [because each request/response would have a clone() call]. Maybe this is
> OK, but I found this conclusion to be jarring.
>
> *Specific comments*:
>
> sec 4.2.2
> I am not sure what this sentence means: "Disabling this property may
> enable to control checksum coverage later."
>
> sec 5.2.9
> "After Stop() is called, the Listener can be disposed of.." I don't
> It would be helpful to clarify the relevance of control vs application
> data in the idle timeout -- for various reasons, the connection could stay
> alive regardless of how active the connection is (in fact, perhaps
> "keepalive" is a generally useful notion that can be exposed in the API?)
>
> sec 7.1..8

---

From: Michael Welzl <[email-masked]>
Date: Fri, 7 Aug 2020 21:29:50 +0200

Thanks so much for doing this!  This was meant as a todo item for us, or me, not you!

Cheers,
Michael

Sent from my iPhone

> On 7 Aug 2020, at 18:25, Martin Duke <[email-masked]> wrote:
> 
>