Re: [Lsr] draft-ginsberg-lsr-isis-invalid-tlv

[<[e-mail]>]

Les,

Thank you for accommodating my comments.
-02 looks good.
1 follow up comment:

§3.2  "ISes MUST NOT accept purges that contain TLVs other than the authentication TLV"

§3.1 "   Therefore TLVs in a PDU which are disallowed MUST be
   ignored and MUST NOT cause the PDU itself to be rejected by the
   receiving IS."

I would assume that a purge is a PDU. Hence both sentences seems to contradict as they are both related to same case (a TLV which is disallowed in a PDU) while they require opposite actions:

-          The first sentence calls for not accepting the PDU

-          The second sentence calls for not rejecting the PDU.

I understand that purge with authentication TLV is an exception, but the "MUST NOT" in §3.1 does not allow for exception.
May be
OLD: Therefore TLVs in a PDU which are disallowed
NEW: Therefore, in a non-purge PDU, TLVs which are disallowed

Thanks you,
Regards,
--Bruno

From: Les Ginsberg (ginsberg) [mailto:[e-mail]]
Sent: Wednesday, March 27, 2019 12:32 AM
To: DECRAENE Bruno TGI/OLN; [e-mail]; [e-mail]
Cc: Alvaro Retana
Subject: RE: draft-ginsberg-lsr-isis-invalid-tlv

Bruno -

Thanx for your comments.

V2 of the draft has been posted which addresses a number of the issues.
More inline.

From: [e-mail] <[e-mail]>
Sent: Friday, March 22, 2019 4:02 AM
To: [e-mail]; [e-mail]
Cc: Alvaro Retana <[e-mail]>
Subject: RE: draft-ginsberg-lsr-isis-invalid-tlv

§4
   The presence of a TLV (or sub-TLV) with content which does not
   conform to the relevant specification MUST NOT cause the LSP itself
   to be rejected.

Again, thank you for your effort to clarify the specification.

Given the "MUST", I feel that this sentence may be read as contradicting with some other text. E.g

"   An implementation that implements HMAC-MD5 authentication and
   receives HMAC-MD5 Authentication Information MUST discard the PDU if
   the Authentication Value is incorrect."
https://tools.ietf.org/html/rfc5304#section-2

Do you think it could be slightly rephrased? May be something along :s/rejected/considered invalid     [or incorrect, although 10589 seems to use the term "invalid PDU"]
(as "rejected" could be read as similar to "discarded")

[Les:] The behavior described in RFC 5304 is normative (i.e., MUST is both intentional and correct) and implementations which conform to RFC 5304 will indeed discard such a PDU.
If your concern is about the word "discard" - this follows the terminology used in ISO 10589 - for example Section 7.3.14.2e:

"An Intermediate system receiving a Link State PDU with an incorrect LSP Checksum or with an invalid PDU syntax shall

1)       generate a corruptedLSPReceived circuit event,

2)         discard the PDU.

Therefore I prefer to continue to use "discard".


Thanks,
--Bruno


From: Lsr [mailto:[e-mail]] On Behalf Of [e-mail]<mailto:[e-mail]>
Sent: Friday, March 22, 2019 11:41 AM
To: [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>
Cc: Alvaro Retana
Subject: [Lsr] draft-ginsberg-lsr-isis-invalid-tlv

Hi all,

To be clear, I support that document. Thanks for writing it.
I have some comments, mostly asking for even more of this/such document. (although I expect that some comments will trigger a discussion ;-) )

----
§3.1
"Any codes in a received PDU that are not recognised shall be
   ignored."

   Therefore the presence of TLVs in a PDU which are not allowed MUST
   NOT cause the PDU itself to be rejected by the receiving IS.


I don't think that "not recognized" is the same as "not allowed".

IMHO, the fact that unknown TLV are to be ignored seem rather obvious as this is a designed goal of the TLV format. Yet, it's good that [ISO10589<https://tools.ietf.org/html/draft-ginsberg-lsr-isis-invalid-tlv-01#ref-ISO10589>] explicitly stated it.
I definitely support that LSR defines the behavior for TLV (and sub-TLV) which are not allowed. But I find the justification and reference debatable. I'd rather prefer that we remove it, or at least remove the "Therefore". I'd favor something along
OLD: Therefore
NEW: In addition, this document specifies that
----
§3.3

Registries associated with sub-TLVs are

   associated with the TLV Codepoints Registry and specify in which TLVs

   a given sub-TLV is allowed.  As with TLVs, it is required that sub-

   TLVs which are NOT allowed MUST be ignored on receipt.

In addition to "not recognized" and "not allowed by the registry" I believe that there are others cases that would benefit from been clarified. Such as:
- not allowed by the specification (in a given case/condition)
- REQUIRED (MUST) but not present e.g. "it MUST include this sub-TLV on point-to-point adjacencies" https://tools.ietf.org/html/rfc5305#section-3.3
- Malformed (bad syntax)
- Correctly formed but illegal value (in a given case/condition)

I would like LSR to specify all types of error handling. Either in this document, or in another document if general error handling is considered out of scope of this document.

[Les:] Error handling associated with incorrect encoding of a TLV is specific to the individual TLV. It is the responsibility of the document which defines the TLV encoding to specify what to do if a particular field does not conform to the set of allowed values, or if the TLV omits required information, or a Reserved bit is set,  etc. I don't think using a central document to define error handling for each individual TLV is a good option.
---

§1

"   PDUs which are valid MUST be accepted even if an individual TLV

   contained within that PDU is invalid in some way."



I wish "valid" be further defined. E.g. do you mean from a syntax/parsing point of view? Or do you mean something else?

In particular [ISO10589<https://tools.ietf.org/html/draft-ginsberg-lsr-isis-invalid-tlv-01#ref-ISO10589>] use the terms "invalid PDU syntax". If you mean the same, I'd favor using the same terms. If you mean something different, please clarify the differences.

---

"   Nevertheless, a certain degree of "common knowledge" is assumed on

   the part of implementors in regards to these behaviors."



What do you mean by that? That some "common knowledge" are required for the protocol to (correctly) work but that this knowledge is not written in a specification? If so, I believe LSR should write down such behavior.



"   This document serves to make explicit what is expected.  While it

   does not alter any existing protocol behavior or specifications, it

   is intended to close any gaps between what is explicitly stated and

   what has been "commonly understood"."



Same comment.

A behavior is either specified or not specified. I don't understand the meaning of the term "commonly understood".



[Les:] Revised text I believe addresses these points - which I think were valid comments. Please let us know if you think these points have been adequately addressed.



   Les

Thank you,

Regards,

--Bruno


PS: To clarify, I'm copying Alvaro as an individual contributor, as he expressed interest in error handling in BGP-LS which for some code points may be related to error handing in IS-IS.


_________________________________________________________________________________________________________________________



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.