Hi, Adrian:

No problem for the typo.
But, I would like to correct some points that you may still have for this document:

1) If the operator want to assure their network behave correctly, all the NODEs within the network MUST all support the MP-TLV capabilities, that is, every node can concatenate the segments MP-TLV pieces together, via some undefined “key”.  
That is to say, Support MP-TLV is NOT optional.

2) Support for MP-TLV is NOT configured for per-TLV. You can confirm it via https://datatracker.ietf.org/doc/html/draft-ietf-lsr-multi-tlv-06#section-8.1

3) If the document doesn’t aim to solve all the possible MP-TLV code point, then https://datatracker.ietf.org/doc/html/draft-ietf-lsr-multi-tlv-06#section-8.2 should be removed. And also the statement in its Abstract part: “ This document codifies the common mechanism of extending the TLV content space through multiple TLVs”.

As I said before, it solves partly  two IS-IS TLV code points(TLV 22 and TLV 135, as mentioned as examples).

Anyway, if you follow the discussions more throughly, you may find more problems existing within the document, as also mentioned in your detail review comments.


Aijun Wang
China Telecom

On Nov 13, 2024, at 16:59, Adrian Farrel <[email protected]> wrote:



Hi Aijun,

 

I realised after I sent the email that the word “field” was open to misinterpretation. But, too late! The email was already sent.

 

I meant, “in particular, clarifying what has already been implemented and deployed, and ensuring that others can interoperate safely.”

 

If you read my review carefully you will see that I have raised some concerns about the way the text describes how MP-TLVs can be built (where are the boundaries to partition).

 

I also have an issue about the use of the word “key” without a more formal definition.

 

But I do not have concerns about the definition of a key for each TLV. The key is, I believe, those fields within a TLV that are necessary to determine the scope of applicability of the TLV. For many TLVs there are no such fields because the YLV applies to the advertising node, and that is all. For other TLVs, the fields that define the scope of applicability are “obvious” from the document that defines the TLV. Could that be made clearer by a wiki that lists the fields that comprise the key in each case? Yes. Is that necessary for this document to work? I don’t think so. My reasoning is that:

  • Support for MP-TLV is optional
  • Support for MP-TLV is configured per-TLV
  • It is likely that most TLVs will not reach the 255 byte limit

 

For what it is worth, it has been a very long time since I touched ISIS code, but I believe I could figure out how to make an interoperable implementation after the resolution of my review questions.

 

Cheers,

Adrian

 

From: Aijun Wang <[email protected]>
Sent: 13 November 2024 02:07
To: [email protected]; [email protected]
Cc: [email protected]
Subject:
答复: [Lsr] Debate on draft-ietf-lsr-multi-tlv

 

Hi, Adrian:

 

I admire your efforts to try to improve the readiness of this document.

But, SURPRINGLY, you get the wrong conclusion-----“in particular clarifying what is in the field and ensuring that others can interoperate safely.”

 

We are discussing where is the field(what constitute a key) to ensuing that others can interoperate safely.

Would you like to follow/chime in the discussions on another thread (https://mailarchive.ietf.org/arch/msg/lsr/eDZWKm-bcG1t7WKYYmwqMvlNAoY/), and to review again and make a new judgment/conclusion of this document?

 

 

Best Regards

 

Aijun Wang

China Telecom

 

发件人: [email protected] [mailto:[email protected]] 代表 Adrian Farrel
发送时间: 20241113 5:22
收件人: [email protected]
抄送: [email protected]
主题: [Lsr] Debate on draft-ietf-lsr-multi-tlv

 

Hi,

 

Well, this whole discussion sent me off to read the draft. I have some suggestions to clarify the text that might help with the current discussion, and also some nits that were thrown up by reading the draft. I don't know where this fits into the last call sequence, but I hope my comments are useful to improve the document – feel free to tell me I have misunderstood.

 

I should add, I support the publication of this document for many of the reasons stated: in particular clarifying what is in the field and ensuring that others can interoperate safely.

 

Cheers,

Adrian

 

= Clarification points =

 

Section 3 has:

 

   A TLV is a tuple of (Type, Length, Value) and can be advertised in

   IS-IS packets.  TLVs sometimes contain information, called a key,

   that indicates the applicability of the remaining contents of the

   TLV.  If a router advertises multiple TLV tuples with the same Type

   code in an IS-IS IIH packet or in the set of LSPs for a level with

   the same key value, they are considered a multi-part TLV (MP-TLV).

 

This seems to be the crunch paragraph that could set the scene for the rest of the document, and could help new readers understand the scope.

 

This is clear except for:

  • "key" is a term which I understand, and which I would also have used, but I wondered whether anything is actually called a key in the current IS-IS literature. So I did a search and didn’t find anything – it’s possible my search wasn’t broad enough. But, if you have a citation for the assertion, then please include it and some of the angst will go away. OTOH, if this is a term being defined here, we might strengthen the definition and scope.
  • "sometimes contains" means that sometimes a key is not included in which case, presumably, the mechanisms in this document can still be applied.

 

So maybe…

 

   A TLV is a tuple of (Type, Length, Value) and can be advertised in

   IS-IS packets.  The applicability of the TLV is indicated by the Type

   field, and for the case of some specific Type codes, may be further

   qualified by specific information in the Value that, in this document,

   are collectively called the key. If a router advertises multiple TLV

   tuples in an IS-IS IIH packet or in the set of LSPs for a level, and they

   have the same Type code, they are considered a multi-part TLV

  (MP-TLV) if any key defined for the specific TLV Type also matches

  in the TLVs

 

---

 

In Section 4

 

   Network operators should not enable Multi-part TLVs until ensuring

   that all implementations that will receive the Multi-part TLVs are

   capable of interpreting them correctly.

 

  • Why is that not “SHOULD NOT”?
  • But also, why is that not “MUST NOT”?

 

---

 

The example in 4.1 is ambiguous.

You have previously stated (section 4) that

   the key information MUST

   be replicated in additional TLV instances

This example defines:

   The key consists of the 7 octets of system ID and pseudonode number

   plus the set of link identifiers which are present.

But then:

   The following key information MUST be replicated

   in each of the additional Extended IS Reachability TLVs:

   *  7 octets of system ID and pseudonode number

   *  The set of link identifiers SHOULD be identical in all TLVs which

      are part of the MP-TLV set.

How is that “SHOULD” consistent with the two cases of “MUST”?

 

I should have thought this should read…

   The following key information MUST be replicated

   in each of the additional Extended IS Reachability TLVs:

   *  7 octets of system ID and pseudonode number

   *  The set of link identifiers

 

Except, this is an example, not normative text so the whole should read…

   The following key information is replicated in each of the

   additional Extended IS Reachability TLVs:

   *  7 octets of system ID and pseudonode number

   *  The set of link identifiers

.

---

 

Similarly in 4.2, the use of BCP14 language is not appropriate. So…

OLD

   The key

   information MUST be replicated identically.

NEW

   The key

   information is replicated identically.

END

 

---

 

Somethings we learn in section 5 about how a sender constructs MP-TLVs are:

  • The series of TLVs with the same T and K values may be sent in any order
  • The contents of a Value that is being split across MP-TLVs cannot be arbitrarily split. Any split must be at a sub-TLV boundary (and no lower, i.e., not at a sub-sub-TLV boundary, and certainly not just “some bytes in one MP-TLV and some bytes in another). This is because the receiver “concatenates” the contents of the received Values in any order and it wouldn’t be clear to which sub-TLV the sub-sub-TLV belonged (and, of course, you could just garble the order of bytes).

These points really need to be brought out in section 4 because they instruct the sender.

 

---

 

Section 5 contains:

   If the internals of the TLV do NOT include key information, the

   relevant key can be found in the parent TLV.

 

I am struggling to parse this. I think it is probably simply not adding anything (except confusion).

 

Suppose I have:

   TLV{key, info, sub-TLV1, sub-TLV2}

Here, TLV is the parent and sub-TLV1 and sub-TLV2 are the children

But it is too long. So I send:

   MP-TLV1{key, info, sub-TLV1}

   MP-TLV2{key, sub-TLV2}

Key all sorted. So, I’m trying to find a case where MP-TLV would not include the key and I would have to go to the parent (especially since the text in section 4 says I must put the key in).

 

Probably, all that this says is, if you need to know the key and the key is not in the sub-TLV, look in the parent. That is, of course, true. But it is nothing to do with MP-TLV: just a bit of ISIS know-how.

 

---

 

I think there is some confusion in Section 6 with:

  Multi-part

   procedures may also be applicable to codepoints which do not support

   sub-TLVs, but which define an unbounded number of attributes which

   may be advertised in a single codepoint.  An example of the latter is

   GMPLS-SRLG as defined in [RFC5307].

 

There is some confusion over the wording here because 5307 *does* use sub-TLVs, even if the SRLG sub-TLV has an unbounded number of attributes (controlled by the length field).

 

How about…

  Multi-part

   procedures may also be applicable both to TLV codepoints which do not

   support sub-TLVs and to those which define an unbounded number of

   attributes which may be advertised within a single sub-TLV.  An example

   of the latter is GMPLS-SRLG as defined in [RFC5307].

 

However, my concern (expressed in a comment on section 5) is that you can’t make MP-TLVs arbitrarily because of the rule that says you can re-assemble in any order.

 

= Nits =

 

Section 1 has:

 

   Today, for example, the Extended IS Reachability TLV (22) [RFC5305]

   and MT Intermediate Systems TLV (222) [RFC5120] are TLVs where

   existing standards do not specify sending multiple TLVs for the same

   object and no other mechanism for expanding the information carrying

   capacity of the TLV has been specified.

 

Consider that you hope to publish this as an RFC that will persist. You need text that will survive the test of time, and also will not contradict the document itself. How about...

 

   For example, [RFC5305] defines the Extended IS Reachability TLV (22)

   and [RFC5120] defines the MT Intermediate Systems TLV (222). Those

   documents do not specify sending multiple TLVs for the same object.

 

---

 

Section 1 has:

 

   [RFC7356] has proposed a 16 bit length field for TLVs in flooding

   scoped Protocol Data Units (PDUs), but this does not address how to

   expand the information advertised when using the existing 8-bit

   length TLVs.

 

RFC 7356 is a standards track document, so it didn't "propose", it "defined". How about...

 

   [RFC7356] defined a 16 bit length field for TLVs in flooding scoped

   Protocol Data Units (PDUs), but does not address how to advertise

   more information when using the existing 8-bit length TLVs.

 

---

 

Section 1 has:

 

   The mechanism described in this document has not been documented for

   all TLVs previously, so it is likely that some implementations would

   not interoperate correctly if these mechanisms were used without

   caution.

 

This is foundational motivation for the document, so maybe it could be made a little clearer? How about...

 

   The mechanism described in this document has been implemented, but

   without formal documentation, there is a risk that these and new 

   implementations would not interoperate correctly. This document provides

   the necessary protocol definition.

 

---

 

Finally in Section 1:

 

   The mechanism described in this document has been used explicitly by

   some implementations, so this document is not creating an

   unprecedented mechanism.  It is specifying a means for extending TLVs

   where no extension mechanism has been previously specified, and

   defining a default extension mechanism for future TLVs, if they

   choose not to specify another extension mechanism.  The mechanism

   described in this document is applicable to top level TLVs as well as

   any level of sub-TLVs which may appear within a top level TLV.

 

This feels like an over-sell to me. Having already had the previous paragraph, you could safely slim this down to:

 

   This document specifies a means for extending TLVs where no extension

   mechanism has been previously specified, and defines a default extension

   mechanism for future TLVs. The mechanism described in this document is

   applicable to top level TLVs as well as any level of sub-TLVs that may appear

   within a top level TLV.

 

---

 

Section 5 has a couple of instances of “NOT”. It is general good practice to only use upper case in the BCP14 phrases.

_______________________________________________
Lsr mailing list -- [email protected]
To unsubscribe send an email to [email protected]
_______________________________________________
Lsr mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to