Subject: [OSPF] Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 27 Apr 2016 14:11:13 +0100

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The
Routing Directorate seeks to review all routing or routing-related drafts as
they pass through IETF last call and IESG review, and sometimes on special
request. The purpose of the review is to provide assistance to the Routing ADs.
For more information about the Routing Directorate, please see 
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir 

Although these comments are primarily for the use of the Routing ADs, it
would be helpful if you could consider them before or along with any IETF
Last Call comments that you receive, and strive to resolve them through
discussion or by updating the draft. 

Document: draft-ietf-ospf-sbfd-discriminator-04.txt
 Reviewer: Adrian Farrel 
 Review Date: 27 April 2016
 IETF LC End Date: 26 April 2016 
 Intended Status: Standards Track

Summary: 
I have some minor concerns about this document that I think should be
resolved before publication. 

Comments: 

This is a simple document that doesn't require much to implement or 
understand.  It was disappointing, however, to find a large number of
small issues and nits.  I don't believe any of these are blocking to
the utility of the document and if it went for publication in its
current state it would not be harmful.  But in the interest of making 
our documents useful and accessible, and for the purpose of eliminating
all possible interoperability and deployment, I think it would be 
valuable to clean up the issues I have listed.

Major Issues: 
No major issues found.

Minor Issues: 

I should like to see some small amount of text on the scaling impact on
OSPF. 1. How much additional information will implementations have to 
store per node/link in the network? 2. What is the expected churn in 
LSAs introduced by this mechanism (especially when the Reflector is
turned on and off)?

In the second case there is a security implication as well. Can I DoS 
the routing system by toggling some BFD Reflectors? Needs text!

You *do* have...
   A change in information in the S-BFD Discriminator TLV MUST NOT
   trigger any SPF computation at a receiving router.
...which is a help.

---

Section 1 has

   This is achieved by using unique
   network-wide discriminators to identify the Network Targets (e.g., IP
   addresses).

You may be aware of IPv6 :-)

Although 2.1 gives some hints on the size of a discriminator, I had to
go back to 5880 to check that *all* discriminators are exactly 4 octets.
So saying "e.g., IP addresses" is at best confusing.

BTW, draft-ietf-bfd-seamless-base and draft-ietf-bfd-seamless-ip don't
give any hints on this.

Oh, and what is "network-wide"?

I suggest...

   This is achieved by using four-octet discriminators as defined in
   [RFC5880] to identify the Network Targets.

---

In Section 2 you have
   Upon receipt of the TLV, a
   router may decide to ignore this TLV or install the S-BFD
   discriminator in BFD Target Identifier Table.

I think "ignore" is ambiguous. You need to be very clear that "ignore"
means:
- take no local action
- retain the TLV in the opaque LSA
- continue to advertise the opaque LSA according to its scope

In Section 3 you also have
   A router not supporting the S-BFD Discriminator TLV will just
   silently ignore the TLV as specified in [RFC7770].

Am I missing something when I read 7770? I don't find anything about
handling unknown TLVs.

---

Section 2 para 3
s/superset/union/  
("superset" would allow you to include any other discriminators!)

---

Section 2.1
To be totally unambiguous...
OLD
   Length - Total length of the discriminator (Value field) in octets,
   not including the optional padding.  The Length is a multiple of 4
   octets, and consequently specifies how many Discriminators are
   included in the TLV.
NEW
   Length - Total length of all discriminator in the Value field in
   octets, not including the optional padding.  The Length is a multiple
   of 4 octets, and consequently specifies how many Discriminators are
   included in the TLV.
END

However (!) are you sure that you can include optional padding? I think
that 7770 uses padding to take the V field up to a 4 octet boundary.
Since all of your discriminators are exactly a multiple of 4 octets it
seems that there will never be any padding and it would be less 
confusing to write...

NEW
   Length - Total length of all discriminators in the TLV counted in
   octets.  The Length is a multiple of 4 octets, and consequently 
   specifies how many Discriminators are included in the TLV.
END

---

At the end of section 2.1 you have

   S-BFD discriminator is associated with the
   BFD Target Identifier type, that allows demultiplexing to a specific
   task or service.

This is a wonderfully throw-away statement with no context and no
further explanation in the document that I could find. Maybe this is 
just missing a reference to another document, or maybe it needs some
clarification.

---

Section 2.2 has

   The flooding scope for S-BFD Discriminator information advertised
   through OSPF can be limited to one or more OSPF areas, or can be
   extended across the entire OSPF routing domain.

   Note that the S-BFD session may be required to pan multiple areas, in
   which case the flooding scope may comprise these areas.  This could
   be the case for an ABR, for instance, advertising the S-BFD
   Discriminator information within the backbone area and/or a subset of
   its attached IGP area(s).

As I understand flooding scope the options for Opaque LSAs (see 5250)
are:

   o  Link-state type-9 denotes a link-local scope.

   o  Link-state type-10 denotes an area-local scope.

   o  Link-state type-11 denotes that the LSA is flooded throughout the
      Autonomous System (AS).

Your text seems to imply something different. In particular, you seem to
be suggesting that I can have a scope that is greater than one area but
less than the whole AS (assuming "whole AS" == "entire OSPF routing 
domain").

This needs re-writing to clarify what you want to achieve and to bring
it in line with 5250.

Note that the 4th para of Section 2.2 seems to have this right.

===
                                  
Nits

Has Trilok's affiliation changed?
--
Capitalise the document title
---
Expand acronyms in the Abstract if they do not appear with an asterisk 
in http://www.rfc-editor.org/materials/abbrev.expansion.txt
---
Throughout the text, expand acronyms on first use if they do not appear
within http://www.rfc-editor.org/materials/abbrev.expansion.txt with an
asterisk.
---
Decide whether "discriminator" or "Discriminator"
---
In 2.1 you have
   Value - S-BFD network target discriminator value or values.
But there is no "Value" in the figure.
---
2.2 para 2
s/pan/span/
---
2.2
   In the case of domain-
   wide flooding, i.e., where the originator is sitting in a remote
   area, the mechanism described in section 5 of [RFC5250] should be
   used.
s/should/SHOULD/?
But if you mean should or SHOULD (not MUST), what are the exception 
cases?
---

Thanks,
Adrian

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Wed, 27 Apr 2016 13:48:37 +0000

Hi Adrian, 

Thanks for the thorough review. See one inline.

>- continue to advertise the opaque LSA according to its scope

Since the content of opaque LSAs are, in fact, opaque to the OSPF
protocol, implementations should not modify the contents or filter the
LSA. 

Thanks,
Acee

>_______________________________________________
>OSPF mailing list
>[email-masked]
>https://www.ietf.org/mailman/listinfo/ospf

---

From: Alia Atlas <[email-masked]>
Date: Wed, 27 Apr 2016 12:19:20 -0400

Adrian,

Thank you very much for the thorough review!

Authors & Shepherd,

Please go through and update this draft ASAP.  It is on the IESG telechat
on May 5
and is/will be reviewed very soon.

Thanks,
Alia

---

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Wed, 27 Apr 2016 18:49:13 +0000

Hi, Adrian,

Many thanks for the thorough review! Please see inline. Will post a new revision momentarily.

> On Apr 27, 2016, at 9:11 AM, Adrian Farrel <[email-masked]> wrote:
> 
> Hello,
> 

I don’t disagree with this comment theoretically, but at the same time, trying to find the right words… if any. We talk about SPF computation churn as you mention, but documents don’t describe implications of say, changing an IP address on a node. This usage is not too dissimilar from other RI uses, and I could not find scaling text to contrast against either.

There is no reason for S-BFD discriminators to change — frequently, or at all.

The information that nodes may store is the discriminators, or they may choose to not store it. An S-BFD reflector is not expected to be constantly configured on/off/on/off, and if its configuration state is toggled continuously maliciously, there’s really a lot worst things than this extra advertisement.

Most of that does not seem to be needed explicitly in the document.

So, net-net, I do not see the need to update this.

>   network-wide discriminators to identify the Network Targets (e.g., IP

Yes, this can be improved, see below.

> BTW, draft-ietf-bfd-seamless-base and draft-ietf-bfd-seamless-ip don't

https://tools.ietf.org/html/draft-ietf-bfd-seamless-base-09#section-2 <https://tools.ietf.org/html/draft-ietf-bfd-seamless-base-09#section-2>
“
   o  S-BFD discriminator - a BFD discriminator allocated for a local
      entity and is being listened by an SBFDReflector.
"

>   [RFC5880] to identify the Network Targets.

This suggestion drops a key-most consideration, “unique”.
https://tools.ietf.org/html/draft-ietf-bfd-seamless-base-09#section-4.1 <https://tools.ietf.org/html/draft-ietf-bfd-seamless-base-09#section-4.1>

I’ll reword to:

  This is achieved by using four-octet discriminators, unique within
  an administrative domain, to identify the Network Targets.

> - continue to advertise the opaque LSA according to its scope

The “ignore” is in reference of what follows, which is install the discriminator. In this case, ignore means ‘or don’t install it’.

I agree this can be improved :-) I’ll just remove “to ignore this TLV or”, since may install it (or may not).

> Am I missing something when I read 7770? I don't find anything about

https://tools.ietf.org/html/rfc7770#section-2.3 <https://tools.ietf.org/html/rfc7770#section-2.3>
"
   Unrecognized types are ignored.
"

> ("superset" would allow you to include any other discriminators!)

OK.

>   specifies how many Discriminators are included in the TLV.

Indeed, and we had fixed this one in our working copy. This is what we have:

<t>
Length - Total length of the discriminator(s) that appear in the
Value field, in octets. Each discriminator is 4 octets, so the Length
is 4 times the number of discriminators included in the TLV. There is
no optional padding for this field.
</t>

> just missing a reference to another document, or maybe it needs some

Old, and should have been removed. Good catch!!! Removed.

> Note that the 4th para of Section 2.2 seems to have this right.

You are right. Seems like removing the first two paragraphs of that section suffice to remove the problem, and keep the goal in a concise form.

> Nits

All nits incorporated — thanks! With only one comment below.

> Has Trilok's affiliation changed?

Updated.

The intention is not to close the door for future exception cases, without listing them here necessarily.

I am personally OK with should or SHOULD, and tempted to leave “should”, but happy to take guidance on this.

Thanks,

— Carlos.

---

From: Manav Bhatia <[email-masked]>
Date: Thu, 28 Apr 2016 11:01:50 +0530

Hi Adrian,

Thanks for the extensive review. I have a minor comment on a minor issue
that you raised.

> LSAs introduced by this mechanism (especially when the Reflector is

Isnt this implementation specific? This is what will differentiate one
vendor implementation from the other.

I am not sure how we can quantify this -- any ideas?

This is akin to saying that IS-IS, in contrast to OSPFv2, is more attuned
for partial SPF runs because the node information is cleanly separated from
the reachability information. However, this isnt entirely true. While i
concede that node information is mixed with prefix information in OSPFv2,
there still are ways in which clever implementations could separate the two
and do exactly what IS-IS does.

I took this rather circuitous approach to drive home the point that
scalability, churn, overheads on the system are in many cases dependent on
the protocol implementation and by that token outside the scope of the IETF
drafts.

>    trigger any SPF computation at a receiving router.

I would be alarmed if an implementation in an absence of this pedantic note
triggered SPF runs each time an S-BFD disc changed ! I mean if you
understand the idea being discussed then you also understand that a change
in this TLV has no bearing on the reachability anywhere. And that knowledge
should be enough to prevent SPF runs in most cases !

I know that we have added this note but if we need to explicitly spell such
things out in all standards then we clearly have bigger problems ! :-)

Cheers, Manav

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Thu, 28 Apr 2016 06:32:39 +0000

Adrian –

As an interested bystander (given I am co-author on the companion IS-IS S-BFD draft) I share the concerns expressed by Carlos and Manav.

Churning S-BFD discriminator assignments is about as likely as churning IP/IPv6 address assignments on a node – it is simply not something that any deployment would be likely to do.
IGP drafts pay close attention to churn for advertisements of information which is expected to be dynamic - https://datatracker.ietf.org/doc/draft-ietf-isis-te-metric-extensions/ is a good example of this. But there is no reason to expect a similar issue with S-BFD discriminators. Therefore, for the same reasons that base protocol specifications do not discuss the impact of churn in advertising prefix reachability we saw no reason to discuss it in the context of advertising S-BFD discriminators.

It would be helpful if you provided some context as to  why you have raised this point.
Thanx.

   Les

From: rtg-dir [mailto:[email-masked]] On Behalf Of Manav Bhatia
Sent: Wednesday, April 27, 2016 10:32 PM
To: Adrian Farrel
Cc: <[email-masked]>; [email-masked]; [email-masked]; OSPF WG List
Subject: Re: [RTG-DIR] Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt

Hi Adrian,

Thanks for the extensive review. I have a minor comment on a minor issue that you raised.

Minor Issues:

Isnt this implementation specific? This is what will differentiate one vendor implementation from the other.

This is akin to saying that IS-IS, in contrast to OSPFv2, is more attuned for partial SPF runs because the node information is cleanly separated from the reachability information. However, this isnt entirely true. While i concede that node information is mixed with prefix information in OSPFv2, there still are ways in which clever implementations could separate the two and do exactly what IS-IS does.

I took this rather circuitous approach to drive home the point that scalability, churn, overheads on the system are in many cases dependent on the protocol implementation and by that token outside the scope of the IETF drafts.

You *do* have...

I would be alarmed if an implementation in an absence of this pedantic note triggered SPF runs each time an S-BFD disc changed ! I mean if you understand the idea being discussed then you also understand that a change in this TLV has no bearing on the reachability anywhere. And that knowledge should be enough to prevent SPF runs in most cases !

I know that we have added this note but if we need to explicitly spell such things out in all standards then we clearly have bigger problems ! :-)

Cheers, Manav

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Thu, 28 Apr 2016 09:20:54 +0000

Hi Manav,

From: Manav Bhatia <[email-masked]<mailto:[email-masked]>>
Date: Thursday, April 28, 2016 at 1:31 AM
To: Adrian Farrel <[email-masked]<mailto:[email-masked]>>
Cc: "<[email-masked]<mailto:[email-masked]>>" <[email-masked]<mailto:[email-masked]>>, Routing Directorate <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, OSPF WG List <[email-masked]<mailto:[email-masked]>>
Subject: Re: Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt

Hi Adrian,

Minor Issues:

I believe what is being requested is a discussion of how often the S-BFD TLV is likely to change, the effects on flooding, and, if required, recommendations for any rate-limiting or other measures to prevent churn.

Thanks,
Acee

You *do* have...

Cheers, Manav

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Thu, 28 Apr 2016 09:25:08 +0000

Hi Les,

From: "Les Ginsberg (ginsberg)" <[email-masked]<mailto:[email-masked]>>
Date: Thursday, April 28, 2016 at 2:32 AM
To: Manav Bhatia <[email-masked]<mailto:[email-masked]>>, Adrian Farrel <[email-masked]<mailto:[email-masked]>>
Cc: "<[email-masked]<mailto:[email-masked]>>" <[email-masked]<mailto:[email-masked]>>, Routing Directorate <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, OSPF WG List <[email-masked]<mailto:[email-masked]>>
Subject: RE: [RTG-DIR] Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt

Adrian –

Explaining why S-BFD churn is not likely a concern and that area or AS flooding will not be impacted would probably satisfy the comment. However, I won’t speak for Adrian.

Thanks,
Acee

   Les

From: rtg-dir [mailto:[email-masked]] On Behalf Of Manav Bhatia
Sent: Wednesday, April 27, 2016 10:32 PM
To: Adrian Farrel
Cc: <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>; OSPF WG List
Subject: Re: [RTG-DIR] Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt

Hi Adrian,

Minor Issues:

You *do* have...

Cheers, Manav

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 28 Apr 2016 13:59:57 +0100

Acee has it right.
 
While (of course) stuff can be done in implementations to mitigate the effects, the protocol extensions here increase the size of LSA and increase the amount of flooding. Since the LSAs have to be stored (in some form), it is reasonable to describe the amount of extra information that reflects across a network - maybe express it as "LSA data" and leave it up to an implementation to choose how to store it. Since the number of LSA updates impacts the routing plane processing and bits on the wire, it is reasonable to ask what impact that might have.
 
I am interested to hear whether turning Reflectors on and off could be a feature that could cause LSAs to flap and so create flooding ripples in the network.
 
Adrian
 
From: Acee Lindem (acee) [mailto:[email-masked]] 
Sent: 28 April 2016 10:21
To: Manav Bhatia; Adrian Farrel
Cc: <[email-masked]>; [email-masked]; [email-masked]; OSPF WG List
Subject: Re: Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt
 
Hi Manav,
 
From: Manav Bhatia <[email-masked]>
Date: Thursday, April 28, 2016 at 1:31 AM
To: Adrian Farrel <[email-masked]>
Cc: "<[email-masked]>" <[email-masked]>, Routing Directorate <[email-masked]>, "[email-masked]" <[email-masked]>, OSPF WG List <[email-masked]>
Subject: Re: Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt
 
Hi Adrian,

Minor Issues:

 

 
 
Thanks,
Acee

You *do* have...

 
Cheers, Manav

---

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Thu, 28 Apr 2016 16:15:06 +0000

Adrian,

I would not oppose to making a clarification similar to the following, if the WG things its useful:

The S-BFD Discriminators are expected to be quite static. S-BFD Discriminators may change when enabling the S-BFD functionality or via an explicit configuration event. These will result in a change in the information advertised in the S-BFD Discriminator TLV in OSPF, but are not expected to happen with any regularity.

[I expect that text needs (a lot of) wordsmithing, and might not be useful or desired at all, but just to make the discussion more real]

Thanks,

— Carlos.

> On Apr 28, 2016, at 8:59 AM, Adrian Farrel <[email-masked]> wrote:
> 
> Acee has it right.
> 
> From: Manav Bhatia <[email-masked] <mailto:[email-masked]>>
> To: Adrian Farrel <[email-masked] <mailto:[email-masked]>>
> Cc: "<[email-masked] <mailto:[email-masked]>>" <[email-masked] <mailto:[email-masked]>>, Routing Directorate <[email-masked] <mailto:[email-masked]>>, "[email-masked] <mailto:[email-masked]>" <[email-masked] <mailto:[email-masked]>>, OSPF WG List <[email-masked] <mailto:[email-masked]>>

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Thu, 28 Apr 2016 16:29:43 +0000

Sorry, while I respect you both I don’t agree.

Base protocol specifications have controls on the rate of generation of LSAs – those apply here as they do to all protocol advertisements.

A “BFD Reflector” is defined in S-BFD Base draft as:

“SBFDReflector - an S-BFD session on a network node that listens
      for incoming S-BFD control packets to local entities and generates
      response S-BFD control packets.”

As far as advertisements of S-BFD discriminators it would not matter whether the Reflector flaps – it would require a change in the assignment of S-BFD Discriminator on that node – which is as likely as reconfiguration of a node address.
Please  explain why this rare event represents something which is of concern to the operation of an IGP.

I do not like cluttering normative specifications with discussions of points that do not reflect real operational concerns – so the argument “what harm could it do to discuss this” doesn’t carry much weight with me.

   Les

From: rtg-dir [mailto:[email-masked]] On Behalf Of Adrian Farrel
Sent: Thursday, April 28, 2016 6:00 AM
To: Acee Lindem (acee); 'Manav Bhatia'; 'Adrian Farrel'
Cc: [email-masked]; [email-masked]; [email-masked]; 'OSPF WG List'
Subject: Re: [RTG-DIR] Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt

Acee has it right.

Adrian

Hi Manav,

Hi Adrian,

Minor Issues:

Thanks,
Acee

You *do* have...

Cheers, Manav