Re: [Pce] Genart last call review of draft-ietf-pce-association-diversity-10

Mahend Negi <mahend.ietf@gmail.com> Thu, 31 October 2019 16:04 UTC

Return-Path: <mahend.ietf@gmail.com>
X-Original-To: pce@ietfa.amsl.com
Delivered-To: pce@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D31EC12012C; Thu, 31 Oct 2019 09:04:16 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.997
X-Spam-Level:
X-Spam-Status: No, score=-1.997 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.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 5IQtsHMibjVX; Thu, 31 Oct 2019 09:04:09 -0700 (PDT)
Received: from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com [IPv6:2607:f8b0:4864:20::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 11600120096; Thu, 31 Oct 2019 09:04:09 -0700 (PDT)
Received: by mail-oi1-x22f.google.com with SMTP id c2so5590302oic.13; Thu, 31 Oct 2019 09:04:09 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VOg0pyd0HXzQfrPlzcero2nBeN3etqM5yqD4u/tuCQI=; b=GypuzLK05tof4GLcpiT8++uJgi4TJoWHWujwN+8IYAATE0RwHgvknHhzB5DbVn9w/O X/eyPWxSq+wKnSFJHCDkNbEDloLB58OzANVPa+HrB+BVwvBpo/HPc2Fmn4OhVD8hzKjS eDYlbDF1XbgGW9ho1jXNU6fTem9N5E6jtyXLwNweJyDAJ3QizrVhxZKoN4CYWy6zJUr4 TLsdYzFZYtQ6+cVqn7QSaoDmpTE6bHffD96GZ0Er3exm8wrH2L7yw6Bfg0VF3WOuDdH5 d3iz9irx/3IzcS+IdgzgPwkRPOCdXemkQ/Z9d++Q2q5AWZcIAVLNPjzD01KylY4HFVNU 0qQg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VOg0pyd0HXzQfrPlzcero2nBeN3etqM5yqD4u/tuCQI=; b=OOf5LXpPu0eui24jWZ656DxLv5x4X5S6AM0eFtulbnGQOt1yLBdZEDOegecdPPc1Nj VKwU7aOC3R8nz3KJxA9/x0TIv/nwDO7r7TCrYaJq4WyIjFjn5JabOxxAZ+MS/tczIT2t 5rsv67Y0AsYFxmwZyYSl6qtfrvhwvDJpMHQ4gamm47SthgPYC08OUcxc6TlfvkdB5v+N k1qJZmQF48HdJPgjy257ppweaGYl3NZniaQJWryJXfAllEN2vMmBI4QYH490vlC4gNUz w9HNWrlU3hrPEfi8fHzTdKG0RNyeS6QzABmUkCuibfID94fkH9RZAz1b530xH5OQ1g7a C9GQ==
X-Gm-Message-State: APjAAAWfLnYLnjtGmneiFRAKrOYr8HGWRoYga6/Z9SqnsawD7/+OWLBX PZpepYCIjRJNesMqHXxVFJBoJwzKNGfZUtfawT115ej2
X-Google-Smtp-Source: APXvYqzxyJttENnJj3XoNarvyKU78idXiOX6S0dmp+2te2qR0JAhfqS8BMsJvOTy7MkzE5BRj6uAV7gqxY2Ld+CUMjw=
X-Received: by 2002:a05:6808:644:: with SMTP id z4mr964798oih.105.1572537848152; Thu, 31 Oct 2019 09:04:08 -0700 (PDT)
MIME-Version: 1.0
References: <157092866490.1506.926010889840435426@ietfa.amsl.com>
In-Reply-To: <157092866490.1506.926010889840435426@ietfa.amsl.com>
From: Mahend Negi <mahend.ietf@gmail.com>
Date: Thu, 31 Oct 2019 21:33:56 +0530
Message-ID: <CAM5Nu_x2oXsyWGg5avnh8Jq3wcWC8b3dTg7c4_3vVhkmD4UJkg@mail.gmail.com>
To: Dale Worley <worley@ariadne.com>
Cc: General Area Review Team <gen-art@ietf.org>, pce@ietf.org, "ietf@ietf.org Discussion" <ietf@ietf.org>, draft-ietf-pce-association-diversity.all@ietf.org
Content-Type: multipart/alternative; boundary="000000000000d5c79c059636ffcf"
Archived-At: <https://mailarchive.ietf.org/arch/msg/pce/8aIwH4eDI742SDmHnAIbttgQ0Fo>
Subject: Re: [Pce] Genart last call review of draft-ietf-pce-association-diversity-10
X-BeenThere: pce@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Path Computation Element <pce.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/pce>, <mailto:pce-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/pce/>
List-Post: <mailto:pce@ietf.org>
List-Help: <mailto:pce-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/pce>, <mailto:pce-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 31 Oct 2019 16:04:17 -0000

Hi Dale,

Thanks for detailed review. All the comments are addressed in version-12:
https://tools.ietf.org/html/draft-ietf-pce-association-diversity-12

PSB inline[M.Negi] to your comments for details, further a working copy
also maintained, any further comments I'll update and post new version.
Working copy:
https://github.com/mahendnegi/ietf/blob/master/pce/assoc-diversity/draft-ietf-pce-association-diversity-13.txt

On Sun, Oct 13, 2019 at 6:34 AM Dale Worley via Datatracker <
noreply@ietf.org> wrote:

> Reviewer: Dale Worley
> Review result: Ready with Issues
>
> I am the assigned Gen-ART reviewer for this draft.  The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed by
> the IESG for the IETF Chair.  Please treat these comments just like
> any other last call comments.
>
> For more information, please see the FAQ at
>
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>
> Document:  draft-ietf-pce-association-diversity-10
> Reviewer:  Dale R. Worley
> Review Date:  2019-10-12
> IETF LC End Date:  2019-10-18
> IESG Telechat date:  not known
>
> Summary:
>
>        This draft is on the right track but has open issues, described
>        in the review.
>
> There are various exposition issues, as detailed below.
>
> There are some minor technical issues about reporting of error
> conditions, the significance of the P bit in DISJOINTNESS-STATUS-TLV,
> the detailed definition of the new objective functions, etc. as
> detailed below.
>
> But it seems like a number of complex cases haven't been clearly
> specified:  (1) the effect of the P bit if multiple LSPs have P=1, (2)
> the interaction of SVEC and disjoint-groups, and (3) the interaction
> of partially overlapping disjoint-groups and/or SVEC sets.
>
> Major issues:
>
> The relationship of this mechanism with SVEC seems to be important but
> is not clearly stated.  The relevant sections of the text seem to be:
> section 4 para 2, section 5.3, and section 5.4 from "[RFC5440] uses
> SVEC diversity flag" on.  I think that they need to be pulled into one
> section.  Then it will be possible to have a good description of the
> interaction with SVEC.

 [M Negi] Done


> At the end of section 5.6 is the revelation that (1) the
> DISJOINTNESS-CONFIGURATION-TLV is not attached to a group (in some
> sense), but instead a separate copy of it is attached to every LSP in
> the group, (2) these copies must match in the group IDs they carry and
> also the T, S, N, and L flags in them must agree, (3) (I suspect) any
> OF-code must agree, and (4) the P flags in them may differ.  This
> information is needed as background for a number of parts of the text
> to make sense, particularly the description of the P bit in section
> 5.2 and section 5.5.  I recommend that the last para of section 5.5 be
> moved to section 5.1 or the beginning of section 5.2, so the reader is
> aware that the descriptions of each LSP carry separate, and not
> necessarily identical, DISJOINTNESS-CONFIGURATION-TLVs.
>
 [M Negi] Done

> The path computation effects of the P bit are described in the "P"
> item in section 5.2 and section 5.5.  But the descriptions are
> unclear, or perhaps they presume that there are only two LSPs in the
> group.  I think the intended meaning is that all of the LSPs in the
> group with P=1 are computed first, and then with those LSPs fixed, the
> LSPs in the group with P=0 are computed.  This will cause
> shortest-path constraints (and other objective functions) to be
> optimized on the P=1 LSPs, and those paths will not be de-optimized by
> competition from the other paths.  This should probably be pulled out
> of the description of the "P" in its TLV and put into a separate
> paragraph.
>
[M Negi] Done, section 5.4.1


> Minor issues:
>
> Nits/editorial comments:
>
> In general, check the uses of "could" and "would".  I think "would" is
> sometimes used to mean "... in that situation, the XXX MUST ...", and
> "could" is sometimes used where "MAY" would be clearer.
>
[M Negi] Done

> The running title is given as "ASSOC-DISJOINT".  I think "PCEP
> Diversity Constraint Signaling" would be clearer.
>
[M Negi] Done, PCEP-Diversity-Constraints-Signaling

> Abstract
>
>    The proposed extension allows a Path Computation Client (PCC) to
>    advertise to a PCE that a particular LSP belongs to a
>    disjoint-group, thus the PCE knows that ...
>

> It would be clearer if "a disjoint-group" was amplified to "a
> particular disjoint-group".
>
[M Negi] Done

> Table of Contents
>
> There are a few section titles in which important words aren't
> capitalized ("functions" in section 5.4), or unimportant words are
> capitalized ("On" in sections 8.5 and 8.6).
>
[M Negi] Done

>
> The title of section 8.4 is a verb phrase, compared to the titles of
> the other subsections of section 8.  I suggest "Verification of
> Correct Operations" or maybe "Correct Operation Verification".
>
[M Negi] Done

> Perhaps change the title of Appendix A to just "Contributors", since
> there is no other enumeration of contributors.
>

> 1.  Introduction
>
>    This document specifies a PCEP extension to signal that a particular
>    group of LSPs should use diverse paths including the requested type
> , ---------------------------------------^
>    of diversity.
>
> Add a comma as indicated, to make it clear that "including" modifies
> "signal" rather than "paths".

[M Negi] Done

>    This document specifies a PCEP extension to signal that a particular
>    group of LSPs should use diverse paths including the requested type
>    of diversity.
>

> This implies that the "group" here is an instance of the association
> "grouping" of the preceding paragraph, but the Intro doesn't state
> that this document's grouping is an instance of the generic grouping
> mechanism.  Perhaps
>
>    This document specifies a PCEP grouping to signal that ...
>
[M Negi] Done, changed to:
   This document specifies a PCEP extension to signal that a set of LSPs
   in a particular group should use diverse paths, including the requested
type of diversity.

> 3.  Motivation
>
>    A customer may request
>    that the operator provide two end-to-end disjoint paths across the
>    IP/MPLS core.
>
> I suggest "the operator's IP/MPLS core".
>
[M Negi] Done

>
> [There are various places where the document seems to be phrased from
> the point of view of the network vendor rather than the point of view
> of the endpoint/user.  The IETF convention is to focus on the endpoint
> or user.]
>
>    The customer may use those paths as primary/backup or
>    active/active.
>

> This phrasing uses an adjective as a noun.  I prefer:
>
>    The customer may use these paths with a primary/backup or an
>    active/active configuration.

[M Negi] Done

>
> --
>
>    Different level of disjointness may be offered:
>
> Perhaps "Different levels of disjointness can be defined:"
>
> 4.  Applicability
>
>    In the figure above, let us consider that the customer wants to have
>    two disjoint paths between CE1/CE2 and CE3/CE4.
>
> I think that to make this accurate, you need to say "two disjoint
> paths, one between CE1 and CE2 and one between CE3 and CE4."
>
> This section seems disorganized.  In particular, 2nd and 3rd paras
> don't seem to add much.  Para 2 should be relocated to the SVEC
> discussion (see "Major issues).  Para 3 seems to have two sentences of
> background information (I think it's duplicated in section 1) and one
> sentence that essentially states that this document defines a
> "diversity group" of LSPs which is a type of the generalized "LSP
> group".
>
>    Using PCEP, the PCC could indicate that a disjoint path computation
>    is required, such indication should include disjointness parameters
>    such as the type of disjointness, the disjoint group identifiers, and
>    any customization parameters according to the configured local
>    policy.  As mentioned previously, the extension described in
>    [I-D.ietf-pce-association-group] is well suited to associate a set of
>    LSPs with a particular disjoint-group.
>
> This touches on the right topics, but doesn't really connect them.
> Also, don't use "could" for an action that this document defines how
> to do.  You want to say something like
>
>    Using the extension to PCEP defined in this document, the PCC uses
>    the [I-D.ietf-pce-association-group] extension to indicate that a
>    group of LSPs are required to be disjoint; such indication should
>    include disjointness parameters such as the type of disjointness,
>    the disjoint group identifiers, and any customization parameters
>    according to the configured local policy.
>
> --

 [M Negi] Done


>      Figure 2 - Sample use-cases for carrying disjoint-group over PCEP
>                                   session
>
> In this figure, there seems to be a conflict in notation:  Case 1 uses
> "PCC" seemingly in the same role for which Case 2 uses "PE 1" and "PE
> 3".  The captions of both cases refer to "PCC".

 [M Negi] Done, changed to PCC1 and PCC2

>
> Also, the Case 1 and Case 2 captions run into each other.
>
>    Using the disjoint-group within a PCEP messages may have two purpose:
>
> Better would be "The disjoint-group within PCEP messages is used for:"
>
[M Negi] Done

>
> 5.1.  Association Group
>
>    As per [I-D.ietf-pce-association-group], LSPs are associated with
>    other LSPs with which they interact by adding them to a common
>    association group.
>
> To be exact, you want to say "by adding all of them"; by default
> "them" refers only to the "LSPs" that is the subject of the sentence.
>
>    The Association parameters, as described in
>    [I-D.ietf-pce-association-group] as the combination of the mandatory
>    fields Association type, Association ID and Association Source in the
>    ASSOCIATION object, that uniquely identify the association group
>    belonging to this association.  If the optional TLVs - Global
>    Association Source or Extended Association ID - are included, then
>    they are included in combination with mandatory fields to uniquely
>    identify the association group.
>
> Use either "field" or "TLV" consistently (unless the TLVs really are
> different from the "fields").
>
> This is awkward.  Better would be
>
>    As described in [I-D.ietf-pce-association-group], the association
>    group is uniquely identified by the combination of these fields in
>    the ASSOCIATION object:  Association Type, Association ID,
>    Association Source, and (if present) Global Association Source or
>    Extended Association ID.

 [M Negi] Done

>
>    ... This document defines a new ...
>
> Start a new para with this sentence.
>
>    o  Association type = TBD1 ("Disjointness Association Type") for
>       Disjoint Association Group (DAG).

 [M Negi] Done

>
> This seems redundant, unless there exists another type that is also
> associated with Disjoint Association Group (DAG).  Otherwise, you can
> probably use
>
>    o  Association type = TBD1 Disjoint Association Type

 [M Negi] Done

>
> --
>
>    This capability exchange for the association type
>    described in this document (i.e.  Disjointness Association Type) MUST
>    be done before using the disjointness association.
>
> This can be simplified to
>
>    This capability exchange for Disjointness Association Type MUST
>    be done before using the disjointness association.
>
[M Negi] Done

>
> There seem to be multiple uses of "Disjoint Association Group (DAG)"
> and "Disjointness Association Type (TBD1)", each of which states the
> equivalence of two things.  You should be able to use one instance of
> each.  Also, it's probably worth putting "Disjointness Association
> Type" into section 2.
>
[M Negi] Done

>
>    This association type is considered to be both dynamic and operator-
>    configured in nature.
>
[M Negi] Done

>
> It might be worth providing references for the meaning of "dynamic"
> and "operator-configured".
>
[M Negi] Done

>
>    A disjoint group can have two or more LSPs, [...]
>
> The significance of this clause to the rest of the sentence is
> unclear; it seems obvious that a disjoint group needs at least two
> members before it is significant.  However, as a matter of protocol,
> can a group that has only one LSP be established?
>
[M Negi] Yes, during synh or configuration change and LSP can be established

>
>    The disjoint status is informed to the PCC.
>
> This seems to be describing how to report to the PCC that the
> computation was not done as requested.  I think you want to provide
> more details here, or a pointer to them.  In any case, "disjoint
> status" isn't really a clear statement that the error condition will
> be reported.
>
   Associating a particular LSP to multiple disjoint groups is
>    authorized from a protocol perspective, however there is no assurance
>    that the PCE will be able to compute properly the multi-disjointness
>    constraint.
>
> You want to state *much* more clearly what might happen.  This starts
> with "A particular LSP MAY be associated to multiple disjoint groups,
> but in that case, the PCE MAY ..."  What are the outer boundaries of
> the results that the PCE MAY return?  Is the PCE required to provide
> any particular error indication?
>
[M Negi] Done

>
> 5.2.  Disjoint TLVs
>
> See "Major issues" regarding explaining that there is a separate copy
> of DISJOINTNESS-CONFIGURATION-TLV for each LSP, and they needn't all
> be the same.  All of the constraints described in the last paragraph
> of section 5.5 should be moved to here, or perhaps section 5.1.
>
>     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 = TBD2           |            Length             |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    |                 Flags                               |T|P|S|N|L|
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> "Flags" should be "Reserved".
>
[M. Negi] Flags field is defined and a IANA registry created so that
future extension can add more flags.

>
>       *  P (Shortest path) bit: when set, this indicates that the
>          computed path of the LSP SHOULD satisfy all the constraints and
>          objective functions first without considering the diversity
>          constraint.  ...
>
> See "Major issues" for providing a separate paragraph to explain the
> semantics of the "P" bit.
> [M. Negi] Done
>       *  T (Strict disjointness) bit: when set, if disjoint paths cannot
>          be found, PCE SHOULD return no path for LSPs that could not be
>          be disjoint.  When unset, the PCE is allowed to relax
>          disjointness by either applying a requested objective function
>          (cf.  Section 5.4 below) or using any other behavior if no
>          objective function is requested (e.g. using a lower disjoint
>          type (link instead of node) or fully relaxing disjointness
>          constraint).  Further see Section 5.6 for details.
>
> Instead of "or using any other behavior if no objective function is
> requested" it would be more conventional to say "or, if no objective
> function is requested, using local policy".
>
[M Negi] Done

>
>    The DISJOINTNESS-STATUS-TLV uses the same format as the DISJOINTNESS-
>    CONFIGURATION-TLV with a different type TBD3 (in the TLV).  The L, N,
>    and S flags are set based if the computed path meet the disjointness
>    criteria.
>
> Should be "... are set if the respective disjointness criterion was
> requested and the computed paths meet it"
>
[M Negi] Done

>
>    The P flag is set to indicate that the computed path is
>    the shortest
>
> Surely "the shortest" needs further explanation -- the shortest path
> that meets what constraints?  I suspect that this just reflects the P
> flag in the CONFIGURATION-TLV -- is there a situation where it would
> not?
>
[M Negi] Done

>
>    and the T flag has no meaning in the DISJOINTNESS-
>    STATUS-TLV and MUST NOT be set while sending and ignored on receipt.
>
[M Negi] Done

>
> This clause should be a separate sentence.
>
>    Any new flag defined for the DISJOINTNESS-CONFIGURATION-TLV is be
>    automatically applicable to the DISJOINTNESS-STATUS-TLV.
>
> This can't be correct, as there is no defined way to automatically
> apply a new flag to DISJOINTNESS-STATUS-TLV.  (The previous paragraph
> defines three separate semantics for flags in DISJOINTNESS-STATUS-TLV!)
> But you can say:
>
>    Any document defining a new flag for the
>    DISJOINTNESS-CONFIGURATION-TLV automatically defines a new flag
>    with the same name and in the same location in
>    DISJOINTNESS-STATUS-TLV; the semantics of the flag in
>    DISJOINTNESS-STATUS-TLV MUST be specified in the document that
>    specifies the flag in DISJOINTNESS-CONFIGURATION-TLV.

 [M Negi] Done

> 5.3.  Relationship to SVEC
>
>    The PCE would consider both the objects
>
> I think you mean MUST instead of "would".
>
[M Negi] Done

>
>    In case no such path is possible (or the
>    constraints are incompatible), the PCE MUST send a path computation
>    reply (PCRep) with a NO-PATH object indicating path computation
>    failure as per [RFC5440].
>
> Adding "(or the constraints are incompatible)" to "no such path is
> possible" is redundant!
>
> This discussion is not clear to me.  Partly it is because I'm not
> familiar with SVEC.  But I suspect that "synchronization of a set of
> path computation requests" means that "the computations have to be
> done together" and "dependent requests" means something similar to the
> constraints between computed paths specified by the
> DISJOINTNESS-CONFIGURATION-TLV.
>
> Is it possible to provide a comparison between the requirements
> specified by SVEC and the requirements specified by
> DISJOINTNESS-CONFIGURATION-TLV?
>
> Also, is there any specific relationship between the set of LSPs in an
> SVEC and the set of LSPs in a disjoint-group?  What if multiple SVECs
> and disjoint-groups overlap in a complex way?
>
[M.Negi]  Done, section 5.4 explains

>
> 5.4.  Disjointness Objective functions
>
>    To minimize the common shared resources (Node, Link or SRLG) between
>    a set of paths during path computation three new OF-codes are
>    proposed:
>
> Actually, the new codes are to allow specification of minimization of
> shared resources.  Also, an RFC doesn't just proposed the codes, it
> specifies them.  So this is better phrased:
>
>    Three new OF-codes allow specification of minimization of common
>    shared resources (Node, Link or SRLG) between a set of paths during
>    path computation:

 [M.Negi] Phrased as:
This document defines three new OF-codes <xref
target="Disjointness-objective-functions"/> to maximize diversity as much
as possible, in other words, new OF-codes allow specification of
minimization of common shared resources (Node, Link or SRLG) among a set of
paths during path computation.

>
> --
>
>    MSL
>
>    * Name:  Minimize the number of shared (common) Links.
>
>    * Objective Function Code:  TBD4
>
>    * Description:  Find a set of paths such that it passes through the
>       least number of shared (common) links.
>
>    MSS
>
>    * Name:  Minimize the number of shared (common) SRLGs.
>
>    * Objective Function Code:  TBD5
>
>    * Description:  Find a set of paths such that it passes through the
>       least number of shared (common) SRLGs.
>
>    MSN
>
>    * Name:  Minimize the number of shared (common) Nodes.
>
>    * Objective Function Code:  TBD6
>
>    * Description:  Find a set of paths such that it passes through the
>       least number of shared (common) nodes.
>
> The use of "it" in the above definitions is unclear, since the paths
> are "they".
>
[M.Negi]  Done

>
> There are a number of similar but different ways that "the number of
> shared" entities can be defined.  E.g., if four LSPs pass through a
> single node, does that count as 1 (a single node is shared), 4 (four
> LSPs pass through a node that is shared), 6 (six pairs of LSPs
> conflict, in that the paths in the pair share a node)?  That is, are
> we counting (and thus minimizing) shared entities, the number of LSPs
> that violate the requested constraint, or the number of pairs of LSPs
> that overlap in an unacceptable way?
>
[M. Negi] In general its for path-protection with least common resource.
sharing minimized resources(node/link/srlg) give absolute protection during
any unwanted network event.

>
>    If the OF-list TLV is included in the Association Object, the OF-code
>    inside the OF Object MUST include one of the disjoint OFs defined in
>    this document.  If this condition is not met, the PCEP speaker MUST
>    respond with a PCErr message with Error-Type=10 (Reception of an
>    invalid object) and Error-Value=TBD9 (Incompatible OF code).
>
> I find it surprising that you've defined a specific error code for an
> unexpected OF-code, but you don't apply that error code to the
> situation where there are multiple OF-codes.  Instead, you have all
> but the first silently ignored.
>
> There's also the intersection case where the first OF-code is
> legitimate, but the second OF-code is not one in this set.  Does that
> get an "Incompatible OF code" error, or is the second one just
> ignored?
>
> I think it's cleaner to change TBD9 to "Incompatible OF-list for
> disjoint-group" and require it for all of these cases.
>
[M.Negi]  Done

>
>    [RFC5440] uses SVEC diversity flag for node, link or SRLG to describe
>    the potential disjointness between the set of path computation
>    requests used in PCEP protocol.
>
> This paragraph seems very important, but completely out of place here.
> Might it fit better in section 5.3?
>
[M.Negi] As part of major issue fix

>
>    This document defines three new OF-codes to maximize diversity as
>    much as possible, in other words, minimize the common shared
>    resources (Node, Link or SRLG) between a set of paths.
>
> This paragraph seems redundant after the detailed discussion of these
> codes.
>
> The remaining paragraphs of this section seem like they should go in
> section 5.3.
>
[M.Negi] Done

>
> 5.5.  P Flag Considerations
>
> See above queries about the P flag.
>
>    As mentioned in Section 5.2, the P flag (when set) indicates that the
>    computed path of the LSP SHOULD satisfies all constraints and
>    objective functions first without considering the diversity
>    constraint.
>
> There are complexities that don't seem to be addressed.  E.g., if
> there are two LSPs with P=1 and two with P=0, I would expect that the
> first two would be computed with full consideration of the
> disjointness constraint between them.  So "without considering the
> diversity constraint" really isn't a good description of what is
> intended.
>
[M.Negi] When multiple LSPs have P=1 then they might not
   be disjoint among themselves. Multiple LSPs in the same disjoint group
may have the P
   flag set.  In such a case, those LSPs may not be disjoint from each
other but will be disjoint from others LSPs in the group
   that have the P flag unset. Because if we consider disjointness among
the LSP with p=1 they may not be shortest anymore!

>
> 5.6.  Disjointness Computation Issues
>
>    Also, in case of network event leading to an impossible
>    strict disjointness, the PCE MUST send a PCUpd message ...
>
> I think you want to start a new paragraph here and say
>
>    In case of a network event which makes it impossible to satisfy the
>    constraints, the PCE MUST send a PCUpd message ...
>
[M.Negi]  Done

>
> --
>
>    The
>    actual level of disjointness computed by the PCE ...
>
> More correct to say
>
>    The
>    actual level of disjointness of the paths computed by the PCE ...
>
[M.Negi]  Done

>
> --
>
>    While the DISJOINTNESS-CONFIGURATION-TLV defines the
>    expected level of disjointness required by configuration, the
>    DISJOINTNESS-STATUS-TLV defines the actual level of disjointness
>    computed.
>
> Better to s/expected/desired/ and s/actual/achieved/.
>
[M.Negi]  Done

>
>    There are some cases where the PCE may need to completely relax the
>    disjointness constraint in order to provide a path to all the LSPs
>    that are part of the association.  A mechanism that allows the PCE to
>    fully relax a constraint is considered by the authors as more global
>    to PCEP rather than linked to the disjointness use case.  As a
>    consequence, it is considered as out of scope of the document.
>
> This para doesn't seem to make any difference.  As long as the T bit
> is clear and the local policy allows complete relaxation of the
> requested disjointness, this case is covered within the mechanisms of
> this document.  And this case will be reported in a straightforward
> way by a DISJOINTNESS-STATUS-TLV with all bits clear.
>
 [M.Negi] This is related to ignoring the whole object itself i.e as per
draft-dhody-pce-stateful-pce-optional/

6.  Security Considerations
>
>    This document defines one new type for association ...
>
> Better "This document defines one new PCEP association type ..."
>
> After para 1, you probably want to add this case: "A spurious LSP can
> have flags that are inconsistent with those of the legitimate LSPs of
> the group and thus cause LSP allocation for the legitimate LSPs to
> fail with an error."
>
[M.Negi]  Done