Subject: [tram] Alissa Cooper's No Objection on draft-ietf-tram-stun-path-data-03: (with COMMENT)

From: "Alissa Cooper" <[email-masked]>
Date: Tue, 19 Apr 2016 13:18:22 -0700

Alissa Cooper has entered the following ballot position for
draft-ietf-tram-stun-path-data-03: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tram-stun-path-data/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

= General =

I find the name the name PATH-CHARACTERISTIC a bit general for this,
since it specifies a single specific characteristic while one could
imagine other characteristics one might want to specify in the future.
Would suggest naming this something more specific.

= Section 1 =

I think it will be important for future ICE specs to document expected
usage of this feature, and those won't necessarily be limited to
continuous nomination. I don't think there is anything to be done in this
draft, but just noting here that close coordination between TRAM and ICE
will be necessary to make sure this happens going forward.

= Section 3.3 =

Section 3.1 defines ReTransCnt as "Number of times request is
re-transmitted with the same transaction ID to the server," but then the
examples in 3.3 all show the client populating this field with 1 in its
first request. If the number is supposed to be about *re-transmits* (not
transmits, but re-transmits), I would have expected it to be 0 in the
first request, then 1 upon an actual re-transmit. Why do these start at
1? It may be that simply re-wording the use of "re-transmits" in 3.1
would fix this.

Also, in the first two examples, why do the ReTransCnt values keep
incrementing after a successful response? For example, in the normal
case, why does the client re-transmit its request with ReTransCnt equal
to 2 when it received a successful response?

---

From: "Pal Martinsen (palmarti)" <[email-masked]>
Date: Wed, 20 Apr 2016 07:34:34 +0000

On 19 Apr 2016, at 22:18, Alissa Cooper <[email-masked]<mailto:[email-masked]>> wrote:

= General =

I agree.

How about; TRANSACTION_TRANSMIT_COUNTER, with the fields just called req and resp.

By explicitly calling it transmit and not retransmit we also get rid of the problems regarding what value do the first retransmit have. And responses really have no retransmits. Slight rewording in the document is needed to rephrase from retransmit to just using transmit with the same transaction ID.

Figure 1 and Figure 2 are identical. Would it be better to have one section describing the attribute and then the two sections can describe usage with request and response?

In hindsight I also find the naming a bit to generic. It calculates fractional loss and RTT.
I think a better name would be; “Discovery of fractional loss and RTT using STUN”

The draft also talks about loss. I am missing a discussion of how to handle crossing reqs and resps. Sometimes a client would have sent a retransmit before receiving the original response. A section on how to calculate RTT (This is now simple because of the counter), and loss (We need to wait to see if we actually get a response from the second retransmit?) in that scenario will be useful for implementers.

= Section 1 =

IMHO there should be no “upwards” dependencies. TRAM should build generic building blocks, however sometimes they are tailored for some use. In this case I think it would be sufficient to describe that ICE can use this to do “smarter things”(tm).  I would prefer if we loss the references to specific ICE drafts that may or may not go forward.

For me this draft is useful due to the new metrics it provides. (That is why Kathleens comment on the security section is important.)

= Section 3.3 =

Renaming of attribute and fields and rewording to consistently talk about transmits with same transaction ID would fix this.

I think it is a bug. But if the clients want to send more packets to get a better measurements of the fractional loss it might end up with a retransmit instead of creating a new transaction. This might simplify the client a bit as it can easily collect metrics by looking at the new attribute. This may break something in the STUN spec so this needs to be investigated and better described.

Disclaimer.. I am one of the co-authors. I am sorry for not catching some of these new issues earlier. I just started to implement this things became a bit clearer when the draft needs to be transformed into actual running code.

.-.
Pål-Erik