Re: [Lsr] AD Review of draft-ietf-lsr-isis-srv6-extensions-11

Peter Psenak <ppsenak@cisco.com> Tue, 06 April 2021 11:54 UTC

Return-Path: <ppsenak@cisco.com>
X-Original-To: lsr@ietfa.amsl.com
Delivered-To: lsr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 3E3D63A1E2A; Tue, 6 Apr 2021 04:54:44 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -9.6
X-Spam-Level:
X-Spam-Status: No, score=-9.6 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_MED=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-0.001, SPF_NONE=0.001, URIBL_BLOCKED=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 xWlAPp2Euwah; Tue, 6 Apr 2021 04:54:41 -0700 (PDT)
Received: from aer-iport-2.cisco.com (aer-iport-2.cisco.com [173.38.203.52]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 85E163A1EA7; Tue, 6 Apr 2021 04:54:33 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=11037; q=dns/txt; s=iport; t=1617710074; x=1618919674; h=subject:to:cc:references:from:message-id:date: mime-version:in-reply-to:content-transfer-encoding; bh=UeW5+VPJBOR3aK8XRepbLehYH+XfK8AnJcpYDH8Hh7o=; b=L1ITBiqrBOz9X+NY0Be0sdMrI9JG6jrwgwpFyqnZbBKSzBy11AQRQ+tl ixdCgX2tSQ0glM41s1sdDmqKeGCxb++rAol1Wksqdp/uTGhLDbcGP7KUf 08xB/kFQJGgsa1nAfAlvOhd1TXds2Uzg4KiLH5HNJg4QcYGjCQGjPKyNg o=;
X-IPAS-Result: =?us-ascii?q?A0AXAADESmxglxbLJq1aGwEBAQEBAQEBBQEBARIBAQEDA?= =?us-ascii?q?wEBAUCBPwUBAQELAYMhVgEnEoRziQSIHy0DmmUUgWgLAQEBDygMBAEBhFACg?= =?us-ascii?q?XcmNQgOAgMBAQEDAgMBAQEBAQUBAQECAQYEFAEBAQEBAQEBaIVQDYZEAQEBA?= =?us-ascii?q?wEjDwEFLwQDBAcFCwsOCgICERUCAlcGAQwIAQGCbQGCZiEPqjN3gTKBAYM/A?= =?us-ascii?q?YEYg0GBPgaBDyoBjUtDgUlCgRIBASYMgm8+gmACgRctWYJYgmAEgUsKUhkHP?= =?us-ascii?q?SYBAy8UEFgtRQwmBwEBDQM2GJBSi2GdE4EUgxSDPIYlkwwFBwMfg02KeASFY?= =?us-ascii?q?ooqhhyVFYIQiVqSXoUKgVUBNoFbMxoIGxU7gjUBATNPGQ6HH4cMDQmIYoVHP?= =?us-ascii?q?wNnAgYBCQEBAwmLBy2CFgEB?=
IronPort-HdrOrdr: A9a23:0TfOhavm1ktaBGjteDLQp+Tm7skCP4Mji2hD6mlwRA09T+Wzna mV8Mgz/xnylToXRTUEkdePJKGPTRrnhPlIyKMWOqqvWxSjhXChK5ts4ZCn7zrrHSD/8eA179 YHT4FVDtrsAV9myfvr+QXQKadF/PCr+L2l7N2/815DVgdvApsP0y5YDUKhHlRyVE16A/MCZf +hz+5mgxblRngNdMS8ARA+Lor+jvnGjojvbxJDJzNP0mizpAil4rL7DBSUty12O1hy6Iwv/m Tfnwvy6r/LiZ6G4yXB3Gze5Yk+orHc4+ZEbfbgtuEoMDn2zi6naIN9Mofy3wwdkaWI9Esgls XKrlMGOcl+gkmhBl2dkF/KxxTq1ioo5jvZ7WKgxVHnocD/WVsBerN8ub4=
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-AV: E=Sophos;i="5.81,309,1610409600"; d="scan'208";a="34796242"
Received: from aer-iport-nat.cisco.com (HELO aer-core-3.cisco.com) ([173.38.203.22]) by aer-iport-2.cisco.com with ESMTP/TLS/DHE-RSA-SEED-SHA; 06 Apr 2021 11:54:25 +0000
Received: from [10.60.140.52] (ams-ppsenak-nitro3.cisco.com [10.60.140.52]) by aer-core-3.cisco.com (8.15.2/8.15.2) with ESMTP id 136BsOIi001879; Tue, 6 Apr 2021 11:54:24 GMT
To: Alvaro Retana <aretana.ietf@gmail.com>, draft-ietf-lsr-isis-srv6-extensions@ietf.org
Cc: John Scudder <jgs@juniper.net>, "lsr-chairs@ietf.org" <lsr-chairs@ietf.org>, lsr@ietf.org, Christian Hopps <chopps@chopps.org>
References: <CAMMESswF4GiLTRAYeLfhkC4w9tsr2J5YaMNFSG=979Bh2tmULw@mail.gmail.com> <836ca254-1273-7339-4c7d-f92d5e17315f@cisco.com> <CAMMESszNithwE6cGy9pkyb7Zxso=BTqwyO9oza-Ascz-5dU=jg@mail.gmail.com> <cf0a8c57-96f7-2684-8752-887887dc1831@cisco.com> <CAMMESszvHXXZpqQhrqF6MFVvpukf7vt4qLVXHocWa1JAneKXRw@mail.gmail.com>
From: Peter Psenak <ppsenak@cisco.com>
Message-ID: <ceab0774-4837-1cc2-23da-8a6945fbebc4@cisco.com>
Date: Tue, 6 Apr 2021 13:54:24 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0) Gecko/20100101 Thunderbird/60.9.1
MIME-Version: 1.0
In-Reply-To: <CAMMESszvHXXZpqQhrqF6MFVvpukf7vt4qLVXHocWa1JAneKXRw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
X-Outbound-SMTP-Client: 10.60.140.52, ams-ppsenak-nitro3.cisco.com
X-Outbound-Node: aer-core-3.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/HAtgICjP4qB23LKfbyZQigDeQSo>
Subject: Re: [Lsr] AD Review of draft-ietf-lsr-isis-srv6-extensions-11
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:lsr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsr/>
List-Post: <mailto:lsr@ietf.org>
List-Help: <mailto:lsr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:lsr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 06 Apr 2021 11:54:49 -0000

Hi Alvaro,

please see inline (##PP3):


On 30/03/2021 19:44, Alvaro Retana wrote:
> On March 25, 2021 at 6:03:53 AM, Peter Psenak wrote:
> 
> 
> Peter:
> 
> Hi!
> 
> I have some comments (see below) -- nothing major.  I look forward to -12.
> 
> Thanks!
> 
> Alvaro.
> 
> 
> 
> 
> ...
>>>>> Just one high-level comment: It is not clear to me why all the
>>>>> behaviors from rfc8986 are not covered in this document. If some are
>>>>> not applicable, or are covered elsewhere, please explain in the text.
>>>>
>>>> ##PP
>>>> not all behaviors from rfc8986 are applicable to IGPs. Section 10
>>>> ("Advertising Endpoint Behaviors") lists the ones that are applicable to
>>>> ISIS.
>>>
>>> I understand that -- other readers may not.
>>
>> ##PP2
>> we defined all behaviors that rfc8986 mentions should be advertised by
>> IGP, except the END.T. The END.T was originally defined, but during the
>> WGLC it was removed based on WG discussion:
>>
>> Mailing list discussion:
>> Thread1:
>> https://mailarchive.ietf.org/arch/msg/lsr/0Bp5DJrRJPvRyzZMZS0P_OE8Y7Q/
>> Thread2:
>> https://mailarchive.ietf.org/arch/msg/lsr/nKJbY5f6SHEwVCqqfoXSPidGKXQ/
> 
> Please just say somewhere that END.T is not included -- no need to
> justify in the document.

##PP3
done
> 
> 
> ...
>> 342 A prefix/SRv6 Locator that is advertised by a single node and without
>> 343 an A-Flag SHOULD be interpreted as a node specific locator.
>>
>> [major] "advertised by a single node and without an A-Flag" This is
>> equivalent to the current behavior of a prefix being "advertised by a
>> single node and without an A-Flag". IOW, you seem to be specifying
>> behavior that a node that doesn't implement (or even know about) this
>> document is expected to follow.
>>
>> ##PP2
>> if I remove the "prefix" and only keep the SRv6 Locator, would you be
>> fine with it? We are defining SRv6 Locator in this document.
> 
> No.  The description throughout this section talks about both prefixes
> and locators.  It reads as a general specification for all prefixes
> (including locators).
> 
> Suggestion (taking onto account the responses below)>
> 
>    A prefix/SRv6 Locator that is advertised by a single node and without
>    an A-Flag is considered node specific.

##PP3
done
> 
> 
>> [major] Related... What is a "node specific locator"? The A-flag
>> functionality could be used in a network that otherwise doesn't
>> implement SRv6, so calling it a "locator" doesn't seem right.
>>
>> ##PP2
>> please see my previous response.
>>
>>
>> [major] "SHOULD be interpreted" Interpreting is not really an
>> interoperability-requiring action. Is there anything here resulting
>> from the interpretation that requires normative language?
>>
>> ##PP2
>> what about:
>>
>> "An SRv6 Locator that is advertised by a single node and without
>> an A-Flag is considered as a node specific locator."
> 
> 
> 
>> ...
>> 349 The Prefix Attribute Flags Sub-TLV can be carried in the SRv6 Locator
>> 350 TLV as well as the Prefix Reachability TLVs. When a router
>> 351 originates both the Prefix Reachability TLV and the SRv6 Locator TLV
>> 352 for a given prefix, and the router is originating the Prefix
>> 353 Attribute Flags Sub-TLV in one of the TLVs, the router SHOULD
>> 354 advertise identical versions of the Prefix Attribute Flags Sub-TLV in
>> 355 both TLVs.
>>
>> [minor] This paragraph doesn't seem necessary given this text in §5:
>>
>> In cases where a locator advertisement is received in both a Prefix
>> Reachability TLV and an SRv6 Locator TLV, the Prefix Reachability
>> advertisement MUST be preferred when installing entries in the
>> forwarding plane.
>>
>> ##PP2
>> above mentioned paragraph does not say anything about the Prefix
>> Attribute Flags Sub-TLV present in the advertisement of the same prefix
>> in the Locator TLV and Prefix Reachability TLV. So we need to keep it.
>>
>>
>>
>> [major] If you decide to keep it... "SHOULD advertise
>> identical...Prefix Attribute Flags Sub-TLV" When is it ok to not do
>> so? Again, given that the Prefix Reachability TLVs are preferred,
>> this statement doesn't seem to matter, or carry interoperability
>> weight. s/SHOULD/should
>>
>> ##PP
>> well, not really. The Prefix Attribute Flags Sub-TLV should be the same
>> to guarantee the same treatment of both locator and legacy prefix
>> advertisements. The fact that the legacy prefix advertisement is
>> preferred when installing reachability of the prefix to forwarding does
>> not mean the Prefix Attribute Flags Sub-TLV advertised with Locator TLV
>> is useless - it can still be used when using Locator for other things -
>> e.g. derive SID for TILFA protection, etc.
> 
> Does this mean that having the same sub-TLV is also a consideration
> when comparing the locator and legacy advertisements?   If so, then
> that should also be mentioned in §5.
> 
> 
>> [] This point is related to Gunter's recent e-mail [1].
>>
>> ##PP2
>> the text has been updated to address Gunter's comment as follows:
>>
>> "The Prefix Attribute Flags Sub-TLV can be carried in the SRv6
>> Locator TLV as well as the Prefix Reachability TLVs. When a router
>> originates both the Prefix Reachability TLV and the SRv6 Locator TLV for
>> a given prefix, and the router is originating the Prefix Attribute Flags
>> Sub-TLV in one of the TLVs, the router SHOULD advertise same flags in
>> the Prefix Attribute Flags Sub-TLV in both TLVs. However, unlike TLVs
>> 236/237 the X-flag in the Prefix Attributes Flags sub-TLV is valid when
>> sent in the SRv6 Locator TLV. The state of the X-flag in the Prefix
>> Attributes Flags sub-TLV when included in the Locator TLV MUST match the
>> setting of the embedded X-flag in any advertisement of the same prefix
>> in TLVs 236/237."
> 
> [nit] s/advertise same flags/advertise the same flags

##PP3
done

> 
> [nit] s/ 236/237 / 236 and 237
>

##PP3
done



> [major] "MUST match the setting of the embedded X-flag"
> 
> What if the setting doesn't match?  What should the receiver do?

##PP3
Added following:


"In case of any inconsistency between the Prefix Attribute
  Flags advertised in the Locator TLV and in the Prefix Reachability TLV,
  the ones advertised in Prefix Reachability TLV MUST be preferred".



> 
> By "embedded", you are referring to the X bit inside 236/237, right?

##PP3
yes

> To differentiate, please call it "X bit" and add references to
> rfc5120/5308.

##PP3
done
> 
> 
> 
> ...
>> [minor] Please add Figure numbers/names for all packet formats.
>>
>> ##PP2
>> s that really required? I have never done it in any of RFCs I was editor
>> for.
> 
> Required, no...but it is good practice.
> 
> 
> ...
>> 666 9. SRv6 SID Structure Sub-Sub-TLV
>>
>> [] I don't understand what this sub-sub-TV is used for. Can you
>> please explain? Is there a relationship between it and the SID that
>> is advertised in the sub-TLVs? For example, I would assume that the
>> SID would have the bits that correspond to the argument set to 0 --
>> what if they're not? What is the purpose of this information? [Of
>> course, none of the supported behaviors take an ARG...]
>>
>> ##PP2
>> The SRv6 SID Structure Sub-Sub-TLV indicates the structure of the SID
>> associated with it. It can be used by implementation for validation of
>> the SID for consistency (e.g. if there is no ARG but there is something
>> in the ARG bits, then it can be ignored). They can be signalled via
>> BGP-LS to controller/apps that can verify the consistency in the block
>> and SID addressing in the domain. Details are outside the scope of this
>> draft.
> 
> Can you please add something like that to the draft?  I think that for
> now (unless other people ask as well) simply saying that the use is
> outside the scope should be enough.

##PP3
done
> 
> 
> ...
>> 741 11. Implementation Status
>> ...
>> 752 Types of SID supported: End, End.X, LAN End.X, END.OP
>>
>> [] "END.OP" is not defined. Also, the others are not types of SIDs,
>> but sub-TLVs.
>>
>> ##PP2
>> removed END.OP. This section is going to be removed anyway.
> 
> That fact doesn't mean that the information in it can be wrong.  If
> inaccurate, I would prefer the section not be there at all.

##PP3
I removed the "Implementation Status" section, it would have to be 
removed later anyway.


> 
> 
> ...
>> 834 12.1.2. Revised sub-TLV table
>> ...
>> 839 Type 27 135 235 236 237
>>
>> 841 1 y y y y y
>> 842 2 y y y y y
>> 843 3 n y y y y
>> 844 4 y y y y y
>> 845 5 y n n n n
>> 846 6 n y y y y
>> 847 11 y y y y y
>> 848 12 y y y y y
>> 849 32 n y y y y
>>
>> [major] Because the structure of the registry is changed, this
>> document should formally Update rfc7370 (where the current registry
>> was defined).
>>
>> ##PP2
>> I added following text:
>>
>> "		 "Sub-TLVs for TLVs 135, 235,
>> 236, and 237 (Extended IP reachability, MT IP. Reach, IPv6 IP. Reach,
>> and MT IPv6 IP. Reach TLVs)" registry defined in [RFC7370] to section
>> §12.1.1."
>>
>> Would that be sufficient?
> 
> We need the header to include the Updates tag, 

##PP3
sorry, I'm not sure I get it, what header and what "Updates tag"?



> and something in the
> Abstract and Introduction.
> 
> In the Abstract>
>     This documents updates rfc7370 by modifying an existing registry.

##PP3
done
> 
> 
> The text in Introduction can be the same, with a reference to §12.1.2.

##PP3
done
> 
> This Section should also ask IANA to add this document as a reference
> in the regsitry.

##PP3
done

> 
> 
> ...
>> 906 12.5. Sub-Sub-TLVs for SID Sub-TLVs
>>
>> 908 This document requests a new IANA registry be created under the IS-IS
>> 909 TLV Codepoints Registry to control the assignment of sub-TLV types
>> 910 for the SID Sub-TLVs specified in this document - Section 7.2,
>> 911 Section 8.1, Section 8.2. The suggested name of the new registry is
>> 912 "Sub-Sub-TLVs for SID Sub-TLVs". The registration procedure is
>> 913 "Expert Review" as defined in [RFC8126]. The following assignments
>> 914 are made by this document:
>>
>> [minor] In line with the name of other registries; suggestion:
>> "Sub-sub-TLVs for sub-TLVs 5, 43 and 44 (SRv6 End SID , SRv6 End.X SID
>> and SRv6 LAN End.X SID)".
>>
>>
>> ##PP2
>> I find that confusing as the sub-TLVs 5 is a locator Sub-TLV, while
>> Sub-TLVs 43 and 44 are IS reachability sub-TLVs.
>> What about:
>>
>> "Sub-Sub-TLVs for SID Sub-TLVs (SRv6 End SID, SRv6 End.X SID
>> and SRv6 LAN End.X SID)"
> 
> Most of the other registries include the number of the TLV.  So I
> think it would be good to remain consistent.  Maybe we should ask the
> current DEs: Chris, Hannes and Les.

##PP3
I have updated this based on the discussion with Les.

thanks,
Peter


> 
> [End]
> 
>