Subject: [Lsr] Tsvart last call review of draft-ietf-isis-te-app-13

From: Kyle Rose via Datatracker <[email-masked]>
Date: Wed, 20 May 2020 16:14:31 -0700

Reviewer: Kyle Rose
Review result: Ready with Nits

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
[email-masked] if you reply to or forward this review.

This document describes a mechanism for specifying per-application attributes
for link state routing using IS-IS. It proposes no routing functionality
changes to applications making use of such attributes, only enabling the
ability to choose which attributes in an advertisement apply to a particular
application, and consequently I see no specifically transport-related issues in
this draft.

Nonetheless, I have a few comments.

* Update language?

There are several places in which it is possible that normative language in
prior RFCs is revised. For example, in section 6.1, the last paragraph states:

  New applications which future documents define to make use of the
  advertisements defined in this document MUST NOT make use of legacy
  advertisements.

This looks like it was written in such a way as to avoid requiring such
updates, but it would be good to confirm that there is no normative language in
older documents that would conflict with this requirement.

* Encoding

In section 4.1, the bit masks are defined with a 7 bit length field for which
only 4 bits are useful (values 0-8). It may make sense to keep the 3 high order
bits as "reserved" and set to zero, but either way it would be nice to
understand the justification for whatever design decision is made.

You go to some length to save space (e.g., a zero-length SABM means "all
applications") but always include an octet for UDABM length, which I presume
will be zero outside the lab in most cases. How much does an extra octet cost?
If it's a lot, you could use the three high-order bits to represent the first
three applications (RSVP-TE, SRTE, and LFA) and save yourself an octet until a
fourth application appears.

For that matter, how likely are you to get to 256 standardized applications
under IS-IS?

The fallback from application-specific advertisements to legacy advertisements
is not entirely clear. It sounds like:

 - An application is to use the legacy advertisements if there is at least one
 application specific advertisement for that application with L=1, in which
 case *all* advertisements for that application must also have L=1. - An
 application is to use the application-specific advertisements if there is at
 least one application specific advertisement for that application with L=0, in
 which case *all* advertisements for that application must also have L=0, *and*
 that application is to ignore all legacy advertisements.

In effect, use of legacy advertisements vs. app-specific advertisements is
all-or-nothing. If that's the case, wouldn't a top-level TLV specifying a
legacy mask for applications be more compact, less redundant, and further
reduce the overall number/size of advertisements?

Anyway, I'm out of my element, so feel free to ignore these comments if I'm way
off the mark.

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Thu, 21 May 2020 06:28:16 +0000

Kyle -

Thanx for the review.
Responses inline.

> -----Original Message-----
> From: Kyle Rose via Datatracker <[email-masked]>
> Sent: Wednesday, May 20, 2020 4:15 PM
> To: [email-masked]
> Cc: [email-masked]; [email-masked]; [email-masked]
> Subject: Tsvart last call review of draft-ietf-isis-te-app-13
> 
> Reviewer: Kyle Rose
[Les:] For applications which preexisted the writing of this draft (the ones defined in Section 7.4) there are existing deployments which use legacy advertisements.
Transitioning to use new advertisements has to account for partial deployment of such support.
And the transition has to be managed by the network operator using knobs provided by implementations. This is onerous - but unavoidable in these cases.

For new applications we want to avoid this complexity/pain. So we specify new applications MUST use the new advertisements from day ONE.

As these are new applications there is no legacy to deal with and no existing specification which would be in conflict.

> For that matter, how likely are you to get to 256 standardized applications
[Les:] The limitation for the xABM length to be no more than 8 bytes was introduced only recently based on a review comment.
The concern was that without a limit, it would be theoretically possible for the ABM itself to consume the full sub-TLV space, leaving no room for the actual link attribute advertisements.
So we placed a maximum length to avoid this potential issue.
As each application consumes one bit, with an 8 byte maximum length we can support 64 applications.
This seems more than we will ever get to - but if anyone in the WG thinks this is not large enough they should speak now so we can increase the maximum.

There are existing implementations of this draft which have been deployed. Changing the bit positions as you suggested would not be backwards compatible.
I do not think the small space savings would be worth the trouble.
 
> reduce the overall number/size of advertisements?
[Les:] The information being advertised here (link attributes) is necessarily associated with a top level TLV describing a link to a neighbor (TLVs 22, 23, 25, 141, 222, and 223) . Without that context the link attribute information is meaningless.

The options regarding use of legacy are discussed in Sections 6.1 and 6.3.

   Les

---

From: Kyle Rose <[email-masked]>
Date: Thu, 21 May 2020 09:08:30 -0400

On Thu, May 21, 2020 at 2:28 AM Les Ginsberg (ginsberg) <[email-masked]>
wrote:

> > updates, but it would be good to confirm that there is no normative
> language
> > in
> [Les:] For applications which preexisted the writing of this draft (the
> ones defined in Section 7.4) there are existing deployments which use
> legacy advertisements.
> And the transition has to be managed by the network operator using knobs
> provided by implementations. This is onerous - but unavoidable in these
> cases.
>
> For new applications we want to avoid this complexity/pain. So we specify
> new applications MUST use the new advertisements from day ONE.
>
> As these are new applications there is no legacy to deal with and no
> existing specification which would be in conflict.
>

That's what I figured. Is it the case that new standardized applications
require an RFC? If so, I think this is fine.

> [Les:] The limitation for the xABM length to be no more than 8 bytes was
> introduced only recently based on a review comment.
> The concern was that without a limit, it would be theoretically possible
> for the ABM itself to consume the full sub-TLV space, leaving no room for
> the actual link attribute advertisements.
Sigh... yes, math is hard.

> This seems more than we will ever get to - but if anyone in the WG thinks
> this is not large enough they should speak now so we can increase the
> maximum.
>

I suspect even 64 is safely more than you'll need, but if not there's
always the possibility of a new TLV later on. :-)

> There are existing implementations of this draft which have been deployed.
> Changing the bit positions as you suggested would not be backwards
> compatible.

Understood, though I will point out that this illustrates the potential
harm in deploying something hard to change prior to standardization.
Thankfully, IS-IS is not interdomain, meaning that any such change would
impose costs only on those who deployed early.

> [Les:] The information being advertised here (link attributes) is
> necessarily associated with a top level TLV describing a link to a neighbor
> (TLVs 22, 23, 25, 141, 222, and 223) . Without that context the link
> attribute information is meaningless.
>

Yes, this makes sense, and probably would have been obvious were I more
familiar with IS-IS and link state routing in general.
Thanks,
Kyle

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Thu, 21 May 2020 14:44:07 +0000

Kyle –

Inline.

From: Kyle Rose <[email-masked]>
Sent: Thursday, May 21, 2020 6:09 AM
To: Les Ginsberg (ginsberg) <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: Tsvart last call review of draft-ietf-isis-te-app-13

That's what I figured. Is it the case that new standardized applications require an RFC? If so, I think this is fine.

[Les:] Yes. In order to obtain a new bit in the new Link Attribute Application Identifier Registry defined in Section 7.4 the process defined in RFC 8126 Standards Action is required – which means an RFC has to be written for the new application.

I suspect even 64 is safely more than you'll need, but if not there's always the possibility of a new TLV later on. :-)

Understood, though I will point out that this illustrates the potential harm in deploying something hard to change prior to standardization. Thankfully, IS-IS is not interdomain, meaning that any such change would impose costs only on those who deployed early.
[Les:] Agreed – but it is also good to have demonstrated the viability of the extensions – as well as interoperability – before going to RFC. So early implementations have great value.
But, also, let’s not overvalue the space optimization.
IS-IS has always tried to be “frugal” in its use of space – but I think we can afford the extra byte. 😊

Yes, this makes sense, and probably would have been obvious were I more familiar with IS-IS and link state routing in general.
[Les:] Appreciate the thought you put into your review.

   Les

Thanks,
Kyle