Subject: Tsvart last call review of draft-ietf-manet-dlep-multi-hop-extension-06

From: Bob Briscoe via Datatracker <[email-masked]>
Date: Tue, 09 Apr 2019 04:22:02 -0700

Reviewer: Bob Briscoe
Review result: Ready with Issues

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
[email-masked] if you reply to or forward this review.

AFAICT, there's nothing technically wrong, but I believe the authors use of
normative text does not convey the meaning they intended, in nearly every case.
 It will be seen below that there seems to be a general misunderstanding of the
use of, 'SHOULD do X' when there will never be a case when X should not be
done. 'SHOULD' and 'MAY' need to be used judiciously, because they make
interoperability harder.

==Inappropriate Normative Text==

S.2.
"The use of the Multi-Hop Forwarding Extension SHOULD be configurable. "
{I think you mean it SHOULD be possible to enable/disable the extension? On a
modem. a router or both? In what circumstances would it not be configurable (if
none, then MUST would be appropriate)? What happens if one device uses the
extension and another doesn't? This last question also applies for incremental
deployment.}

S.3.1.
"The Hop Count Data Item SHOULD be carried in the Destination Up, Destination
Update, Destination Announce Response, and Link Characteristics Response
messages." {I don't think the use of "SHOULD" here achieves what you intend. I
think you're trying to say that this data item can only be carried in these 4
messages. But what you've said is that all these messages SHOULD contain a Hop
Count data item. If any normative text is needed here, I think it would say
this data item MUST NOT be carried in messages other than these 4. But maybe
just substitute 'SHOULD' with 'can'?}

S.3.2.
The Hop Control Data Item MAY be carried in a Session Update Message sent by a
router when the control applies to the whole device, or a Link Characteristics
Request Message when the control applies to a particular destination. {Again, I
think you're trying to say, "If used, the Hop Control Data Item MUST only be
carried in" ...one of these two message types.}

"A modem that receives the Hop Control Data Item in a [XXX] Message SHOULD take
whatever actions are needed to make the change indicated by the data item for
[YYY]." (two occurrences) {Inappropriate use of SHOULD: a) Surely a modem MUST
"take whatever actions are needed." Why would it not? b) Anyway, it's
meaningless to normatively require a vaguely defined action}

S.3.2.3 Direction Connection

"It indicates that the modem SHOULD attempt to establish a direct connection
with the destination identified in the message." {I think you mean 'MUST'. Why
would it not even attempt to?}

"This action SHOULD only be sent for destinations for which the Hop Count is
greater than 1 and has the P-Bit set in the previously received Hop Count Data
Item." {I think you mean MUST. Why would it be sent otherwise?}

S.3.2.4. Suppress Forwarding
I suggest that you don't gratuitously switch from 'MUST' to 'SHALL' just for
this section? Many implementers search the text for 'MUST'.

==Authentication==

S.4.
RFC8175 says
   'For all "networked deployments" ..., the implementation and use of TLS are
   STRONGLY RECOMMENDED.'

I believe it would be worth identifying which extensions would be unsafe if TLS
were not used. Certainly all the Multi-Hop Extensions would be unsafe if not
authenticated.

==General Nits==
Inappropriate and/or inconsistent capitalization of certain phrases, like 'Data
Item' or 'Action' or 'Message'.

==Specific Nits==
I've provided suggested corrections in the following xml files:
http://bobbriscoe.net/tmp/draft-ietf-manet-dlep-multi-hop-extension-06_BB.xml
http://bobbriscoe.net/tmp/draft-ietf-manet-dlep-multi-hop-extension-06_BB.txt
http://bobbriscoe.net/tmp/draft-ietf-manet-dlep-multi-hop-extension-06_BB-DIFF-06.html

---

From: Lou Berger <[email-masked]>
Date: Thu, 25 Apr 2019 11:45:16 -0400

Hi Bob,
	Sorry for the late reply. See below for specific responses.

On 4/9/19 7:22 AM, Bob Briscoe via Datatracker wrote:
> {I think you mean it SHOULD be possible to enable/disable the extension?

To me that's what this says.

> On a
> modem. a router or both? 
yes, by any node that supports the extension.

> In what circumstances would it not be configurable (if
> none, then MUST would be appropriate)?
We generally don't stop people from building products that meet their
specific market requirements as long as there's no interop issue, hence
the SHOULD vs MUST.> What happens if one device uses the
> extension and another doesn't? This last question also applies for incremental
This is covered in the base DLEP RFC.

> just substitute 'SHOULD' with 'can'?}

I'd be fine with MAY as this is more aligned with RFC8175 statement of
what DIs to carry in a message.  This siad, I believe the feeling was
that MAY was too weak for an extension-specific DI.  Unless there is
strong objection with this, I'm inclined to leave it with how the WG
approved it.

> S.3.2.3 Direction Connection

this should be Direct Connection

> would it not even attempt to?}

local policy, or that it know that a connection is no longer possible.

> Item." {I think you mean MUST. Why would it be sent otherwise?}

A must would translate to a dlep error on the receiving side, which
would translate to a session reset, which is really disruptive.

> were not used. Certainly all the Multi-Hop Extensions would be unsafe if not

umm, I think all of DLEP would be unsafe without TLS, or some other
equivalent, e.g., macsec.

> http://bobbriscoe.net/tmp/draft-ietf-manet-dlep-multi-hop-extension-06_BB-DIFF-06.html

Thanks!
I've pushed changes to https://github.com/louberger/dlep-extensions and
will coordinate an update with the AD.
Lou
> 
> 
> _______________________________________________
> manet mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/manet
>