Re: [Lsr] New Version Notification for draft-pkaneria-lsr-multi-tlv-02.txt

2022-12-07 Thread Acee Lindem (acee)
Speaking as WG member.

See one inline.

From: Lsr  on behalf of Tony Li 
Date: Tuesday, December 6, 2022 at 6:23 PM
To: "Les Ginsberg (ginsberg)" 
Cc: Bruno Decraene , lsr 
Subject: Re: [Lsr] New Version Notification for 
draft-pkaneria-lsr-multi-tlv-02.txt


Bruno, Les,

Some responses inline – speaking only for myself – not necessarily for all of 
the co-authors…


Likewise.




1.
“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.”

I would rather call for a « must not ».

From a manageability standpoint, since the burden is now on the network 
operators to ensure that this will work/not break the network, for existing 
TLV, this seem to call:
[LES:] There was never a choice here – the burden is on network operators – and 
would be even if an advertisement of MP support were provided. Why? Because 
there is no safe way for implementations to react to changes in the network 
state regarding MP support without risking flooding storms.
And, repeating a point I have made in the past, suppressing advertisement of 
MP-TLVs when the current configuration requires sending them does not result in 
a working network.
I don’t think it is a productive discussion to try to determine which condition 
is worse:

a)To send MP-TLVs in the presence of one or nodes which are unable to process 
them or
b)To suppress sending of some information required by the existing configuration

Either way, the network will not be working as expected.


I disagree with Les, I don’t see how flooding storms can ensue, and never have. 
 But we’ve been over that point and I see no point in beating a dead horse. 
There was really no reason to bring it up again.

More specifically, I don’t see that “MUST NOT” buys us anything over “should 
not”.  What are we going to do if the operator chooses not to comply? Write a 
ticket? Wag our finger at them? If it was a protocol implementation feature, we 
would shrug our shoulders and say “bad implementation”. Since this is 
operational, and the consequences will impact the operator, I don’t think we 
would have any impact, especially after the fact.



-  for a MUST implement knob to enable/disable (MAY/SHOULD on a per TLV 
basis?). And possibly a SHOULD NOT be enabled by default. Current text says 
RECOMMENDED and I don’t feel that this is enough given that this involves an 
interop issue, and that some vendors/implementers tends to only implements MUST.

[LES:] I am fine either way. But as you are trying to apply normative language 
to a behavior which is not reflected “on the wire”, it is hard to see that 
there is any ability to enforce this.
SHOULD/RECOMMENDED seems appropriate to me.


I concur with Les here. This is not meant to be a BCP about how to run the 
network.


 - a CLI and I would also suggest the document to specify a YANG extension to 
allow network operators to know whether a given implementation support this or 
not (on a per TLV basis?)

[LES:] This makes sense to me.


I can get behind saying that there should be a CLI output and a YANG model 
extension/augmentation.  If you’re asking us to actually propose the YANG 
augmentation, I would be very hesitant due to the OpenConfig/IETF split.  
Anything that we would do here seems like a total waste of time and effort.


Since we’d like to complete this document swiftly, let’s defer YANG 
augmentations to 
https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-yang-augmentation-v1/

Thanks,
Acee


 2)

“the key information MUST

   be replicated in additional TLV instances so that all contents

   specific to that key can be identified”


Are we all certain that for all TLVs and sub-TLVs, everyone/implementation will 
agree on what the key is? (especially if the key were to be spread over 
multiple fields or if a sub-TLV were to contain both a key and non-key data).
In order to err on the safe side, I would prefer that this doc specifies the 
key for all existing TLVs.


e.g. 
“4.1<https://datatracker.ietf.org/doc/html/draft-pkaneria-lsr-multi-tlv-02.txt#section-4.1>.
  Example: Extended IS Reachability”

“Optionally one or more of the following identifiers:



  -  IPv4 interface address and IPv4 neighbor address as specified

 in [RFC5305<https://datatracker.ietf.org/doc/html/rfc5305>]



  -  IPv6 interface address and IPv6 neighbor address as specified

 in [RFC6119<https://datatracker.ietf.org/doc/html/rfc6119>] »



If multiple (N) IPv6 interface addresses are advertised, these N sub-TLVs are 
part of the key and must be advertised in all instances./part? Or is a single 
common

ID enough to be used as a key of the interface?



[LES:] Initially, we did not target this draft to serve as “closing the gap” as 
regards existing TLVs where the relevant RFC is not explicit

regarding MP support. However, I think the discussion in the WG has highlighte

Re: [Lsr] New Version Notification for draft-pkaneria-lsr-multi-tlv-02.txt

2022-12-06 Thread Tony Li

Bruno, Les,

> Some responses inline – speaking only for myself – not necessarily for all of 
> the co-authors…


Likewise.


> 
> “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.”
>  
> I would rather call for a « must not ».
>  
> From a manageability standpoint, since the burden is now on the network 
> operators to ensure that this will work/not break the network, for existing 
> TLV, this seem to call:
> [LES:] There was never a choice here – the burden is on network operators – 
> and would be even if an advertisement of MP support were provided. Why? 
> Because there is no safe way for implementations to react to changes in the 
> network state regarding MP support without risking flooding storms.
> And, repeating a point I have made in the past, suppressing advertisement of 
> MP-TLVs when the current configuration requires sending them does not result 
> in a working network.
> I don’t think it is a productive discussion to try to determine which 
> condition is worse:
>  
> a)To send MP-TLVs in the presence of one or nodes which are unable to process 
> them or
> b)To suppress sending of some information required by the existing 
> configuration
>  
> Either way, the network will not be working as expected.


I disagree with Les, I don’t see how flooding storms can ensue, and never have. 
 But we’ve been over that point and I see no point in beating a dead horse. 
There was really no reason to bring it up again.

More specifically, I don’t see that “MUST NOT” buys us anything over “should 
not”.  What are we going to do if the operator chooses not to comply? Write a 
ticket? Wag our finger at them? If it was a protocol implementation feature, we 
would shrug our shoulders and say “bad implementation”. Since this is 
operational, and the consequences will impact the operator, I don’t think we 
would have any impact, especially after the fact.


> -  for a MUST implement knob to enable/disable (MAY/SHOULD on a per TLV 
> basis?). And possibly a SHOULD NOT be enabled by default. Current text says 
> RECOMMENDED and I don’t feel that this is enough given that this involves an 
> interop issue, and that some vendors/implementers tends to only implements 
> MUST.
>  
> [LES:] I am fine either way. But as you are trying to apply normative 
> language to a behavior which is not reflected “on the wire”, it is hard to 
> see that there is any ability to enforce this.
> SHOULD/RECOMMENDED seems appropriate to me.


I concur with Les here. This is not meant to be a BCP about how to run the 
network.


>  - a CLI and I would also suggest the document to specify a YANG extension to 
> allow network operators to know whether a given implementation support this 
> or not (on a per TLV basis?)
>  
> [LES:] This makes sense to me.


I can get behind saying that there should be a CLI output and a YANG model 
extension/augmentation.  If you’re asking us to actually propose the YANG 
augmentation, I would be very hesitant due to the OpenConfig/IETF split.  
Anything that we would do here seems like a total waste of time and effort.


>  2)
> “the key information MUST
>be replicated in additional TLV instances so that all contents
>specific to that key can be identified”
>  
> Are we all certain that for all TLVs and sub-TLVs, everyone/implementation 
> will agree on what the key is? (especially if the key were to be spread over 
> multiple fields or if a sub-TLV were to contain both a key and non-key data).
> In order to err on the safe side, I would prefer that this doc specifies the 
> key for all existing TLVs.
>  
> e.g. “4.1 
> .
>   Example: Extended IS Reachability”
> “Optionally one or more of the following identifiers:
>  
>   -  IPv4 interface address and IPv4 neighbor address as specified
>  in [RFC5305 ]
>  
>   -  IPv6 interface address and IPv6 neighbor address as specified
>  in [RFC6119 ] »
>  
> If multiple (N) IPv6 interface addresses are advertised, these N sub-TLVs are 
> part of the key and must be advertised in all instances./part? Or is a single 
> common 
> ID enough to be used as a key of the interface?
>  
> [LES:] Initially, we did not target this draft to serve as “closing the gap” 
> as regards existing TLVs where the relevant RFC is not explicit 
> regarding MP support. However, I think the discussion in the WG has 
> highlighted that some codepoints have explicit language regarding 
> MP support and some do not – and that it would be useful to have a place 
> where what is implicit is stated explicitly. This draft might be the 
> right place to do that. Be interested in feedback from others on this point.


Long, long ago, Jon Postel used to