[OPS-DIR]Re: draft-ietf-rift-kv-tie-structure-and-processing-05 ietf last call Opsdir review

["Head, Jordan" <[e-mail]>]

Italo,

Thank you for your patience and the very thorough review, I think your feedback has improved the spec.

I have posted the updated spec.

Comments inline as jhead>>

From: Italo Busi via Datatracker <[e-mail]>
Date: Friday, October 31, 2025 at 6:40 AM
To: [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: draft-ietf-rift-kv-tie-structure-and-processing-05 ietf last call Opsdir review

[External Email. Be cautious of content]


Document: draft-ietf-rift-kv-tie-structure-and-processing
Title: RIFT Key/Value TIE Structure and Processing
Reviewer: Italo Busi
Review result: Has Issues

*Summary*

The document has issues and nits that should be resolved before publication.

*Issues*

Note that I am not a subject expert: if some of these comments can be addressed
by reading some specific documents, it would be worthwhile adding some explicit
pointers (better indicating also the sections, in addition to the documents).

1) Section 2

If I understand correctly the Key Identifier is unique within Key-Type.
If so, I would mention this explicitly.

      jhead>> The Key Identifier is unique in any context, Key-Type or Sub-Type. I will clarify this.

2) Section 2

The document says that the Values field "SHOULD contain 1 or more elements"

Is your intention to preclude using the KV TIEs to carry empty Values?
If so, why it is a SHOULD and not a MUST? What happens if the Values is empty?

      jhead>> We don’t intend to preclude a scenario with empty values. If an implementer wants to build some semantics around that, the behavior would be defined in their own spec.

3) Section 2

What happens if the Values field length is not an integer multiplier of 4 bytes
(e.g., 2 bytes)? It will be just 2 bytes long or padded to become 4 bytes long?
I think this should be explicitly defined

      jhead>> The draft already RECOMMENDs that a serialized Thrift object is used, if an implementation adheres to this won’t be a factor. Beyond that it’s out of scope for our spec.

4) Section 2.1

If I understand correctly, in this case, the Key Sub-Type is unique within
Key-Type and the Key Identifier MUST be unique Key Sub-Type If so, I would
mention this explicitly.

      jhead>> Good catch. We already state that if Key Sub-Type is used within the Well-Known Key-Type, it MUST be registered (implies unique).
      jhead>> I added that the Key Identifier "SHOULD be unique" within the given Key Sub-Type (which covers both Well-Known and future Key-Types that elect to use a Sub-Type). We simply cannot say MUST here as depending upon the specifications derivation procedures for the Key Identifier, hashing collisions are possible or likely, it would be like telling someone that they MUST NOT accidentally do something.

5) Section 2.1

The document says that the Key Sub-Type MUST be used when the Key-Type is
either Well-Known or Experimental.

As a consequence, it cannot be used for other Key-Type to be defined in future.
Is this the intention?
I am wondering whether the real intent was intention to say that whether the
Key Sub-Type MUST be used or MUST NOT be used is defined by the Key-Type? If
this is the case, I would rephrase the sentence.

      jhead>> Good catch, I’ve rephrased it to say “The Key Sub-Type MUST be used when the Key-Type is either Well-Known or Experimental in order to avoid interoperability issues, but is OPTIONAL for any other Key-Type.”

6) Section 2.1

I am not sure the requirement OPTIONAL should be capitalized.
If I understand correctly, the defining the Key Sub-Type is optional for the
designer of the Key-Type and not optional to be implemented. If the specific
Key-Type (e.g., the Well Known Key-Type) defines the Key Sub-Type, the Key
Sub-Type MUST be implemented. If the specific Key-Type (e.g., the OUI Key-Type)
does not defined the Key Sub-Type, the Key Sub-Type MUST NOT be implemented.

      jhead>> I’m good with removing the normative language from this particular section. I’ll add the normative language to the more specific sub-sections where the behavior would actually be implemented (e.g., Well-Known and Experimental Key-Types will have a mention that it’s a MUST, but others will be normative OPTIONAL as mentioned above.)

7) Section 2.3

The document requires that all the  "implementations SHOULD support" the
Well-Known Key-Types

I am not sure we can RECOMMEND all the implementations to support all the
values, especially sine some of these values can defined in future. IMHO, the
compliancy requirements for the Well-Known Key-Types should be defined in the
RFC that defines the specific Well-Known Key-Type and not here.

Proposed rephrasing: "This section reserves a value in the RIFT Key-Type
Registry to indicate a Well-Known Key-Type.”

      jhead>> My text should be “The Well-Known Key-Type” (singular, not plural) meaning that an implementation should recognize a given Key-Type as Well-Known, not every single Well-Known Key-Type.

8) Section 3

The document says that "Key-Value elements SHOULD NOT be used to carry topology
information used by RIFT itself to perform distributed computations."

Why it is a SHOULD and not a MUST? What happens if a Key-Value element is used
to carry topology information?

      jhead>> The short answer is that you may end up in race conditions between the underlying RIFT convergence and the data you need in the KV-TIEs. We state SHOULD NOT because if someone really wants to, they can, it’s up to the implementer.

9) Section 3.1.1

How to understand the format of the Values field (i.e., which bytes in the
Values field carry the System ID and which bytes carry the Level)? Maybe it is
sufficient to provide a reference where the format and length for System ID and
Level fields are defined.

      jhead>> This is governed by Thrift encoding, I should have a reference here to point to the normative Thrift model in Appendix A. That model then references the “common” model which is defined normatively in the base spec.
      jhead>> Please note that the Thrift model syntax uses words like “required” and “optional”, but these are not considered normative themselves in the context of the document.

10) Section 3.2

This section defines the Key Targets and their processing and the Key Targets
can be present on KV TIEs However, it is not clear where and how the Key
Targets can be present in the KV TIE format

      jhead>> I caught this as I was reading through your review, will fold it back into the spec. In short, the Key Target comes before the underlying KV header.

11) Section 4.2.1

I am not sure we can allocate values with a Description and Reference "To be
defined"

Either remove these values from the registry (Expert Review can be requested at
a later stage) or add the Description and Reference\

      jhead>> Someone else mentioned this too, I have removed those items.

12) Section 4.3

I think that the specification for a new RIFT Well-Known Key Sub-Type should
not only provide the required fields but also how the Values field of the KV
TIE is formatted to carry these fields (see my comment 9 above) It is
worthwhile mentioning this explicitly

      jhead>> This goes back to me referencing the normative Thrift model in the spec. If someone really wants to implement it without using Thrift, it’s out of scope for this document.

13) Section 4.3

I think that the specification for a new RIFT Key-Type should also defined how
the Key Identifier are assigned and, depending on whether Key Sub-Type can be
used in future RIFT Key-Types (see comment 5 above), also whether the Key
Sub-Type is defined or not for that Key-Type. Please consider also the option
to add a flag in Section 4.1.1 to track within the IANA Registry whether the
Key Sub-Type is defined or not for that Key-Type.

      jhead>> I like this a lot, I have added a bullet point to this section to cover this.
      jhead>> If an implementation gets a new a new Key-Type from IANA and that implementation uses Key Sub-Types, those Sub-Types have no relevance outside of that spec. If a new spec wants to leverage an existing Key Sub-Type for a new use case, they do so at their own risk knowing that the behavior could change. I do not think that needs to be specified.

*Nits*

13) Title

Please expand the acronyms in the I-D title: Routing in Fat Trees (RIFT)
Key/Value Topology Information Elements

      jhead>> Sure, I updated it.

14) Abstract and Introduction

If I understand correctly, the RIFT protocols allows to advertise information
using KV TIEs but it does not define the format of KV TIEs. If so, it might be
worthwhile mentioning this explicitly in the Abstract and Introduction.

      jhead>> The higher level structure of the KV-TIE is defined in the RIFT base spec (RFC9692).

15) Abstract and Introduction

In section 3, this document is also defining one Well-Known KT TIE for
tie-breaking It might be worthwhile mentioning this explicitly in the Abstract
and Introduction

      jhead> I’ll add this to the abstract.

17) Figure 6

In Figure 4, the Key-Type allocated value 2 is explicitly shown.
I would align the style and shown explicitly the value 2 for the Key-Type and
the value 127 for the Key Sub-Type fields.

      jhead>> I like this, will do.

18) Section 4:

I think that the name of second registry should be "RIFT Well-Known Key
Sub-Types", as correctly used in the title of section 4.2

      jhead>> Ah yes, good catch. Will adjust.

Regards, Italo