Justin, Lou,
Replying to both at once, inline...
On 11/04/2019 03:25, Justin Dean wrote:
A few comments inline marked JD, Justin Dean WG chair.
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
> tsv-art@xxxxxxxx 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.
[BB] What I was looking for was some indication of valid and/or
invalid use cases. I couldn't find anything about what problem this
extension solves.
>
> ===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.
[BB] OK.
> ==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.
[BB] I agree that it should be possible to make pause in itself
robust.
My point about control plane errors doesn't have to be taken as an
architectural point. I was merely saying that if restart fails for
some reason, you don't want data to be paused for ever.
One way to do that is simply to have a globally known constant
timeout on a pause. Then, if a modem wants the router to pause
longer than this timeout, it has to repeat the pause before the
(globally known) timeout. This just ensures that the protocol fails
safe.
Design is up to you - you don't have to use my suggestion.
>
> ==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.
[BB] I suggest then that the draft says "A value of zero means
unknown or unavailable."
> 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?
[BB] I think this part of the question has been overlooked.
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.
[BB] "Didn't come up in the WG" is surely not a valid excuse -
that's why the IETF does out-of-area reviews, isn't it? Measuring
queues in time units has become common practice since about 2010, at
least in transport circles, in order to better handle variable rate
links.
>
> ==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.
[BB] All I am highlighting here is that it's best practice to
accompany a 'SHOULD' with an explanation of the cases where the
recommendation doesn't apply. And if there are none, then the draft
ought to say 'MUST', which makes for simpler code - fewer exceptions
to handle.
Even if there's no data at the moment, restart surely still means
resume transmission (whenever there is data).
> ==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?
[BB] Subverting the dlep session is not an additional vulnerability
introduced by this extension.
Whereas an unauthenticated pause message is an additional
vulnerability introduced by this extension.
I'm just pointing out that the sentence isn't true.
> 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.
That's better.
But, as I said, I didn't see anything in RFC8175 about using TLS
with client authentication. That just might mean I didn't find it.
>
> ==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
Lou, Thx
Bob
_______________________________________________
Tsv-art mailing list
Tsv-art@xxxxxxxx
https://www.ietf.org/mailman/listinfo/tsv-art
--
________________________________________________________________
Bob Briscoe http://bobbriscoe.net/
|
