Re: [Idr] shepherd review of draft-ietf-idr-bgp-ls-segment-routing-ext-08.txt

["Ketan Talaulikar (ketant)" <[e-mail]>]

Hi Sue,

Many thanks for your detailed review and your suggestions for fixing issues and improving the document.

Please check inline for responses.

From: Susan Hares <[e-mail]>
Sent: 17 June 2018 18:05
To: [e-mail]
Cc: [e-mail]; 'John Scudder' <[e-mail]>; 'Alvaro Retana' <[e-mail]>; Ketan Talaulikar (ketant) <[e-mail]>
Subject: shepherd review of draft-ietf-idr-bgp-ls-segment-routing-ext-08.txt

This is a shepherds review after WG LC, RTG-DIR QA review, and OPS QA Review.

Summary: I have minor issues and editorial comments on this document. The minor issues will need to be resolved before this document is sent to the IESG. The minors issues arise out of the shepherd's review of the document and the two QA Reviews.


Thanks:
I want to thank the WG for its active review during WG LC, and following people who provided specific early reviews on this document:

1)  Robert Raszuk and Tony Li - during WG LC

2)  Jeff Hass - during WG LC

3)  Aijun Wang - during WG LC

4)  Victoria Pritchard - RTG-DIR QA review

5)  Joel Jaeggli - OPS QA review

I want to thank the authors for a well written draft and productive dialogues with the reviewers.

Susan Hares


----------

Minor issues:

Source: Shepherd
Issue 1: BGP Error handling
[KT] The issue that you raise related to NLRI content errors and restrictions on which TLVs are allowed for which of the BGP-LS NLRIs is a generic topic to be addressed for BGP-LS. This is being discussed also on parallel threads. I will get back on the actions/updates on this specific document on this specific issue.

Problem:
The BGP Error handling in the document has two types:  malformed TLVs and restrictions on the TLVs which can be inserted in the Node NLRI (section 2.1), Link NLRI (Section 2.2), and Prefix NLRI (section 2.3).  The error handling in RFC5572 does not specifically handle the restrictions on TLVs.

Suggested solution:
Denote these errors as NLRI content errors and specifically indicate how these are treated.

Textual changes:
Section 2.1 on page 6, just before 2.1.1 paragraph
New/
If a BGP Peer supporting BGP-LS incorrectly adds these TLVs to the Link Attribute or the Prefix attribute of the BGP-LS NLRI, this a   content error rather than a malformed NLRI.  See the manageability     section for an explanation of how content errors are handled.
/
Section 2.2 on page 10, after table 2 and the paragraph that ends "TLV/Sub-TLV described below."

New/
If a BGP Peer supporting BGP-LS incorrectly adds these TLVs to the Node Attribute or the Prefix attribute of the BGP-LS NLRI, this a   content error rather than a malformed NLRI.  See the manageability     section for an explanation of how content errors are handled.
/

Section 2.3 on page 14, just before 2.3.1

New
/If a BGP Peer supporting BGP-LS incorrectly adds these TLVs to the Node Attribute or the Link attribute of the BGP-LS NLRI, this content error rather than a malformed NLRI.  See the manageability     section for an explanation of how content errors are handled.
/

Replace the 3rd sentence of the second paragraph of 5. Manageability Considerations

Old/

Specifically the determination of malformed attributes and their

handling follow the base BGP-LS specification [RFC7752<https://tools.ietf.org/html/rfc7752>].
/
New /
Specifically, the malformed NLRIs attribute tests
in the FAULT Management section of [RFC7752<https://tools.ietf.org/html/rfc7752>]
now encompass the new TLVs for the BGP-LS NLRI in this document.

Three content restrictions for TLV insertion are given in sections 2.1, 2.2, and 2.3 to which limit the inclusion of TLVs to certain BGP-LS NLRI attributes. Any BGP-LS NLRI which does not abide by the content restriction will be treated as a malformed BGP-LS NLRI.

If BGP-LS NLRI with these extensions is determined to be malformed
by the receiving BGP peer, the BGP peer receiving the BGP packet
drops the attribute the malformed BGP-LS NLRI was detect in  (either the BGP_MP_REACH_NLRI attribute or the BGP_MP_UNREACH_NLRI attribute).
/

Issue 2:
Source: Shepherd:
Problem:
section 2.1.3 does not indicate where the values come for the 1 octet identifying the algorithm.  If these are configured on a network basis, please identify this point. If these are part of an ISIS or OSPF standard please specify.
[KT] Thanks for this catch. Will fix this.

Issue 3:
Source: Joel Jaeggli's OPS-DIR review
Problem:  None of the review points from the OPS-DIR QA review have been addressed
Recommended solution: The authors should consider these suggestions and address his 3 points.
[KT] Please find attached emails where I have responded to all of Joel's comments and made modifications in version 7 of the document to update. Do let know specifically which point you feel is not addressed.

Issue 4:
Source: Victoria Pritchard QA review concern
Problem: RTG-QA review issue 7
2.1.5
The SRMS Preference TLV only contains the Preference value, so it was
unclear how this is linked to a mapping server? Is the mapping server some other field in the message?

[KT response] The SRMS Preference TLV indicates the preference value for the node NLRI that it is a part of when that node is a mapping server. One would need to go through the referenced IGPs specs and likely the relevant SR specs to understand the mapping server.

[Shepherd's comment: text in draft-ietf-spring-segment-routing-ldp-interop states:
   We introduce the definition of the Segment Routing Mapping Server
   (SRMS) which consists of an IGP (IS-IS, OSPF and OSPFv3) extension
   allowing an SR capable router to advertise mappings between prefixes
   and Segment Identifiers (SID).  The details on how the mappings are
   encoded and advertised are protocol specific and defined in
   [I-D.ietf-isis-segment-routing-extensions<https://tools.ietf.org/html/draft-ietf-spring-segment-routing-ldp-interop-08#ref-I-D.ietf-isis-segment-routing-extensions>],
   [I-D.ietf-ospf-segment-routing-extensions<https://tools.ietf.org/html/draft-ietf-spring-segment-routing-ldp-interop-08#ref-I-D.ietf-ospf-segment-routing-extensions>] and
   [I-D.ietf-ospf-ospfv3-segment-routing-extensions<https://tools.ietf.org/html/draft-ietf-spring-segment-routing-ldp-interop-08#ref-I-D.ietf-ospf-ospfv3-segment-routing-extensions>].

   The SRMS function of a SR capable router allows distribution of
   mappings for prefixes not locally attached to the advertising router
   and therefore allows advertisement of mappings on behalf of non-SR
   capable routers.

I suggest this definition moves up to the segment routing draft (draft-ietf-spring-segment-routing-16), then draft-ietf-idr-bgp-ls-segment-routing-ext and draft-ietf-spring-segment-routing-ldp-interop can utilize the draft.  In the meantime, put a similar definition in this draft in section with a comment that states it will disappear.
[KT] I do not believe the SRMS concept belongs to the draft-ietf-spring-segment-routing and it has been recently elaborated (including SRMS Preference aspects) in the v13 of the draft-ietf-spring-segment-routing-ldp-interop. I will update the text in the draft to point to this reference.

--------------------------

Editorial issues

Issue 1: explaining what capability architecture

Source: from RTG-QA issue 5  (editorial)
(5)2.1.2. The capabilities TLV "advertise the node's SR capabilities and its Segment Routing Global Base (SRGB) range(s)", but the format diagram only contains Segment ID information. What are the capabilities? Or does the presence of this TLV indicate the capability?

[KT Response] The Capabilities are the SRGB and the flags carried in the TLV.
Shepherd: A short explanation on why this is considered "capability" is in order here.
[KT] Sure. I will do this.
Thanks,
Ketan

--- Begin Message ---

Hi Joel,

A new version 07 of this draft has been just posted to address the comments from your review as discussed in the mail thread below.

https://datatracker.ietf.org/doc/draft-ietf-idr-bgp-ls-segment-routing-ext/

Thanks,
Ketan

-----Original Message-----
From: Les Ginsberg (ginsberg)
Sent: 09 May 2018 20:06
To: Ketan Talaulikar (ketant) <[e-mail]>; Joel Jaeggli <[e-mail]>; [e-mail]
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: RE: Opsdir early review of draft-ietf-idr-bgp-ls-segment-routing-ext-06

> 2.3.3.  Source Router Identifier (Source Router-ID) TLV
>
>    The Source Router-ID TLV contains the IPv4 or IPv6 Router-ID of the
>    originator of the Prefix.  For IS-IS protocol this is as defined in
>    [RFC7794].  The Source Router-ID TLV may be used to carry the OSPF
>    Router-ID of the prefix originator.
>
>    The Source Router-ID TLV has the following format:
>
>     0                   1                   2                   3
>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |            Type               |            Length             |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    //                  IPv4/IPv6 Address (Router-ID)              //
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>    where:
>
>       Type: TBD, see Section 4.
>
>       Length: 4 or 16.
>
>       IPv4/IPv6 Address: 4 octet IPv4 address or 16 octet IPv6 address.
>
>    The semantic of the Source Router-ID TLV is defined in [RFC7794].
>
> * While RFC7794 Router-IDs are in fact IP addresses. OSPF Router-IDs
> are not even if they happen to look like them, this is particularly
> germain with ospfv3 but it’s worth making the distinction.
> [KT] Agree. I would say "The semantic of the Source Router-ID TLV for
> ISIS is defined in [RFC7794]. For OSPF, the TLV carries the OSPF
> Router-ID of the originator of the prefix."

[Les:] Joel - your statement is technically correct - but Ketan's suggested text revision is repeating text that is present in the first paragraph of the section.

Since the point being made is in regards to the semantics of the value field would this be more clearly dealt with by  replacing

//                  IPv4/IPv6 Address (Router-ID)              //

With

//       4 or 16 octet Router-ID     //

??

   Les

--- End Message ---

--- Begin Message ---

Hi Joel,

Many thanks for your review and please find responses inline below. I will update the draft with the same based on your feedback.

Thanks,
Ketan

-----Original Message-----
From: Joel Jaeggli <[e-mail]>
Sent: 09 May 2018 12:16
To: [e-mail]
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: Opsdir early review of draft-ietf-idr-bgp-ls-segment-routing-ext-06

Reviewer: Joel Jaeggli
Review result: Has Nits

I have performed a requested early review on the behalf of the operations directorate of the current draft-ietf-idr-bgp-ls-segment-routing-ext-06.
Generally I think this document is good, I won't say ready, as this review is intended as an early review not a final one.

Some nits

1-
introduction

   This way, all BGP speakers (specifically the route-reflectors) obtain
   Link-State information from all IGP areas (and from other ASes from
   EBGP peers).

* BGP speakers are agnostic about the source of the information beyond that it was exported with certain properties from the rib of it’s neighbor.
[KT] I agree this is generally true in case of BGP. However, in this specific case of BGP-LS, the Node, Link and Prefix NLRIs actually contain the identifiers/descriptors which include the source of the information - as in the specific IGP node originating it. So would you agree that in this specific BGP-LS case the statement is valid?

2 -
   An external component (e.g., a controller) then can
   collect SR information in the "northbound" direction across IGP areas
   or ASes and construct the end-to-end path (with its associated SIDs)
   that need to be applied to an incoming packet to achieve the desired
   end-to-end forwarding.

* Unqualified use of the term northbound I find generally problematic, particularly in the case of a route reflector. Previously RFC 7752 manged to use the term in the title and then never again within the text.
[KT] Sure. I would remove the "northbound" term.

3-

2.3.3.  Source Router Identifier (Source Router-ID) TLV

   The Source Router-ID TLV contains the IPv4 or IPv6 Router-ID of the
   originator of the Prefix.  For IS-IS protocol this is as defined in
   [RFC7794].  The Source Router-ID TLV may be used to carry the OSPF
   Router-ID of the prefix originator.

   The Source Router-ID TLV has the following format:

    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |            Type               |            Length             |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   //                  IPv4/IPv6 Address (Router-ID)              //
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   where:

      Type: TBD, see Section 4.

      Length: 4 or 16.

      IPv4/IPv6 Address: 4 octet IPv4 address or 16 octet IPv6 address.

   The semantic of the Source Router-ID TLV is defined in [RFC7794].

* While RFC7794 Router-IDs are in fact IP addresses. OSPF Router-IDs are not even if they happen to look like them, this is particularly germain with ospfv3 but it’s worth making the distinction.
[KT] Agree. I would say "The semantic of the Source Router-ID TLV for ISIS is defined in [RFC7794]. For OSPF, the TLV carries the OSPF Router-ID of the originator of the prefix."

4 -
5.1.1.  Operations

   Existing BGP and BGP-LS operational procedures apply.  No additional
   operation procedures are defined in this document.

* An informative or normative reference (probably to 7752 especially fault
mangement) is probably required.
[KT] Agree and I would specifically put the normative reference to RFC7752.

--- End Message ---