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

[Ketan Talaulikar <[e-mail]>]

Hi Med,

Thanks for catching that. I've made the fix in my edit buffer and the
change will reflect on the next update.

Thanks again for the detailed review and your suggestions.

Ketan


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

> Re-,
>
>
>
> Thanks, Ketan. This looks good to me.
>
>
>
> Apologies for missing this in the previous round. Please make this change
> in Section 2.4.1:
>
>
>
> OLD:
>
> Flags SHOULD be set to zero on transmission and MUST be ignored…
>
>
>
> NEW:
>
> Flags MSUT be set to zero on transmission and MUST be ignored …
>
>
>
> Cheers,
>
> Med
>
>
>
> *De :* rtg-dir <[e-mail]> *De la part de* Ketan Talaulikar
> *Envoyé :* mercredi 27 juillet 2022 10:00
> *À :* 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,
>
>
>
> 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.
>
> _________________________________________________________________________________________________________________________
>
> 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.
>
>