Re: [Int-dir] Intdir early review of draft-ietf-bier-ping-08

[Greg Mirsky <[e-mail]>]

Hi Brian,
and again, thank you for your review, helpful comments, and questions. I
reworked the IANA Considerations section throughout. Please review the new
working version or diff and let me know if the updates are satisfactory and
address your concerns. Also, answers to your questions are inline below
under the GIM>> tag. The new version of the draft
<https://datatracker.ietf.org/doc/draft-ietf-bier-ping/> also includes
updates addressing comments from Rtg and Sec areas.
Please let me know what you think.

Regards,
Greg

On Fri, Apr 14, 2023 at 3:36 PM Brian Haberman via Datatracker <
[e-mail]> wrote:

> Reviewer: Brian Haberman
> Review result: Almost Ready
>
> I am an assigned INT directorate reviewer for draft-ietf-bier-ping-08.txt.
> These comments were written primarily for the benefit of the Internet Area
> Directors. Document editors and shepherd(s) should treat these comments
> just
> like they would treat comments from any other IETF contributors and resolve
> them along with any other Last Call comments that have been received. For
> more
> details on the INT Directorate, see
> https://datatracker.ietf.org/group/intdir/about/
> <https://datatracker.ietf.org/group/intdir/about/>.
>
> Based on my review, the document IS NOT ready to go to IETF Last Call and
> therefore CANNOT be forwarded to the IESG
>
> There are a number of issues that I think should be addressed before
> advancing
> this document...
>
> 1. IANA Considerations - The draft calls out in section 5 the creation of a
> registry and three sub-registries without giving any guidance on how those
> registries should be managed, per RFC 8126. It is also unclear is section
> 5.3
> is calling for the creation of an IANA registry or not.
>
> 2. Section 3.1
>
>     * It is unclear if the two header formats described here both occur in
> a
>     transmitted frame or if the second header format is a modified version
> of
>     the first header. If it is the former, it seems odd to have to
> duplicate
>     versions, message type, protocol, and reserved fields.
>
GIM>> Thank you for pointing that out. Indeed, these are the same. I've
updated the figures. Please let me know if it helped to make the text
clearer.

>
>      * Is there a requirement that the timestamp formats be the same in
> both
>      the Echo Request and the Echo Reply? If not, is there a requirement
> that
>      BFRs MUST support both formats?
>
GIM>> A good question. I believe that there is no requirement for the
Sender and Responder to use the same timestamp format, as the format used
can be explicitly indicated for each actor separately. As a result, a BFR
can support one format or both. The decision of which one to use may be
based on the comparison of the accuracy of the timestamp each method
provides.

>
>      * The description of the Reserved fields seems incomplete. What
> should a
>      BFR do if the Reserved field is not set to 0?
>
GIM>> Good catch, thank you. Clarified that the value of the Reserved
fields MUST be ignored on receipt.

>
>      * The Echo Request/Reply frame format includes multiple Timestamp
> Sent and
>      Timestamp Received fields. How are they initialized? Which fields get
> set
>      by the BFIR and which ones get set by the BFERs?
>
GIM>> Another great catch. I've updated the description. Let me know if it
is clearer now.

>
>      * The BFR acronym is used in this section, but not defined in Section
> 2.1.
>
GIM>> Added, thanks.

>
> 3. Section 3.2 - If a BFR does not recognize a TLV, it can send a response
> with
> Return Code 2. Does it also drop that frame? What are the frame handling
> rules
> for other error-related Return Codes?,
>
GIM>>I imagine that a BFR that responds to the Echo Request may use the
received packet for the Echo Response. On the other hand, an implementation
may allocate memory for the Echo Response, copy necessary fields and then
drop the Echo Request. Section 4.4 describes *the processing of the
received Echo Request message, including handling errors and appropriately
setting the Return Code value.

>
> 4. Section 3.3 - Some of the defined TLVs/Sub-TLVs indicate that they MAY
> be
> included. Are there specific conditions in which they are, or are not,
> included?
>
GIM>> Section 5.5 lists TLVs and sub-TLVs defined in the draft, including
the relationship between TLVs and sub-TLVs. Only Downstream Mapping TLV may
include sub-TLVs - Multipath Entropy Data or Egress BitString.

>
> 5. Section 4
>
>      * The whole description of the protocol flows would be much better
> with
>      illustrative sequence diagrams.
>
>      * The bulleted lists contain entries with narrative comments
> delineated by
>      '/*' and '*/'. These were hard to parse given the use of '*' bullets.
> I
>      would suggest converting these narratives to 'Notes:'.
>
GIM>> Excellent suggestion, thanks.

>
> 6. Security Considerations - While I see the relationship to traditional
> ICMP
> Ping and LSP Ping, the use of multicast-based ping opens up other types of
> threats. I believe it would be quite useful to articulate how the threats
> described in RFC 6450 do, or don't, relate to this functionality.
>
GIM>> AFAIK, LSP ping is equally applicable to p2p and p2mp LSPs.