Re: [manet] Tsvart last call review of draft-ietf-manet-dlep-pause-extension-06

[Justin Dean <[e-mail]>]

A few comments inline marked JD, Justin Dean WG chair.
On Wed, Apr 10, 2019, 8:41 PM Lou Berger <[e-mail]> wrote:

> Bob,
>
> Thanks for the comments - see below for responses.
>
> On 4/4/2019 7:56 PM, Bob Briscoe via Datatracker wrote:
> > Reviewer: Bob Briscoe
> > Review result: On the Right Track
> >
> > 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
> > [e-mail] 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.
>
> I guess I'll have to talk to the Shepherd on this one to see how much
> he/the WG would want to add on this at this point.  Perhaps giving a
> reference to the earlier PPPOE (rfc5578) would suffice, what do you think?
>
JD will talk with Lou and respond in separate email.

>
>
> >
> > ===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.
>
> Done.
>
> >
> > Why is the scope limited to DLEP radio links? I would have thought this
> > protocol is generally useful between a and modem and router.
>
> Because this is a DLEP extension.  Other mechanisms exist for other
> technologies, e.g., RFC5578 or Ethernet PAUSE/PFC.
>
>
> > ==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.../
> okay (paraphrase a bit)
> >
> > ===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?
>
> I generally agree that a control plane fault should not result in a data
> plane loss -- in some environments, I'd say this is a must.  This said,
> your comment goes to a design principle in DLEP RFC8175 where control
> plane error result in session resets and data plane impacts.  I think
> changing this basic behavior of DLEP is beyond the scope of this
> extension.  I think a general modification of base DLEP to support such
> would be a fine idea.
>

JD there may be more robust and clever solutions here that don't break the
base specification. Absence of an acknowledgement is not the same as an
error, syntax or otherwise, which causes a reset. Allowing for a periodic
restart message if data does not resume may be sufficient.

>
>
> >
> > ==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.
> It is largely for reporting to a user/operator for "informational
> purposes".  If an implementation chooses, it can put in zero or
> infinity.  In most cases I understand this will be a straightforward
> value that can be reported to the router and its operators.
> > 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.
>
> I think a service rate / queue delay property would be very interesting
> - but this didn't come up in the WG, so I don't think it is appropriate
> to add it here.  There is also nothing preventing such information being
> added in a future extension.
>
>
> >
> > ==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?
>
> if it has no data to send. I don't object changing this to a MUST if you
> think important.
>
>
> JD I actually think this should be a MAY but with added text explaining
why data may not be sent to the modem. No data, better external links,
other.

>
> > ==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 same attacker can subvert the dlep session an cause a session reset
> and take down all traffic.  So how is this case different?
>
> > 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.
>
> Fair point - how about:
>
>
>    Note that this extension does allow a compromised or impersonating
>    modem to suppress transmission by the router.  Similar attacks are
>    generally possible base DLEP, for example an impersonating modem may
>    cause a session reset or a compromised modem simply can
>    drop all traffic destined to, or sent by a router.  <xref
>    target="RFC8175"/> defines the use of TLS to protect against the
>    impersonating attacker.
>
>
> >
> > ==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."
>
> sure.
>
>
> > 3.1
> > s/with Queue Index/
> >   /with a Queue Index/
>
> okay
>
> > Scale:
> > s/An 4-bit/
> >   /A 4-bit/
> okay
> > 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'.
>
> will clarify the intent.
>
>
> > Delete gratuitous 'is':
> >    "The motivating use case [is] for this
> >     data item is when a modem's internal queue length exceeds a
> >     particular threshold."
> yes,
> >
> > 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."
> >
> >
> Done!
>
> Thank you for the comments.
>
> Lou
>
> PS the working document has been updated, if interested see
> https://github.com/louberger/dlep-extensions/tree/master/pause
>
> > _______________________________________________
> > manet mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/manet
> >
>