Subject: [Idr] Tsvart last call review of draft-ietf-idr-te-pm-bgp-15

From: Yoshifumi Nishida <[email-masked]>
Date: Thu, 13 Dec 2018 01:05:04 -0800

Reviewer: Yoshifumi Nishida
Review result: Ready with Nits

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.

Summary: Ready with Nits

1: The TLV formats in the draft look identical with RFC7471 except the value in
Type field.
     it would be better to clarify this points so that the readers who are
     familiar with RFC7471 can interpret them easily. I am also wondering if
     the format figures of TLV are necessary when the same figures are already
     presented in RFC7471.

2: There is no guidance for default values such as measurement interval in the
draft. If these values should also be inherited from other draft, it should be
stated.

3: (Editorial) The length of Type filed in the figures look 15 bits length.
But, I believe it should be 16 bits.

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Thu, 13 Dec 2018 18:13:02 +0000

Yoshi -

Thanx for the review.
Replies inline.

> -----Original Message-----
> From: Yoshifumi Nishida <[email-masked]>
> Sent: Thursday, December 13, 2018 1:05 AM
> To: [email-masked]
> Cc: [email-masked]; [email-masked]; [email-masked]
> Subject: Tsvart last call review of draft-ietf-idr-te-pm-bgp-15
> 

[Les:] The draft says:

Section 2

" TLV formats follow the rules defined in [RFC7752]."

Then in each subsequent sub-section 2.x  both RFC 7471 and draft-ietf-lsr-isis-rfc7810bis are explicitly referenced.

The TLV formats need to be presented here since these are the BGP-LS encodings, which are similar to but NOT identical to the IGP specific encodings (for example IS-IS encoding uses an 8-bit type/length).

> draft. If these values should also be inherited from other draft, it should be
[Les:] This is not within the purview of this draft. All this draft is doing is defining the encodings for the BGP-LS advertisements which are essentially copies of what the IGPs are advertising.

> But, I believe it should be 16 bits.

[Les:] Indeed. Good catch. I will fix this.

   Les

---

From: Yoshifumi Nishida <[email-masked]>
Date: Thu, 13 Dec 2018 11:15:44 -0800

Hi Les,

On Thu, Dec 13, 2018 at 10:13 AM Les Ginsberg (ginsberg) <[email-masked]>
wrote:

> Then in each subsequent sub-section 2.x  both RFC 7471 and
> draft-ietf-lsr-isis-rfc7810bis are explicitly referenced.
>
> The TLV formats need to be presented here since these are the BGP-LS
> encodings, which are similar to but NOT identical to the IGP specific
> encodings (for example IS-IS encoding uses an 8-bit type/length).
>

Right. But, but in case of OSPF, I think the format will be identical
except the type field value.
I just thought if you add one sentence to clarify the formats happen to be
the same as 7471, it could be a small benefit for OSPF users.
But, this is not a strong opinion.

> > 2: There is no guidance for default values such as measurement interval
> in
> > the
> [Les:] This is not within the purview of this draft. All this draft is
> doing is defining the encodings for the BGP-LS advertisements which are
> essentially copies of what the IGPs are advertising.
>

Does it mean when an IGP advertises these metrics at 10 sec interval, the
BGP-LS will be advertised at the same interval?

Thanks,
--
Yoshi

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Thu, 13 Dec 2018 21:56:57 +0000

Yoshi –

Inline.

From: Yoshifumi Nishida <[email-masked]>
Sent: Thursday, December 13, 2018 11:16 AM
To: Les Ginsberg (ginsberg) <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: Tsvart last call review of draft-ietf-idr-te-pm-bgp-15

Hi Les,
On Thu, Dec 13, 2018 at 10:13 AM Les Ginsberg (ginsberg) <[email-masked]<mailto:[email-masked]>> wrote:
Yoshi -

Thanx for the review.
Replies inline.

> From: Yoshifumi Nishida <[email-masked]<mailto:[email-masked]>>
> To: [email-masked]<mailto:[email-masked]>
> Cc: [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>
> [email-masked]<mailto:[email-masked]> if you reply to or forward this review.
>
> Summary: Ready with Nits
>

[Les:] The draft says:

Section 2

Right. But, but in case of OSPF, I think the format will be identical except the type field value.
I just thought if you add one sentence to clarify the formats happen to be the same as 7471, it could be a small benefit for OSPF users.
But, this is not a strong opinion.

[Les:] The format follows RFC7752. It just happens in this case the format for these TLVs matches OSPF – that isn’t always the case. As a practical matter I don’t think it helps implementers to point out when BGP-LS TLVs are identical to protocol X or protocol Y.

Does it mean when an IGP advertises these metrics at 10 sec interval, the BGP-LS will be advertised at the same interval?

[Les:] The IGP RFCs provide significant detail on the importance of controlling the rate of change of advertisements.
Given that, yes – I would expect that updates are provided to BGP whenever the IGP advertisements change.
BGP implementations typically have their own knobs to control the frequency of BGP updates – but given the filtering done at the source I would hope implementations have no need for additional dampening in BGP itself – but that is an implementation choice.

  Les

Thanks,
--
Yoshi

---

From: "Susan Hares" <[email-masked]>
Date: Fri, 14 Dec 2018 09:49:18 -0500

Les and Yoshi:

You both raised two sides of the issue regarding the BGP-LS TLVs when these TLVs align with IGP TLVs. 

Side 1: It is helpful to note that the TLVs are the same

Side 2: It is confusing to have this information in individual drafts.    

This is not a single draft issue but a concern regarding several drafts in the BGP-LS series from IDR.

Yoshi -  Would it help the clarity of the BGP-LS series to have an explanatory draft that indicates the places to find the alignment?  Or should we suggest a note to IANA registration to provide clarity.  

Thank you for raising both sides of this issue.   Your IDR chairs and AD have been discussing how to make this easier to understand for implementers and those who deploy the BGP-LS code.  

Susan Hares 

From: Les Ginsberg (ginsberg) [mailto:[email-masked]] 
Sent: Thursday, December 13, 2018 4:57 PM
To: Yoshifumi Nishida
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: RE: Tsvart last call review of draft-ietf-idr-te-pm-bgp-15

Yoshi –

Inline.

Hi Les,

On Thu, Dec 13, 2018 at 10:13 AM Les Ginsberg (ginsberg) <[email-masked]> wrote:

Yoshi -

Thanx for the review.
Replies inline.

[Les:] The draft says:

Section 2

  Les

Thanks,

--

Yoshi

---

From: Alvaro Retana <[email-masked]>
Date: Fri, 14 Dec 2018 09:54:58 -0800

On December 14, 2018 at 9:49:28 AM, Susan Hares ([email-masked]) wrote:

Sue:

Hi!

I think we might be talking about two different things…or I got lost, which
is completely possible. :-)

I think Yoshi is asking about the format of the TLVs, which happens to be
the same as the OSPF ones (rfc7471), but slightly different than ISIS
(rfc7810bis): the type and length fields are different.  I had already
pointed this out in my AD review, so the text changed from "The semantic of
the TLV is described in [RFC7810] and [RFC7471]” (in -14) to "The semantics
of the value field in the TLV are described in
[I-D.ietf-lsr-isis-rfc7810bis] and [RFC7471]” (in -15)…and the text about
"TLV formats follow the rules defined in [RFC7752]” was added, as Les
pointed out.  I think this is enough.

When you say "align with IGP TLVs”, are you talking about the format of the
TLV, or are you referring to something else?

In some BGP-LS document I’ve seen tables that map the BGP-LS TLVs to the
corresponding ISIS/OSPF TLVs — with the understanding that the format is
different.  One such example is in §2.4/2.5
of draft-ietf-idr-bgp-ls-segment-routing-ext [1].  Is that the alignment
you’re asking about?  If so, then I think that such a table would be useful.

Thanks!

Alvaro.

[1]
https://tools.ietf.org/html/draft-ietf-idr-bgp-ls-segment-routing-ext-11#section-2.4

You both raised two sides of the issue regarding the BGP-LS TLVs when these
TLVs align with IGP TLVs.

This is not a single draft issue but a concern regarding several drafts in
the BGP-LS series from IDR.

Yoshi -  Would it help the clarity of the BGP-LS series to have an
explanatory draft that indicates the places to find the alignment?  Or
should we suggest a note to IANA registration to provide clarity.

Thank you for raising both sides of this issue.   Your IDR chairs and AD
have been discussing how to make this easier to understand for implementers
and those who deploy the BGP-LS code.

---

From: "Susan Hares" <[email-masked]>
Date: Fri, 14 Dec 2018 17:36:28 -0500

Alvaro: 

Yes – this is the format I was indicating – 

In some BGP-LS document I’ve seen tables that map the BGP-LS TLVs to the corresponding ISIS/OSPF TLVs — with the understanding that the format is different.  One such example is in §2.4/2.5 of draft-ietf-idr-bgp-ls-segment-routing-ext [1].  Is that the alignment you’re asking about?  If so, then I think that such a table would be useful.

This makes the implementations easier to create.  More importantly helps those who deploy the protocol additions to understand how to debug issues.   Having started out my Internet career as a OPS person, I like to see things that make life easier for the OPS/NM people.  We can comments in each draft or put pointers in a single draft (living or RFC).  … just wondering what would help.. 

I agree with your ADs comment on the text – that said 

“The semantic of the TLV is described in [RFC7810] and [RFC7471]” (in -14) to "The semantics of the value field in the TLV are described in [I-D.ietf-lsr-isis-rfc7810bis] and [RFC7471]” (in -15)…and the text about "TLV formats follow the rules defined in [RFC7752]” was added, as Les pointed out.  

Your comment makes the text correct. Thanks for catching that point. I thought Les already agreed to that change – so I did “ack” it or expand on it.   

Sue 

From: Alvaro Retana [mailto:[email-masked]] 
Sent: Friday, December 14, 2018 12:55 PM
To: Susan Hares; Les Ginsberg (ginsberg); Yoshifumi Nishida
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: RE: Tsvart last call review of draft-ietf-idr-te-pm-bgp-15

Sue:

Hi!

I think we might be talking about two different things…or I got lost, which is completely possible. :-)

I think Yoshi is asking about the format of the TLVs, which happens to be the same as the OSPF ones (rfc7471), but slightly different than ISIS (rfc7810bis): the type and length fields are different.  I had already pointed this out in my AD review, so the text changed from "The semantic of the TLV is described in [RFC7810] and [RFC7471]” (in -14) to "The semantics of the value field in the TLV are described in [I-D.ietf-lsr-isis-rfc7810bis] and [RFC7471]” (in -15)…and the text about "TLV formats follow the rules defined in [RFC7752]” was added, as Les pointed out.  I think this is enough.

When you say "align with IGP TLVs”, are you talking about the format of the TLV, or are you referring to something else?

In some BGP-LS document I’ve seen tables that map the BGP-LS TLVs to the corresponding ISIS/OSPF TLVs — with the understanding that the format is different.  One such example is in §2.4/2.5 of draft-ietf-idr-bgp-ls-segment-routing-ext [1].  Is that the alignment you’re asking about?  If so, then I think that such a table would be useful.

Thanks!

Alvaro.

[1] https://tools.ietf.org/html/draft-ietf-idr-bgp-ls-segment-routing-ext-11#section-2.4

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Fri, 14 Dec 2018 22:42:52 +0000

Alvaro (and everyone) - -

I have posted V17 which includes the mapping table suggested by Alvaro.

I apologize in advance – but I find the mindset here flawed.

The names given to the parameters are identical in the BGP-LS draft to the ones used in the referenced IGP drafts. I would think that is sufficient to unambiguously identify the relationship between the two. And it has the advantage that it is a symbolic reference – which means that if (however unlikely) the actual codepoint for one of the IGP advertisements were to change, that instead of being able to address this by updating one document – we now have to update both the referencing document(s) and the referenced document. And why do we do this? Because we think that someone who is actually interested in the topic enough to look at both documents won’t be able to easily map between the two???

Sorry, I am shaking my head at this one…and I only take the time to say this in the hope that the collective mindset will change for future documents.

But it is done.

   Les

From: Alvaro Retana <[email-masked]>
Sent: Friday, December 14, 2018 9:55 AM
To: Susan Hares <[email-masked]>; Les Ginsberg (ginsberg) <[email-masked]>; Yoshifumi Nishida <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: RE: Tsvart last call review of draft-ietf-idr-te-pm-bgp-15

On December 14, 2018 at 9:49:28 AM, Susan Hares ([email-masked]<mailto:[email-masked]>) wrote:

Sue:

Hi!

Thanks!

Alvaro.