Re: [Last-Call] Rtgdir telechat review of draft-ietf-sidrops-8210bis-06

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Randy, 

Thank you for taking care of the comments. -07 addresses almost all of them.

Please find below few remaining points based on -07: 

* Section 5.1 (Fields of a PDU): This section does not list the fields of the newly added ASPA PDU (e.g., AFI Flags, Customer Autonomous System Number, Provider Autonomous System Numbers, Provider AS Count).

* Section 5.12

(1) 

OLD:
   Receipt of an ASPA PDU announcement
   (announce/withdraw flag == 1) when the router already has an ASPA PDU
   with the same Customer Autonomous System Number and the same Address
   Family (see Flags field), replaces the previous one.^
               ^^^^^^

NEW:
   Receipt of an ASPA PDU announcement
   (announce/withdraw flag == 1) when the router already has an ASPA PDU
   with the same Customer Autonomous System Number and the same Address
   Family (see AFI Flags field), replaces the previous one.
              ^^^^^^^^

(2) Shouldn't the following (and similar text) be updated to precise that the behavior is per AF?

CURRENT:
   The router should see at most one ASPA from a cache for a particular
   Customer Autonomous System Number active at any time.

   ...
   
   For the ASPA PDU, the announce/withdraw Flag is set to 1 to indicate
   either the announcement of a new ASPA record or a replacement for a
   previously announced record with the same Customer Autonomous System
   Number.  The announce/withdraw flag set to 0 indicates removal of the
   ASPA record in total.  Here, only the customer AS of the ASPA record
   MUST be provided, the Provider AS Count as well as the Provider AS
   Numbers list MUST BE zero.

(nit: s/MUST BE/MUST be)

* Section 6:

OLD: 
  Recommended default:  3600 seconds (2 hours).

NEW: 
  Recommended default:  3600 seconds (1 hour).

(I noted that you are discussing reverting for the Expire Interval).

* Section 7:

(1)

I would add some text to be explicit how the content of the Arbitrary Text field is used on the receiver side to decide whether/which version to use for subsequent exchanges. Typically, the highest common version will be picked. If no common version is supported, the router will cease the connection.

(2) 

>    1.  The cache may terminate the connection, perhaps with a
> version 4
>        Error Report PDU, Unsupported Protocol Version.

I guess you meant "version 2 Error Report PDU with Error Code 4".

Cheers,
Med

> -----Message d'origine-----
> De : last-call <last-call-bounces@xxxxxxxx> De la part de Randy
> Bush
> Envoyé : mercredi 1 juin 2022 06:21
> À : Mohamed Boucadair via Datatracker <noreply@xxxxxxxx>
> Cc : rtg-dir@xxxxxxxx; draft-ietf-sidrops-8210bis.all@xxxxxxxx;
> last-call@xxxxxxxx; sidrops@xxxxxxxx
> Objet : Re: [Last-Call] Rtgdir telechat review of draft-ietf-
> sidrops-8210bis-06
> 
> mohamed,
> 
> the thorough review is VERY much appreciated.  excuse my delay;
> too much going on
> 
> >    o  Sections 5.6 and 5.10: The following conflicts with the
> new
> >       definition of the flags as per Section 5.1
> >
> >    "The lowest-order bit of the Flags field is 1 for an
> announcement and
> >     0 for a withdrawal."
> 
> i think all three are now in sync.
> 
> >    o  Section 7:
> >
> >    (1)
> >
> >      "A router  MUST start each transport connection by issuing
> either a
> >      Reset Query or a Serial Query."
> >
> >      Shouldn't the text indicate, e.g., that the router has to
> use the
> >      highest version it supports?
> >
> >    (2)
> >
> >     "This query will tell the cache which
> >       version of this protocol the router implements."
> >
> >       What if the router supports more than one version?  This
> text
> >       seems to assume that only one version can be supported.
> >
> >    (3)
> >
> >    "If a cache which supports version  N receives a query from a
> router
> >    which specifies version Q < N, the cache MUST downgrade to
> protocol
> >    version Q [RFC6810] or [RFC8210] or send a version 1  Error
> Report PDU
> >    with Error Code 4 ("Unsupported Protocol Version")  and
> terminate the
> >    connection."
> >
> >
> >       *  I suggest "s/supports version/supports a highest
> version" to
> >          cover the case of multiple versions.
> >
> >       *  Shouldn't the Error Report PDU be the version 2, not
> version 1.
> >
> >       *  Shouldn't the error PDU be designed to return the
> supported
> >          versions to avoid cycles in the version negotiation?
> >
> >
> >    (4)
> >
> >    "1.  The cache may terminate the connection, perhaps with a
> version 0
> >         Error Report PDU.  In this case, the router MAY retry
> the
> >         connection using protocol version C."
> >
> >       Why version 1 error is not considered?
> 
>    A router MUST start each transport connection by issuing either
> a
>    Reset Query or a Serial Query.  This query MUST tell the cache
> the
>    highest version of this protocol the router implements.
> 
>    If a cache which supports version N receives a query from a
> router
>    which specifies its highest supported version Q < N, the cache
> MUST
>    downgrade to protocol version Q [RFC6810] or [RFC8210] or send
> a
>    version 2 Error Report PDU with Error Code 4 ("Unsupported
> Protocol
>    Version") and terminate the connection; in which case the
> Arbitrary
>    Text field of the ERROR Report PDU MUST be a list of one octet
> binary
>    integers indicating the version numbers the cache supports.
> 
>    If a router which supports version N sends a query to a cache
> which
>    only supports version C < N, one of two things will happen:
> 
>    1.  The cache may terminate the connection, perhaps with a
> version 4
>        Error Report PDU, Unsupported Protocol Version.  In this
> case,
>        the router MAY retry the connection using protocol version
> C.
> 
>    2.  The cache may reply with a version C response.  In this
> case, the
>        router MUST either downgrade to version C or terminate the
>        connection.
> 
>    In any of the downgraded combinations above, the new features
> of the
>    higher version will not be available, and all PDUs will have
> the
>    negotiated lower version number in their version fields.
> 
>    If either party receives a PDU containing an unrecognized
> Protocol
>    Version (neither 0, 1, nor 2) during this negotiation, it MUST
> either
>    downgrade to a known version or terminate the connection, with
> an
>    Error Report PDU unless the received PDU is itself an Error
> Report
>    PDU.
> 
> >    o  Section 8: ASPA PDUs are not illustrated in this section.
> 
> done
> 
> >    o  Section 1 says that "This document updates [RFC8210]."
> while this
> >       should say that RFC8210 is obsoleted.
> 
> i hope this is no longer the case
> 
> >    o  Section 1.2: The changes of the meaning or some flags are
> not
> >       listed in this section.
> 
> the flag semantics reverted
> 
> >    o  Section 2: Consider adding NEW entries for ASPA, CAS, and
> SPAS.
> 
> CA yes.  there are cites for ASPA; and i do not know what SPAS
> are.
> 
> >    o  Section 5.1: "Customer Autonomous System Number" and
> "Provider
> >       Autonomous System Number" are not listed here.
> 
> see ASPA
> 
> >    o  Section 5.1: Shouldn't version be set to 2 in the
> following text:
> >
> >    "Protocol Version:  An 8-bit unsigned integer, currently 1 ,
> denoting
> >          the version of this protocol."
> 
> indeed it should
> 
> >    o  Section 5.1: Isn't this behavior redundant for IPv4/IPv6
> PDU
> >       types?  Which one takes precedence if there is a conflict
> between
> >       the flag and the IPvX PDU Type?
> 
> i do not understand
> 
> >     "Flags:  The lowest-order bit of the Flags field is 0 for
> IPv4 and 1
> >        for IPv6."
> 
> that got moved into the ASPA PDU's AFI Flags Field
> 
> >    o  Section 5.1: Consider adding a pointer to the section
> where the
> >       new PDU is defined.
> >
> >    OLD:
> >          For the ASPA PDU, the announce/withdraw Flag ...
> >
> >    NEW:
> >          For the ASPA PDU (Section 5.12), the announce/withdraw
> Flag ...
> 
>    *  A new ASPA PDU type (Section 5.12) has added to support
>       [I-D.ietf-sidrops-aspa-profile].
> 
> 
> >    o  Section 5.1: Isn't the following redundant with this text
> in the
> >       preamble of Section 5?
> >
> >       "In protocol version 2, they MUST be zero on transmission
> and MUST
> >       be ignored on receipt."
> >
> >       I'm referring to this preamble:
> >
> >       "Reserved fields (marked "zero" in PDU diagrams) MUST be
> zero on
> >       transmission and MUST be ignored on receipt."
> 
> sure
> 
> >    o  Section 5.8: I have several comments about this part:
> >
> >    "Note that the End of Data PDU changed significantly between
> versions
> >    0 and 1.   For version 0 compatibility , the following is the
> version 0
> >    End of Data PDU."
> >
> >       *  It seems this text wasn't updated to reflect the
> version
> >          change.  At least, the text should indicate that the
> same
> >          format is preserved for version 2.
> >
> >       *  Not sure to understand the intent of "For version 0
> >          compatibility" as the version negotiation will lead to
> the use
> >          of a common version (if supported).
> >
> >       *  Isn't the last part of the text already covered in
> >          rfc6810#section-5.8?
> 
> ok, dumped the v0 diagram and reduced to
> 
>    Note that the End of Data PDU changed significantly between
> versions
>    0 and 1.  For version 0 compatibility, the following is the
> version 0
>    End of Data PDU.
> 
> >    o  Section 5.12:
> >
> >       "An ASPA
> >       PDU represents one single customer AS and its provider ASs
> for a
> >       particular Address Family."
> >
> >       Did the WG considered whether it useful to refer to all
> AFs rather
> >       than sending PDUs, each per AF?
> 
> yes.  i fear i will have left the building before v4 and v6
> topologies are mostly congruent.
> 
> >    o  Section 5.12:
> >
> >    "The router should see at most one ASPA from a cache for a
> particular
> >    Customer Autonomous System Number active at any time.  As a
> number of
> >    conditions in the global RPKI may present multiple valid ASPA
> objects
> >    for a single customer to a particular RP cache , this places
> a burden
> >    on the cache to form the union of multiple ASPA records it
> has
> >    received from the global RPKI into one ASPA PDU."
> >
> >       *  Is "ASPA object" the same as "ASPA record"?
> 
> multiple valid ASPA RPKI records
> 
> >       *  What is meant by "RP cache"?
> 
> glossary whacked to say
> 
>     A Cache, AKA Relying Party Cache
> 
> >    o  Section 5.12:
> >
> >       "the cache SHOULD set  the Provider AS Count to zero, and
> have
> >       a null Provider AS Numbers list."
> >
> >       You may explain why "MUST" is not used here.
> 
> it should be
> 
> >    o  Section 6:
> >
> >       *  Please explain why the recommended Refresh Interval was
> changed
> >          from 3600 (v1).
> >
> >       *  Idem for the recommended Expire Interval that was
> changed from
> >          from 7200 (v1).
> 
> we're discussing reverting
> 
> >    o  Section 13:
> >
> >       "There is an IANA registry where valid error codes
> >       are listed; see Section 15."
> >
> >       The IANA registry is authoritative here.  I suggest to add
> a
> >       pointer to that registry instead of Section 15.
> 
> the registry is an authorative *list*, and points back to the RFCs
> for semantics.  but it is worth citing; which i'll do if i can
> figure out how in antique xml :)
> 
> 
> >
> >    o  Section 13: The flag values are not aligned with this text
> "The
> >       lowest-order bit of the Flags field is 0 for IPv4 and 1
> for IPv6".
> >       Unless I'm mistaken, the check should be against next
> lowest flag.
> >       If my understanding is correct, then the following should
> be updated:
> >
> >   OLD:
> >      6: Withdrawal of Unknown Record (fatal):  The received PDU
> has
> >         Flag=0, but a matching record ({Prefix, Len, Max-Len,
> ASN} tuple
> >         for an IPvX PDU or {SKI, ASN, Subject Public Key} tuple
> for a
> >         Router Key PDU) does not exist in the receiver's
> database.
> >
> >      7: Duplicate Announcement Received (fatal):  The received
> PDU has
> >         Flag=1, but a matching record ({Prefix, Len, Max-Len,
> ASN} tuple
> >         for an IPvX PDU or {SKI, ASN, Subject Public Key} tuple
> for a
> >         Router Key PDU) is already active in the router.
> 
>    6: Withdrawal of Unknown Record (fatal):  The received PDU has
>       Flag=0, but a matching record ({Prefix, Len, Max-Len, ASN}
> tuple
>       for an IPvX PDU, or {SKI, ASN, Subject Public Key} tuple for
> a
>       Router Key PDU), or Customer Autonomous System for an ASPA
> PDU
>       does not exist in the receiver's database.
> 
>    7: Duplicate Announcement Received (fatal):  The received PDU
> has
>       Flag=1, but a matching record ({Prefix, Len, Max-Len, ASN}
> tuple
>       for an IPvX PDU or {SKI, ASN, Subject Public Key} tuple for
> a
>       Router Key PDU), or Customer Autonomous System for an ASPA
> PDU is
>       already active in the router.
> 
> >    o Section 15: I suggest to update the following text to
> reflect
> >    only v2 changes
> >
> > OLD:
> >    All existing entries in the IANA "rpki-rtr-pdu" registry
> remain valid
> >    for protocol version 0.  All of the PDU types allowed in
> protocol
> >    version 0 are also allowed in protocol version 1, with the
> addition
> >    of the new Router Key PDU.  To reduce the likelihood of
> confusion,
> >    the PDU number used by the Router Key PDU in protocol version
> 1 is
> >    hereby registered as reserved (and unused) in protocol
> version 0.
> >
> >    o  Section 15: The following should be updated because code 8
> does
> >       not apply for all versions.
> >
> >    OLD:
> >       All previous  entries in the IANA "rpki-rtr-error"
> registry remain
> >       valid for all protocol versions.
> 
>    All of the PDU types in the IANA "rpki-rtr-pdu" registry in
> protocol
>    versions 0 and 1 are also allowed in protocol version 2, with
> the
>    addition of the new ASPA PDU.
> 
> >
> >    # Nits:
> >
> >    o  Section 1.2: Please expand ASPA in the following text:
> >
> >       "*  A new ASPA  PDU type (Section 5.12) has added to
> support
> >          [I-D.ietf-sidrops-aspa-profile]."
> >
> >    o  Section 1.2: Please expand ROA in the following text:
> >
> >    "*  A small section, Section 11, has been added to handle two
> ROA  PDU
> >       race conditions, Break Before Make and Shorter Prefix
> First."
> 
> sure
> 
> >    o  Section 1.2: Consider the following change
> >
> >    OLD:
> >       *  The protocol version number incremented from 1 (one) to
> 2 (two)
> >          and the Section 7 section has been updated accordingly.
> >
> >    NEW:
> >       *  The protocol version number incremented from 1 (one) to
> 2 (two)
> >          and Section 7 has been updated accordingly.
> 
> yup
> 
> >    o  Section 5.1: Please fix this nit
> >
> >    OLD:
> >      AS Count as well as the Provider AS Numbers list MUST BE
> zero.
> >
> >    NEW:
> >      AS Count as well as the Provider AS Numbers list MUST be
> zero.
> 
> sure
> 
> >    o  Section 5.5: Consider making this change for consistency
> with the
> >       definition of flags in Section 5.1:
> >
> > OLD:
> >    When replying to a Reset Query
> >    (Section 5.4), the cache sends the set of all data records it
> has; in
> >    this case, the withdraw/announce field in the payload PDUs
> MUST have
> >    the value 1 (announce).
> >
> > NEW:
> >    When replying to a Reset Query
> >    (Section 5.4), the cache sends the set of all data records it
> has; in
> >    this case, the announce/withdraw field in the payload PDUs
> MUST have
> >    the value 1 (announce).
> 
> ok
> 
> >    o  Section 5.12: Consider these changes
> >
> >    (1)
> >
> >    OLD:
> >       The ASPA PDU is to support [I-D.ietf-sidrops-aspa-
> profile].
> >
> >    NEW:
> >      The ASPA PDU is meant to support [I-D.ietf-sidrops-aspa-
> profile].
> >
>    The ASPA PDU supports [I-D.ietf-sidrops-aspa-profile].  An ASPA
> PDU
> 
> >    OLD:
> >       withdrawn PDU and a new announced PDU.
> >
> >    NEW:
> >       withdrawn PDU and a new announced ASPA PDU.
> 
>    race condition when a BGP announcement is received between a
>    withdrawn ASPA PDU and a newly announced ASPA PDU.
> 
> >
> >    (3) This text is redundant with  what is already provided in
> 5.2.
> >        Consider deleting it:
> >
> >       The Flags field is defined as follows:
> >
> >           Bit     Bit Name
> >           ----    -------------------
> >            0      AFI (IPv4 == 0, IPv6 == 1)
> >            1      Announce == 1, Delete == 0
> >            2-7    Reserved, must be zero
> >    (4)
> >
> >    OLD:
> >      MUST BE ignored by the router when the Flags field
> indicates a
> >
> >    NEW:
> >      MUST be ignored by the router when the Flags field
> indicates a
> 
> ok
> 
> >    o  Section 5.12:
> >
> >    "Receipt of an ASPA PDU announcement
> >    (Flag.Announce == 1 ) when the router already has an ASPA PDU
> with the
> >    same Customer Autonomous System Number and the same Address
> Family
> >    (see Flags field), replaces the previous one."
> >
> >       I would refer to announce/withdraw flag instead of
> "Flag.Announce"
> >       for consistency.
> 
> hokay
> 
> >    o  Section 11:
> >
> >    "When a cache is sending ROA PDUs to the router, especially
> during an
> >    initial full load, two undesirable race conditions are
> possible:"
> >
> >       *  Not sure what is meant by "initial full load" here.
> >
> >       *  Consider s/ROA PDUs to the router/ROA PDUs to a router
> 
>       When a cache is sending ROA PDUs to a router, especially an
>       initial full load in response to a Reset Query PDU, two
>       undesirable race conditions are possible:
> 
> >    Also, fix this nit:
> >
> >    OLD:
> >      This is is a case of "make before
> >
> >    NEW:
> >      This is a case of "make before
> 
> whoops
> 
> >    o  Section 15: Consider the following change
> >
> >   OLD:
> >      This section only discusses updates required in the
> existing IANA
> >      protocol registries to accommodate version 1 of this
> protocol.  See
> >      [RFC8210] for IANA considerations from the original
> (version 0)
> >      protocol.
> >
> >   NEW:
> >      This section only discusses updates required in the
> existing IANA
> >      protocol registries to accommodate version 2 of this
> protocol.  See
> >      [RFC8210] for IANA considerations from the original
> (version 1)
> >      protocol.
> 
>    This section only discusses updates required in the existing
> IANA
>    protocol registries to accommodate version 2 of this protocol.
> See
>    [RFC8210] for IANA considerations of the previous (version 1)
>    protocol.
> 
> whew!
> 
> thanks again.
> 
> randy
> 
> --
> last-call mailing list
> last-call@xxxxxxxx
> https://www.ietf.org/mailman/listinfo/last-call

_________________________________________________________________________________________________________________________

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.

-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call




[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux