Last Call Review of draft-ietf-manet-dlep-pause-extension-06
review-ietf-manet-dlep-pause-extension-06-tsvart-lc-briscoe-2019-04-04-00

Request Review of draft-ietf-manet-dlep-pause-extension
Requested rev. no specific revision (document currently at 08)
Type Last Call Review
Team Transport Area Review Team (tsvart)
Deadline 2019-04-03
Requested 2019-03-14
Draft last updated 2019-04-04
Completed reviews Rtgdir Last Call review of -05 by Andy Smith (diff)
Secdir Last Call review of -05 by Stephen Kent (diff)
Genart Last Call review of -05 by Dale Worley (diff)
Tsvart Last Call review of -06 by Bob Briscoe (diff)
Assignment Reviewer Bob Briscoe
State Completed
Review review-ietf-manet-dlep-pause-extension-06-tsvart-lc-briscoe-2019-04-04
Reviewed rev. 06 (document currently at 08)
Review result On the Right Track
Review completed: 2019-04-04

Review
review-ietf-manet-dlep-pause-extension-06-tsvart-lc-briscoe-2019-04-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.

==1. Introduction==

It would be useful to describe the use-case where the modem does not implement active queue management but the router does, so the modem can use flow control  to push the queue back into the router, where it can be more intelligently controlled.

===Scope within Intro===
Please extend the single sentence about scope (end of 2nd para of Intro) to explain that pause control only applies to data in the router to modem direction. 

Please also mention that the scope of pause-based flow control is limited to one hop and to a point-to-point link between router and modem, not multipoint. The modem does not track the source of the data in its queue, so it cannot focus pause messages onto particular sending stations on a multipoint link, or onto particular ingress ports of the router.

Why is the scope limited to DLEP radio links? I would have thought this protocol is generally useful between a and modem and router. 

==3. Extension Data Items==

Pls define the direction of the messages:
s/The xxx Data Item is used by a modem to.../
 /A modem sends the xxx Data Item to its peer router to.../

===Unsafe design?===
Is it not good practice to make the data plane robust to unexpected control plane failures (e.g. key expiry, incorrect or mis-timed change of address, etc.) and vice versa? So, would it not be more robust for the router to time-out a pause if no restart appears? Also, if the last message received before shutting down or suspending was a pause, should the router restart or resume in the paused state? What if the router enters a power-saving state after it is paused and misses a restart message?

==Queue size in bytes for informational purposes?==
Why? This strikes me as like one of those Government forms you have to fill in with an ill-formed question that is mandatory to answer, even though sometimes there is no answer, and you cannot proceed until you've answered, even though the answer is not needed anyway. For instance, if there is a shared physical buffer, a size cannot be straightforwardly given for each logical buffer. So, if buffer size info is not used by the protocol, do not include it in the protocol.

On the other hand, how is the threshold at which the modem sends a pause configured. Is that out of scope? If so, where is it specified? Wherever this is specified, it should be possible to specify the threshold in time units (queue delay at the current service rate of the queue), not just in bytes. This is important for queues in a hierarchy where the service rate varies, e.g. dependent on traffic in another queue with priority over it. Or simply where the modem can operate at different rates.

==3.3 Restart==

  "A router which receives the Restart Data Item SHOULD resume
   transmission of the identified traffic to the modem."

Why only SHOULD? Under what conditions would it not?

==4. Security Considerations==

  "The extension does not inherently
   introduce any additional vulnerabilities above those documented in
   [RFC8175]."

Er, no... What about the ability for an off-path attacker to stop the router sending data!?

The last part about TLS needs to be worded differently. Because of the above additional vulnerability, the router MUST verify that all 3 types of message are authenticated by the modem. This requires client authentication mode of TLS, which is not mentioned in RFC8175, so it needs to be mentioned here.  Or is the TLS session set up by the router (so the modem is the authenticated server)? Also this raises the question of key management, if every modem has to be authenticated by its router.

   "but this is not a
   substantively different attack by such a compromised modem simply
   dropping all traffic destined to, or sent by a router."

Er, no... The modem does not need to be compromised for a 3rd party to send spoof messages purporting to be from the modem.

==Nits==

1. Intro
"DLEP peers are comprised of a modem and a router" is incorrect English for what you mean (it means that each peer consists of a modem and a router). Better to do away with this sentence completely, and alter to the previous sentence to "It provides the exchange of link related control information between
a modem and its DLEP peer router."

3.1 
s/with Queue Index/
 /with a Queue Index/

Scale: 
s/An 4-bit/
 /A 4-bit/

In general, I think the term "queue size" is wrongly being used where "buffer size" should be used (the queue size is the varying size of the queue within the buffer at any one time).

Also, pls consistently use the term 'paused' not 'suppressed', which has the potentially ambiguous meaning of 'discarded'.

Delete gratuitous 'is':
  "The motivating use case [is] for this
   data item is when a modem's internal queue length exceeds a
   particular threshold."

CURRENT:
  "e.g., when there
   a non queue related congestion points within a modem, but such are
   not explicitly described in this document."
SUGGESTED:
  "e.g., when there
   are non queue related congestion points within a modem. Such use-cases are
   not explicitly described in this document."