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
