Re: [RTG-DIR] Rtgdir early review of draft-ietf-idr-segment-routing-te-policy-18

[Ketan Talaulikar <[e-mail]>]

Hi Med,

We've just posted an updated version with changes as discussed. It also
includes descriptions of the field as suggested by you :-)

https://datatracker.ietf.org/doc/html/draft-ietf-idr-segment-routing-te-policy-20

Please let us know if there are any outstanding comments to be addressed.

Thanks,
Ketan


On Wed, Jul 27, 2022 at 5:07 PM <[e-mail]> wrote:

> Hi Ketan,
>
>
>
> The proposed changes for the IANA section looks good to me. Thanks.
>
>
>
> For the description comment, I strongly suggest that you add the
> references to the fields description themselves. If you don’t have any
> reference, please consider updating the description to be meaningful. I
> trust you will do the right thing.
>
>
>
> Thank you.
>
>
>
> Cheers,
>
> Med
>
>
>
> *De :* Ketan Talaulikar <[e-mail]>
> *Envoyé :* lundi 25 juillet 2022 15:08
> *À :* BOUCADAIR Mohamed INNOV/NET <[e-mail]>
> *Cc :* [e-mail];
> [e-mail]; idr@ietf. org <
> [e-mail]>
> *Objet :* Re: [RTG-DIR] Rtgdir early review of
> draft-ietf-idr-segment-routing-te-policy-18
>
>
>
> Hi Med,
>
>
>
> Please check inline below with KT2. We can post an update once we are
> converged.
>
>
>
>
>
> On Mon, Jul 25, 2022 at 2:34 PM <[e-mail]> wrote:
>
> Re-,
>
>
>
> Please see inline.
>
>
>
> Cheers,
>
> Med
>
>
>
> *De :* rtg-dir <[e-mail]> *De la part de* Ketan Talaulikar
> *Envoyé :* lundi 25 juillet 2022 03:32
> *À :* BOUCADAIR Mohamed INNOV/NET <[e-mail]>
> *Cc :* [e-mail];
> [e-mail]; idr@ietf. org <
> [e-mail]>
> *Objet :* Re: [RTG-DIR] Rtgdir early review of
> draft-ietf-idr-segment-routing-te-policy-18
>
>
>
> Hi Med,
>
>
>
> Thanks for your quick response and once again for your very detailed and
> helpful review.
>
>
>
> Please check inline below for clarifications.
>
>
>
>
>
> On Mon, Jul 25, 2022 at 3:24 AM <[e-mail]> wrote:
>
> Hi Ketan,
>
>
>
> Thanks for taking care for most of the comments. This version looks much
> more better. There are still some few pending points:
>
>
>
> ·       The description of some fields should be elaborated (e.g., “Preference: a 4-octet value.”, “Local IPv4 Address: a 4-octet IPv4 address.”, ...).
>
> KT> If we take the example of Preference in sec 2.4.1. At the start of
> that section, there is a reference to sec 2.7 of the SR Policy arch (now
> RFC9256) which describes the field. In some of the recent reviews, I've got
> the comment to use references rather than repeating them unless necessary.
> In this case, there is no processing or validation of the preference value
> to be done by BGP and hence only the reference. For the segment types, the
> description of the fields is again covered by RFC9256 and we are just using
> the reference to them via the matching segment types.
>
> *[Med] I don’t see any references for the examples listed in the comments.*
>
>
>
> KT2> Sec 2.4.1 for Preference has the following text right at its start:
>
>
>
>    The Preference sub-TLV is used to carry the preference of an SR
>
>    Policy candidate path.  The contents of this sub-TLV are used by the
>
>    SRPM as described in section 2.7 of
>
>    [I-D.ietf-spring-segment-routing-policy <https://datatracker.ietf.org/doc/html/draft-ietf-idr-segment-routing-te-policy-19#ref-I-D.ietf-spring-segment-routing-policy>].
>
>
>
> Sec 2.4.4.2.6 for Segment Type F has the following text right at its start:
>
>
>
>    The Type F Segment Sub-TLV encodes an adjacency local address, an
>
>    adjacency remote address, and an optional SR-MPLS SID.  The format is
>
>    as follows:
>
>
>
> Additionally, at the start of Sec 2.4.4.2 there is the text that gives an
> overview of all the types and the reference to SR Policy document (now
> RFC9256) section 4 where the parameters for each type are described in more
> detail.
>
>
>
>
>
>
>    - I still don’t get how a meaning is associated with some field, but
>    then ask the implem to ignore that meaning:
>
>
>
> “Traffic Class (TC), S, and TTL (Total of
>
>       12 bits) are RESERVED and MUST be set to zero and MUST be ignored.”
>
>
>
> KT> This is the MPLS label encoding format.
>
> *[Med] :-) *
>
>
>
> It is used for both BSID TLV (from where you have quoted the above text)
> and the segment types where the values can actually be set. For BSID,
> these fields are "reserved" in this document and future documents can
> update this behavior.
>
> *[Med] Still don’t understand the rationale, but I’m not insisting on
> making any change. *
>
>
>
>    - The IANA section should include a note asking IANA to update the
>    I-ID (currently used for the early allocation,
>    [draft-previdi-idr-segment-routing-te-policy]) with this document. Having
>    clear instructions recorded in the document will save some cycles with
>    IANA.
>
>
>
> KT> In the IANA section, we use the "This document" convention as a
> reference pointer against each code point. My understanding (and this is
> what I've seen happen) is that as part of the IANA actions (e.g., after
> IESG evaluation is done or during the RFC editor process), the reference is
> updated as RFC-to-be... and then finally to RFCXXXX.
>
>
>
> Med : IANA does not know if a new ref will be added or will replace the
> existing one. This is better handled by providing clear guidance.
>
>
>
> KT2> I think I've got your point. How about the following change in sec
> 6.1? Note that we are asking for an "update" and only "this document" is
> listed as a reference. In some of my other documents in the past, we have
> made specific requests to add/append when there are more references.
>
>
>
> This document introduces a SAFI in the registry "Subsequent Address Family
> Identifiers (SAFI) Parameters" that has been assigned a code point by IANA.
> The entry needs to be updated as follows:
>
>
>
>               Code Point    Description          Reference
>
>               -----------------------------------------------
>
>                  73        SR Policy SAFI       This document
>
>
>
> A similar change is also in sec 6.2.
>
>
>
> Thanks,
>
> Ketan
>
>
>
>
>
>
>
> So I believe it is clear enough for IANA as also RFC Editor, but I will be
> happy to update based on any guidance from the IANA team.
>
>
>
> KT> However, if your point was that we need to trigger an IANA action to
> update the registries to reflect the WG draft name instead of the
> individual one against which the very initial allocations were made, then I
> agree. I can check on that. However, I don't believe that requires any
> change in the draft.
>
>
>
> Thanks,
>
> Ketan
>
>
>
>
>    -
>
>
>
> Cheers,
>
> Med
>
>
>
> *De :* rtg-dir <[e-mail]> *De la part de* Ketan Talaulikar
> *Envoyé :* dimanche 24 juillet 2022 02:26
> *À :* BOUCADAIR Mohamed INNOV/NET <[e-mail]>
> *Cc :* [e-mail];
> [e-mail]; idr@ietf. org <
> [e-mail]>
> *Objet :* Re: [RTG-DIR] Rtgdir early review of
> draft-ietf-idr-segment-routing-te-policy-18
>
>
>
> Hi Med,
>
>
>
> The draft update has just been posted:
> https://datatracker.ietf.org/doc/html/draft-ietf-idr-segment-routing-te-policy-19
>
>
>
> Please let us know if it addresses your comments and if you have any
> further feedback.
>
>
>
> Thanks,
>
> Ketan
>
>
>
>
>
> On Wed, Jul 20, 2022 at 1:56 PM <[e-mail]> wrote:
>
> Hi Ketan,
>
>
>
> Thanks for the follow-up.
>
>
>
> Will monitor when the new version is available and react if I have any
> further comments.
>
>
>
> Cheers,
>
> Med
>
>
>
> *De :* rtg-dir <[e-mail]> *De la part de* Ketan Talaulikar
> *Envoyé :* mardi 19 juillet 2022 19:55
> *À :* BOUCADAIR Mohamed INNOV/NET <[e-mail]>
> *Cc :* [e-mail];
> [e-mail]; idr@ietf. org <
> [e-mail]>
> *Objet :* Re: [RTG-DIR] Rtgdir early review of
> draft-ietf-idr-segment-routing-te-policy-18
>
>
>
> Hi Mohamed,
>
>
>
> Thanks for your very detailed review and helpful suggestions. Please check
> inline below for responses.
>
>
>
> We will post the update once the submission tool reopens.
>
>
>
>
>
> On Fri, Jul 8, 2022 at 6:35 PM Mohamed Boucadair via Datatracker <
> [e-mail]> wrote:
>
> Reviewer: Mohamed Boucadair
> Review result: Has Issues
>
> Document: draft-ietf-idr-segment-routing-te-policy-18
> Reviewer: Mohamed Boucadair
> Review Date: 08/07/2022
> IETF LC End Date: N/A
> Intended Status: Standards Track
>
> I appreciate the effort that was spent to progress this draft since more
> than 6
> years!
>
> Before reviewing the document, I started first by re-reading
> RFC8024/RFC9012
> and reading draft-ietf-spring-segment-routing-policy for establishing the
> context. Overall, the approach documented in
> draft-ietf-idr-segment-routing-te-policy is sound and straightforward.
>
> I didn’t find major concerns from a routing standpoint other than the need
> to
> motivate some few claims (see the detailed review file about RRs, for
> example)
> and the lack of considerations related to the handling of the various
> sub-TLVs
> by intermediate routers (if any).
>
> However, there are a number of generic issues that I would recommend to
> consider (see the detailed review for the full list). All these are
> easy-to-fix
> issues.
>
> # General Comments (in no specific order)
>
> ## Consistency
>
> ### Single or multiple paths
>
> There is an apparent inconsistency in the document about the handling of
> multiple paths. For example, Section 1 says :"Selection of the best
> candidate
> path for an SR Policy" while the same section says also “this will result
> in
> one or more candidate paths being installed into ..”.
>
>
>
> KT> The first is about the selection of the best candidate path for an SR
> Policy by the SRPM - this is what gets installed in the forwarding. The
> second is about the installation of the received candidate paths into the
> BGP table. There is no inconsistency.
>
>
>
>
> If multipath is supported, then please add an explicit statement and make
> sure
> the overall text is consistent.
>
>
>
> KT> Only a single CP is selected for a given SR Policy. This is per the
> draft-ietf-spring-segment-routing-policy and this document does not change
> that.
>
>
>
>
> ### Value 0 is marked as reserved for some registries, while that value is
> associated with a meaning for other registries.
>
> Is there any reason why a consistent approach isn’t followed here? what is
> the
> issues if value 0 is open for assignment?
>
>
>
> KT> It is normal routing protocol practice to not assign the TLV 0 values.
> Can you indicate where the TLV code point 0 is being assigned?
>
>
>
>
> ## Modifications to the format of the Color Extended Community
>
> The text says that you are modifying the format the Color Extended
> Community,
> while this is not true. What this draft does is just associating a meaning
> with
> some bits. I would update the text accordingly.
>
>
>
> KT> We are changing the format of only the Flags field and not of the
> entire EC. Flags are normally independent bits and here we are combining
> two bits to convey 4 values. Clarified this in the Introduction section.
>
>
>
>
> ## Normative language
>
> The use of the normative language should be double-checked. The most
> apparent
> concern is related the statement related to the handling of the reserved
> bits
> (SHOULD) while this RFC9012 uses MUST (which is correct, IMO).
>
>
>
> KT> Ack. I will fix it and change it to MUST.
>
>
>
>
> I tagged many others in the full review, fwiw.
>
> ## Lack of description
>
> Many fields are provided without acceptable description (e.g., “Local IPv4
> Address: a 4-octet IPv4 address.” or “Preference: a 4-octet value” !!).
>
>
>
> KT> These fields are in the context of a sub-TLV. There is text in the
> description of that sub-TLV that provides a reference (e.g., to the
> draft-ietf-spring-segment-routing-policy section or a segment type, etc.)
> There is no need to repeat a detailed description for each field IMO.
>
>
>
>
> Also, some fields are provided with a structure but the text says also that
> these are reserved (e.g., 2.4.2 says “TC, S and TTL (Total of 12 bits) are
> RESERVED”).
>
>
>
> KT> This is the MPLS label field. I am not sure that I follow your concern
> here.
>
>
>
>
> I wonder whether you can add a statement to say that multiple flags can be
> set
> simultaneously unless this is precluded by future flag assignments.
>
>
>
> KT> Not sure that is necessary. In most cases, the bits/flags are
> independent. Where they are not, there is generally text explaining their
> relationship or dependency.
>
>
>
>
> Last, the document does not include the expected behavior of intermediate
> routers (e.g., whether it is allowed or not to alter some fields). I guess,
> they must not touch the content of the attributes but it is better if this
> is
> explicitly mentioned in the text.
>
>
>
> KT> Yes, the contents must not be altered. Will clarify in sec 4.2.4.
>
>
>
>
> ## Reserved vs. Unassigned
>
> Almost all the “reserved” bits in the spec can be assigned in the future. I
> would use “Unassigned” as per RFC8126.
>
>
>
> KT> Ack. Will change in a few places where this has been missed.
>
>
>
>
> FWIW, 8126 says the following:
>
>       Unassigned:  Not currently assigned, and available for assignment
>             via documented procedures.
>
>       Reserved:  Not assigned and not available for assignment.
>             Reserved values are held for special uses, such as to extend
>             the namespace when it becomes exhausted.
>
> ## Deprecated values
>
> The document includes notes about some “deprecated” codepoints. I’m not
> sure
> there is a value in having such notes in the final RFC.
>
>
>
> KT> Yes, there is a need. One is to avoid them being used for any other
> sub-TLV in the future. Two is that there are early implementations out
> there that have some degree of support - even if they are just doing some
> parsing/showing.
>
>
>
>
> ## IANA considerations
>
> ### The document uses a mix of TBD statements (e.g., Section 2.4.3) and
> hard-coded values (early assignments). Not sure what’s was the rationale
> especially that code 20 was assigned but not listed as such.
>
>
>
> KT> Fixed.
>
>
>
>
> ### The IANA actions should be more explicit and ask IANA to update
> existing
> entries. For example, the current registry for code 73 points to
> [draft-previdi-idr-segment-routing-te-policy]. Need to update that entry
> and
> similar ones.
>
>
>
> KT> Have fixed the text. IANA will update "This document" to the RFC
> number before publication. There is no need to keep changing the draft name
> through its lifecycle.
>
>
>
>
> ### The document lists (under IANA section) some values that are
> deprecated.
> The document should be clear whether these codes are available for future
> assignment or not.
>
>
>
> KT> Deprecated means they are not available for assignment by IANA unless
> the IETF changes that via an RFC.
>
>
>
>
> ### Many sub-TLVs have flag bits but not all of them have a registry to
> track
> future flag bit assignments.
>
>
>
> KT> The registries would be added by future documents that start using
> those flags.
>
>
>
>
> ## Manageability considerations
>
> No such considerations are included in the document.
>
>
>
> KT> Will add.
>
>
>
>
> # Detailed review
>
> FWIW, you can find my full review at:
>
> * pdf:
>
> https://github.com/boucadair/IETF-Drafts-Reviews/raw/master/draft-ietf-idr-segment-routing-te-policy-18-rev%20Med.pdf
> * doc:
>
> https://github.com/boucadair/IETF-Drafts-Reviews/raw/master/draft-ietf-idr-segment-routing-te-policy-18-rev%20Med.doc
>
>
>
> KT> This was helpful and have incorporated most of those suggestions.
>
>
>
> Thanks,
>
> Ketan
>
>
>
> _________________________________________________________________________________________________________________________
>
>
>
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
>
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
>
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
>
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
>
>
>
> This message and its attachments may contain confidential or privileged information that may be protected by law;
>
> they should not be distributed, used or copied without authorisation.
>
> If you have received this email in error, please notify the sender and delete this message and its attachments.
>
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
>
> Thank you.
>
> _________________________________________________________________________________________________________________________
>
>
>
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
>
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
>
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
>
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
>
>
>
> This message and its attachments may contain confidential or privileged information that may be protected by law;
>
> they should not be distributed, used or copied without authorisation.
>
> If you have received this email in error, please notify the sender and delete this message and its attachments.
>
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
>
> Thank you.
>
> _________________________________________________________________________________________________________________________
>
>
>
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
>
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
>
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
>
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
>
>
>
> This message and its attachments may contain confidential or privileged information that may be protected by law;
>
> they should not be distributed, used or copied without authorisation.
>
> If you have received this email in error, please notify the sender and delete this message and its attachments.
>
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
>
> Thank you.
>
> _________________________________________________________________________________________________________________________
>
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
>
> This message and its attachments may contain confidential or privileged information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.
>
>