Last Call Review of draft-ietf-detnet-tsn-vpn-over-mpls-05
review-ietf-detnet-tsn-vpn-over-mpls-05-tsvart-lc-ott-2021-02-04-00

Request Review of draft-ietf-detnet-tsn-vpn-over-mpls
Requested rev. no specific revision (document currently at 07)
Type Last Call Review
Team Transport Area Review Team (tsvart)
Deadline 2021-02-05
Requested 2021-01-22
Authors Balazs Varga, János Farkas, Andy Malis, Stewart Bryant, Don Fedyk
Draft last updated 2021-02-04
Completed reviews Secdir Last Call review of -05 by Magnus Nystrom (diff)
Tsvart Last Call review of -05 by Joerg Ott (diff)
Genart Last Call review of -05 by Vijay Gurbani (diff)
Assignment Reviewer Joerg Ott 
State Completed
Review review-ietf-detnet-tsn-vpn-over-mpls-05-tsvart-lc-ott-2021-02-04
Posted at https://mailarchive.ietf.org/arch/msg/tsv-art/IKWott5JLsAS2qzCizMzSxoAy0g
Reviewed rev. 05 (document currently at 07)
Review result Almost Ready
Review completed: 2021-02-04

Review
review-ietf-detnet-tsn-vpn-over-mpls-05-tsvart-lc-ott-2021-02-04

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
tsv-art@ietf.org if you reply to or forward this review.

The draft describes interconnecting two TSN networks via a DetNet MPLS data plane
and specifies the interconnecting entities.

Note: I read through RFC8655, RFC8938, and draft-detnet-mpls prior to this review
but I may have missed some document or detail.

From a transport area perspective, there are no specific issues in this draft.
Possible encapsulation and MTU size issues would be covered generically 
by the underlying services and not be specific to this document.

I have one question, though, concerning flooding, which may be an issue
and deserve clarification: p.9, para 2 states (and para 4 has similar text):

   "If there are no TSN
   Stream specific forwarding configurations the PE MUST flood the
   packet to other locally attached CE(s) and to the DetNet Service
   Proxy.  If the administrative policy on the PE does not allow
   flooding the PE MUST drop the packet."

In case of a misconfiguration, there does not appear to be a rate limiting
or termination criterion defined (maybe the next hop or some other 
entity would respond and a corresponding forwarding configuration
installed, e.g., similar to MAC address mapping in switches), so that it
seems flooding could persist as long as the traffic and other DetNet
services might be impacted. Does it make sense to add some further
explanation here (and for para 4).

Also curious: in 5.2 on page 10, is there no identification validation in
this direction?

The draft has a few general issues:

1. Terminology: Stream Identification vs. Stream ID vs. Stream-ID vs. Stream.
Maybe to a short introduction of the acronym to use, even though this is
almost obvious. 

2. The Requirements Language don't seem to be used in all places where it should be.
Examples: last para of section 4.1? Section 5.1, including 2nd para (has keywords)
vs. 4th para (doesn't use them) on page 9.

3. Does the last para of 5.2 (PREOF and FRER interworking) contradict the 2nd para
of 5.3? While being rightly out of scope: how would sequence number copying work
(as suggested in the last para of 5.2) in case there is an N:1 stream mapping?

4. Does it make sense to provide an example in sect. 6. after the bullet list on page 11?

Nits:
Won't list all since the RFC Editor will do a check on those, but it would be good
to use use consistent capitalisation (e.g., Edge Node vs Edge node, Domain) and
avoid extra plural s. Many articles are missing.
Term "required in 2nd para on p.9
Unclear references: 5.1, 3rd para: "it", "before forwarded"