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

"Carlos Pignataro (cpignata)" <cpignata@cisco.com> Wed, 27 April 2016 18:49 UTC

Return-Path: <cpignata@cisco.com>
X-Original-To: ospf@ietfa.amsl.com
Delivered-To: ospf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 30B8112D53F; Wed, 27 Apr 2016 11:49:20 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -15.516
X-Spam-Level:
X-Spam-Status: No, score=-15.516 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H4=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-0.996, SPF_PASS=-0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id sbUMGe_dQAov; Wed, 27 Apr 2016 11:49:16 -0700 (PDT)
Received: from alln-iport-3.cisco.com (alln-iport-3.cisco.com [173.37.142.90]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 7524312D534; Wed, 27 Apr 2016 11:49:16 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=31012; q=dns/txt; s=iport; t=1461782956; x=1462992556; h=from:to:cc:subject:date:message-id:references: in-reply-to:mime-version; bh=rH24F/kBDiEwJXdDDM3ffRIc6/IH5dKMjNratTorZLI=; b=afwuHSCejYYQ/lbSlZfFYnByT30RgY5t3BAEANpg3kRUooB2uNPpex8N KCyAAoD1vbMTyEsLdiJOorTAuG0w5QEWmqaR2dKkbObFvJymQ9bRbgCb1 VUT8kSrma7qekv76kxHcX7Kxa9eR+pvMygOK18RR3Y4fKFizOgCP8//B0 0=;
X-Files: signature.asc : 841
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0DWAgB5CCFX/4UNJK1UCoM4U30GuWYOgXUXAQqFbQKBMzgUAQEBAQEBAWUnhEEBAQEDAQEBASBLCwULAgEIFAQgCgICJwslAQEEDgUOiBQIDrFxkSsBAQEBAQEBAQEBAQEBAQEBAQEBAQENCIYhgXWBVIEChBUERIJgK4IrBZMfhHEBgyeBZ22IG4FnToN/gymFNIYkiQsBDw8BQ4NrbAEBh24/fwEBAQ
X-IronPort-AV: E=Sophos;i="5.24,542,1454976000"; d="asc'?scan'208,217";a="266163739"
Received: from alln-core-11.cisco.com ([173.36.13.133]) by alln-iport-3.cisco.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 27 Apr 2016 18:49:15 +0000
Received: from XCH-RTP-016.cisco.com (xch-rtp-016.cisco.com [64.101.220.156]) by alln-core-11.cisco.com (8.14.5/8.14.5) with ESMTP id u3RInEdC021482 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Wed, 27 Apr 2016 18:49:14 GMT
Received: from xch-rtp-020.cisco.com (64.101.220.160) by XCH-RTP-016.cisco.com (64.101.220.156) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Wed, 27 Apr 2016 14:49:13 -0400
Received: from xch-rtp-020.cisco.com ([64.101.220.160]) by XCH-RTP-020.cisco.com ([64.101.220.160]) with mapi id 15.00.1104.009; Wed, 27 Apr 2016 14:49:13 -0400
From: "Carlos Pignataro (cpignata)" <cpignata@cisco.com>
To: Adrian Farrel <adrian@olddog.co.uk>
Thread-Topic: [OSPF] Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt
Thread-Index: AdGghcVG0QuPS2RxQ6ajDKPyM64RjwAUT+OA
Date: Wed, 27 Apr 2016 18:49:13 +0000
Message-ID: <A8FA74F5-152F-4A1A-B427-6090EB6801C8@cisco.com>
References: <069b01d1a086$46d4d470$d47e7d50$@olddog.co.uk>
In-Reply-To: <069b01d1a086$46d4d470$d47e7d50$@olddog.co.uk>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: yes
X-MS-TNEF-Correlator:
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.150.49.220]
Content-Type: multipart/signed; boundary="Apple-Mail=_08D97DD0-416C-475B-A688-8AF18C8487DB"; protocol="application/pgp-signature"; micalg="pgp-sha256"
MIME-Version: 1.0
Archived-At: <http://mailarchive.ietf.org/arch/msg/ospf/Z39ghlbMzBtgYVVCCMxzh8pogDs>
Cc: "<rtg-ads@ietf.org>" <rtg-ads@ietf.org>, "draft-ietf-ospf-sbfd-discriminator.all@ietf.org" <draft-ietf-ospf-sbfd-discriminator.all@ietf.org>, "ospf@ietf.org" <ospf@ietf.org>, "<rtg-dir@ietf.org>" <rtg-dir@ietf.org>
Subject: Re: [OSPF] Rtg Dir review of draft-ietf-ospf-sbfd-discriminator-04.txt
X-BeenThere: ospf@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: The Official IETF OSPG WG Mailing List <ospf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ospf>, <mailto:ospf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ospf/>
List-Post: <mailto:ospf@ietf.org>
List-Help: <mailto:ospf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ospf>, <mailto:ospf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 27 Apr 2016 18:49:20 -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 <adrian@olddog.co.uk> wrote:
> 
> 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.

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.

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

Yes, this can be improved, see below.

> 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.
> 

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.
"

> 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.
> 

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.

> ---
> 
> 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
> 

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).

> 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.
> 

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

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

OK.

> ---
> 
> 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
> 

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>


> ---
> 
> 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.
> 

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

> ---
> 
> 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.
> 

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.

> --
> 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?

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.

> ---
> 
> Thanks,
> Adrian
> 
> _______________________________________________
> OSPF mailing list
> OSPF@ietf.org
> https://www.ietf.org/mailman/listinfo/ospf