Subject: [Lsr] Robert Wilton's Discuss on draft-ietf-ospf-te-link-attr-reuse-14: (with DISCUSS)

From: Robert Wilton via Datatracker <[email-masked]>
Date: Wed, 10 Jun 2020 07:32:08 -0700

Robert Wilton has entered the following ballot position for
draft-ietf-ospf-te-link-attr-reuse-14: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ospf-te-link-attr-reuse/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

I found parts of this document hard to understand, but I'm not familiar with
the specifics of the protocols.

This discuss is in the vein of "I think that folks might struggle to implement
this correctly/consistently".   In particular I had some questions/concerns
about section 5 which, if clarified, would probably help this document.

In Section 5:

   The ASLA sub-TLV is an optional sub-TLV and can appear multiple times
   in the OSPFv2 Extended Link TLV and OSPFv3 Router-Link TLV.  The ASLA
   sub-TLV MUST be used for advertisement of the link attributes listed
   at the end on this section if these are advertised inside OSPFv2
   Extended Link TLV and OSPFv3 Router-Link TLV.  It has the following
   format:

I think that it would be useful to clarify when/why the ASLA sub-TLV can be
included multiple times.  I.e. when different applications want to control
different link attributes.

   Standard Application Identifier Bits are defined/sent starting with
   Bit 0.  Undefined bits which are transmitted MUST be transmitted as 0
   and MUST be ignored on receipt.  Bits that are not transmitted MUST
   be treated as if they are set to 0 on receipt.  Bits that are not
   supported by an implementation MUST be ignored on receipt.

It was not clear to me what it means if the SABM (or UDABM) fields are entirely
empty.  This paragraph states that they are treated as if they are 0, but
sections 8 and 11 imply that if the field is omitted then it acts as if all
applications are allowed.  Section 12.2 implies that if the field is omitted
then it is as if all applications are allowed unless there there is another
ASLA with the given application bit set, in which case it is treated as being a
0 again.  I think that this document would be helped if the specific behaviour
was defined in section 5, retaining the justification/clarification in the
subsequent sections.

It is also not entirely clear to me exactly how the bits are encoded on the
wire.  My assumption is that if bit 0 is set, then this would sent the highest
bit of the first byte.  E.g. 0x80?  Is that correct?  If not, then I think that
the document needs more text, if so, then an example of the encoding may still
aid readability.

   User Defined Application Identifier Bits have no relationship to
   Standard Application Identifier Bits and are not managed by IANA or
   any other standards body.  It is recommended that bits are used
   starting with Bit 0 so as to minimize the number of octets required
   to advertise all UDAs.

Doesn't this need more constraints to ensure easy interop (i.e. bits default to
0).  Otherwise, it would seem that anyone is allowed to put any value in this
field that they like that could harm interop, or otherwise it might be tricky
to compare a 4 byte UDABM to an 8 byte UDABM?

   This document defines the initial set of link attributes that MUST
   use the ASLA sub-TLV if advertised in the OSPFv2 Extended Link TLV or
   in the OSPFv3 Router-Link TLV.  Documents which define new link
   attributes MUST state whether the new attributes support application
   specific values and as such MUST be advertised in an ASLA sub-TLV.
   The link attributes that MUST be advertised in ASLA sub-TLVs are:

I think that I get what this means, but I find the last two sentences slightly
jarring given than the ASLA TLV is optional.  Perhaps predicate both of these
constraints with "(if supproted)".  E.g., something like,

 Documents which define new link
 attributes MUST state whether the new attributes support application
 specific values and as such MUST be advertised in an ASLA sub-TLV (if
 supported). The link attributes that MUST be advertised in ASLA sub-TLVs (if
 supported) are:

Regards,
Rob

---

From: Alvaro Retana <[email-masked]>
Date: Wed, 10 Jun 2020 11:48:44 -0400

On June 10, 2020 at 10:32:09 AM, Robert Wilton wrote:

Rob:

Hi!

Thanks for the review.

I’m replying for the authors as I think that your
questions/suggestions should be easy to address.  Please see below.

Thanks!

Alvaro.

> This discuss is in the vein of "I think that folks might struggle to
> implement this correctly/consistently". In particular I had some
> questions/concerns about section 5 which, if clarified, would probably help
> this document.
>
> In Section 5:
>

That sounds like a simple addition.

> It was not clear to me what it means if the SABM (or UDABM) fields are
> entirely empty. This paragraph states that they are treated as if they are
> 0, but sections 8 and 11 imply that if the field is omitted then it acts as
> if all applications are allowed. Section 12.2 implies that if the field is
> omitted then it is as if all applications are allowed unless there there is
> another ASLA with the given application bit set, in which case it is treated
> as being a 0 again. I think that this document would be helped if the
> specific behaviour was defined in section 5, retaining the
> justification/clarification in the subsequent sections.

Empty is different than omitted.

Omitted (the length of the field is 0) means: "these attributed apply
to all applications, so I'm not even going to bother setting the
bits".

Empty (the field is present, but all the bits are set to 0) means that
the sender is saying that "no application applies to this set of
attributes".  I can't think of a circumstance when a sender would do
this, as it is basically an empty announcement: it doesn't apply to
anything....but it is also not wrong and would simply not be used.

OTOH, there is a case where the sender can set a bit (or more) for an
application it supports (maybe a new one) -- if the receiver doesn't
support that application it will then ignore the bit, and it will look
as if nothing was set: none of the supported applications (at the
receiver) match.  Again, the receiver will simply not use the
information.

> wire. My assumption is that if bit 0 is set, then this would sent the
> highest bit of the first byte. E.g. 0x80? Is that correct? If not, then I
> think that the document needs more text, if so, then an example of the
> encoding may still aid readability.

Correct.

We should have added a figure similar to what is in the ISIS draft:

             0 1 2 3 4 5 6 7 ...
            +-+-+-+-+-+-+-+-+...
            |R|S|F|          ...
            +-+-+-+-+-+-+-+-+...

> Doesn't this need more constraints to ensure easy interop (i.e. bits
> default to 0). Otherwise, it would seem that anyone is allowed to put any
> value in this field that they like that could harm interop, or otherwise it
> might be tricky to compare a 4 byte UDABM to an 8 byte UDABM?

Good point!

Even if user-defined, I think that text similar to standard
applications could be added:

   Undefined bits which are transmitted MUST be transmitted as 0
   and MUST be ignored on receipt.  Bits that are not transmitted MUST
   be treated as if they are set to 0 on receipt.  Bits that are not
   supported by an implementation MUST be ignored on receipt.

> I think that I get what this means, but I find the last two sentences
> slightly jarring given than the ASLA TLV is optional. Perhaps predicate
> both of these constraints with "(if supproted)". E.g., something like,
>

Sure...

The ASLA sub-TLV is optional from the protocol point of view: it is
not expected in every OSPF advertisement.  IOW, only implementations
supporting this specification would even know about it.

Given that the MUSTs are within the context of this document, then I
think that "if supported" is not needed...

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Wed, 10 Jun 2020 17:18:02 +0000

Hi Alvaro,

> -----Original Message-----
> From: Alvaro Retana <[email-masked]>
> Sent: 10 June 2020 16:49
> To: Rob Wilton (rwilton) <[email-masked]>
> Cc: Acee Lindem (acee) <[email-masked]>; draft-ietf-ospf-te-link-attr-
> [email-masked]; [email-masked]; [email-masked]; Yingzhen Qu
> <[email-masked]>; The IESG <[email-masked]>
> Subject: Re: Robert Wilton's Discuss on draft-ietf-ospf-te-link-attr-
> reuse-14: (with DISCUSS)
> 
[RW] 

Yes, hopefully.

> to all applications, so I'm not even going to bother setting the
[RW] 

I don’t particularly like this as the solution, since if feels inconsistent to me.  I.e. if you don’t advertise a bit always treat it as zero, unless you don’t say anything at all, in which case treat it as 1.  I would have preferred for it to have an explicit flag bit in the ASLA TLV to indicate that all applications are supported ... more below (see *).

> anything....but it is also not wrong and would simply not be used.
[RW] 

I agree.  Arguably, the document could state that at least one bit in the SABM or UDABM flags SHOULD be set.

> receiver) match.  Again, the receiver will simply not use the
[RW] 

This is okay with me.

But it is this text in section 12.2 that seems to define different/complicated semantics:

12.2.  Use of Zero Length Application Identifier Bit Masks

   If link attributes are advertised associated with zero length
   Application Identifier Bit Masks for both standard applications and
   user defined applications, then any Standard Application and/or any
   User Defined Application is permitted to use that set of link
   attributes so long as there is not another set of attributes
   advertised on that same link which is associated with a non-zero
   length Application Identifier Bit Mask with a matching Application
   Identifier Bit set.  If support for a new application is introduced
   on any node in a network in the presence of such advertisements,
   these advertisements are permitted to be used by the new application.
   If this is not what is intended, then existing advertisements MUST be
   readvertised with an explicit set of applications specified before a
   new application is introduced.

I read this as:

If a sender only advertises 0 for SABM and UDABM length fields then the associated ASLA link attributes apply to all applications.

However, if a different router advertises 0 for SABM and UDABM length fields in an ASLA TLV but includes a second ASLA TLV with an SABM or UDABM bit (possibly with one or more link attributes), then the attributes associated with the first ASLA TLV, that match the application bit in the second ASLA TLV, are now disabled rather than being enabled.  I think that I can understand why this is done, but it also seems ... complicated.

I would have preferred to see something like the following, stated in section 5:
- A given link attribute should only appear in a single ASLA TLV, and is ignored if it appears in a second ASLA TLV (already stated)
- The link attributes in an ASLA TLV can either:
   (i) apply to all applications (preferable indicated with an explicit flag (*), or otherwise by SABM and UDABM length fields both = 0), or
   (ii) only applies to the specified bit list of applications.

I.e. I would prefer the first part of 12.2 text is dropped, which really means that if the link attributes are being explicitly advertised for one application, then they should be be explicitly listed for all applications that support them.  But perhaps this doesn't work well in practice.

However, I appreciate that this suggestion is perhaps beyond my remit.  Hence, keeping the existing behaviour is okay, abeit more complicated that I like, but I do think that it would aid the document clarity if the behaviour was clearly specified in section 5.

> We should have added a figure similar to what is in the ISIS draft:
[RW] 

Yes, having reviewed the ISIS draft after the OSPF draft, I found the equivalent section in the ISIS draft easier to understand.  Hence, adding the diagram would be good.

>    supported by an implementation MUST be ignored on receipt.
[RW] 

I agree.

> think that "if supported" is not needed...
[RW] 

I don't feel too strongly on this point, so if you wish to leave it as is that is okay with me ...  although on further reading it is worth nothing that both the link attribute and ASLA TLV are optional.  An alternative way of wording this could be:

Documents which define new link attributes MUST state whether the new attributes support application
specific values and as such are advertised in an ASLA sub-TLV.  The standard link attributes that are advertised in ASLA sub-TLVs are: ...

Thanks,
Rob

---

From: Peter Psenak <[email-masked]>
Date: Thu, 11 Jun 2020 12:30:09 +0200

Hi Rob,

thanks for your comments, please see inline (##PP)

>> That sounds like a simple addition.

##PP
Updated as:

The ASLA sub-TLV is an optional sub-TLV of OSPFv2 Extended Link TLV and
OSPFv3 Router-Link TLV. Multiple ASLA sub-TLVs can be present in its 
parent TLV when different applications want to control different link 
attributes or when different value of the same attribute needs to be 
advertised by multiple applications.

> However, if a different router advertises 0 for SABM and UDABM length fields in an ASLA TLV but includes a second ASLA TLV with an SABM or UDABM bit (possibly with one or more link attributes), the…

##PP
What is complicated? The logic is actually quite simple - more specific 
advertisement wins above a wildcard advertisement.

> - A given link attribute should only appear in a single ASLA TLV, and is ignored if it appears in a second ASLA TLV (already stated)

##PP
This is already present in the draft:

   "If the same attribute is advertised in more than single ASLA sub-TLVs
    with the application listed in the Application Bit Masks, the
    application SHOULD use the first instance of advertisement and ignore
    any subsequent advertisements of that attribute."

>     (ii) only applies to the specified bit list of applications.

##PP
Above is exactly what is proposed.  I do not see a need for a special 
flag though. The only additional logic is that more specific 
advertisement with app bit set can overwrite the less specific 
advertisement (zero length SABM).

> I.e. I would prefer the first part of 12.2 text is dropped, which really means that if the link attributes are being explicitly advertised for one application, then they should be be explicitly lis…

##PP
there is a good reason for leaving the "Zero Length Application 
Identifier Bit Masks" support there - if there is no need for an app 
specific values, using the wildcard advertisement is simple and efficient.

> However, I appreciate that this suggestion is perhaps beyond my remit.  Hence, keeping the existing behaviour is okay, abeit more complicated that I like, but I do think that it would aid the docum…

##PP
sure, I have moved the text from 12.2 to 5.

> Yes, having reviewed the ISIS draft after the OSPF draft, I found the equivalent section in the ISIS draft easier to understand.  Hence, adding the diagram would be good.

##PP
Added the drawing of bits.

>>     supported by an implementation MUST be ignored on receipt.

##PP
Added:

    "Undefined bits which are transmitted MUST be
    transmitted as 0 and MUST be ignored on receipt. Bits that are not
    transmitted MUST be treated as if they are set to 0 on receipt. 
Bits
    that are not supported by an implementation MUST be ignored on
    receipt."
> specific values and as such are advertised in an ASLA sub-TLV.  The standard link attributes that are advertised in ASLA sub-TLVs are: ...

##PP
changed to the above.

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Thu, 11 Jun 2020 13:39:55 +0000

Hi Peter,

Please see inline ...

I think that there is probably just one point of confusion/ambiguity that needs to be resolved.

> From: Peter Psenak <[email-masked]>
> Sent: 11 June 2020 11:30
> To: Rob Wilton (rwilton) <[email-masked]>; Alvaro Retana
> <[email-masked]>
> >>> I found parts of this document hard to understand, but I'm not
> familiar
> >> with
> >>> I think that it would be useful to clarify when/why the ASLA sub-TLV
> can
> >> be
[RW] 

That looks fine.

> >>> 0, but sections 8 and 11 imply that if the field is omitted then it
> acts
> >> as
> >>> if all applications are allowed. Section 12.2 implies that if the
> field
> >> is
> >>> omitted then it is as if all applications are allowed unless there
> there
> >> is
> > I don’t particularly like this as the solution, since if feels
> inconsistent to me.  I.e. if you don’t advertise a bit always treat it as
> zero, unless you don’t say anything at all, in which case treat it as 1.
> I would have preferred for it to have an explicit flag bit in the ASLA TLV
> to indicate that all applications are supported ... more below (see *).
> >
> >>
> >>
> > If a sender only advertises 0 for SABM and UDABM length fields then the
> associated ASLA link attributes apply to all applications.
> >
> > However, if a different router advertises 0 for SABM and UDABM length
> fields in an ASLA TLV but includes a second ASLA TLV with an SABM or UDABM
> bit (possibly with one or more link attributes), then the attributes
> associated with the first ASLA TLV, that match the application bit in the
> second ASLA TLV, are now disabled rather than being enabled.  I think that
> I can understand why this is done, but it also seems ... complicated.
> 
> ##PP
[RW] 

Okay, if that is what you are trying to achieve then I am okay with that, but that was not how I interpreted the text in 12.2, and I suspect that this is the root of why I perceive the algorithm is complex :-).

My interpretation (which I think is valid) was thus:

I.e. by advertising say a single attribute (e.g. "Unidirectional Link Delay") in one ASLA TLV for a specific application changes how the receiver interprets all other attributes e.g. say "Unidirectional Delay Variation" in the wildcard advertisement (due to the following text in 12.2):

   Identifier Bit set.

In particular the "so long as ..." clause seems to change how the default attributes are interpreted.

Probably this paragraph could be improved to make its intent more clear?  I.e. the attributes values in the default ASLA apply for all applications, unless that are overridden by application specific definitions of particular attributes, and all other default attribute value still apply.

> > - A given link attribute should only appear in a single ASLA TLV, and is
> ignored if it appears in a second ASLA TLV (already stated)
> 
> ##PP
[RW] 

Yes, sorry that is what I meant by (already stated).

> >     (i) apply to all applications (preferable indicated with an explicit
> flag (*), or otherwise by SABM and UDABM length fields both = 0), or
[RW] 

I'm okay with this, as long as it is clearly described.

> 
> >
> > I.e. I would prefer the first part of 12.2 text is dropped, which really
> means that if the link attributes are being explicitly advertised for one
> application, then they should be be explicitly listed for all applications
> that support them.  But perhaps this doesn't work well in practice.
> 
> ##PP
[RW] 

Having defaults is fine, I think that it was my different interpretation (or misinterpretation) of your 12.2 text that was throwing me off track.

> 
> >
> > However, I appreciate that this suggestion is perhaps beyond my remit.
> Hence, keeping the existing behaviour is okay, abeit more complicated that
> I like, but I do think that it would aid the document clarity if the
> behaviour was clearly specified in section 5.
> 
> ##PP
[RW] 

Thanks.  As above, I also think that this text needs to be refined/clarified.

> 
> >
> >
> >
> >>
> >>
> >>
> >>> It is also not entirely clear to me exactly how the bits are encoded
> on
> >> the
> >>> highest bit of the first byte. E.g. 0x80? Is that correct? If not,
> then
> >> I
> > Yes, having reviewed the ISIS draft after the OSPF draft, I found the
> equivalent section in the ISIS draft easier to understand.  Hence, adding
> the diagram would be good.
> 
> ##PP
[RW] 
Thanks.

> >>> value in this field that they like that could harm interop, or
> otherwise
> >> it
[RW] 

Looks good.

> >>> supported). The link attributes that MUST be advertised in ASLA sub-
> TLVs
> >> (if
> >>> supported) are:
> >>
> >> Sure...
> >>
> > I don't feel too strongly on this point, so if you wish to leave it as
> is that is okay with me ...  although on further reading it is worth
> nothing that both the link attribute and ASLA TLV are optional.  An
> alternative way of wording this could be:
> >
> > specific values and as such are advertised in an ASLA sub-TLV.  The
> standard link attributes that are advertised in ASLA sub-TLVs are: ...
> 
> ##PP
> changed to the above.
[RW] 

Thanks,
Rob

---

From: Peter Psenak <[email-masked]>
Date: Fri, 12 Jun 2020 10:24:43 +0200

Hi Rob,

> Probably this paragraph could be improved to make its intent more clear?  I.e. the attributes values in the default ASLA apply for all applications, unless that are overridden by application specif…

##PP
ok, I see.

What about this:

    attributes. If support for a new application is introduced

    An application specific advertisement (Application Identifier Bit
    Mask with a matching Application Identifier Bit set) for an attribute
    MUST always be preferred over the advertisement of the same attribute
    with the zero length Application Identifier Bit Masks for both
    standard applications and user defined applications on the same link.

thanks,
Peter

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Fri, 12 Jun 2020 09:21:36 +0000

Hi Peter,

Cutting the response to just the text in question.

> Sent: 12 June 2020 09:25
> > Probably this paragraph could be improved to make its intent more clear?
> I.e. the attributes values in the default ASLA apply for all applications,
> unless that are overridden by application specific definitions of
> particular attributes, and all other default attribute value still apply.
> 
> 
> ##PP
> ok, I see.
> 
> What about this:
> 
> 

[RW] 

This is okay, but could potentially still be improved by giving a name to the wildcard attributes (e.g. wildcard, common, or default).  Possibly splitting the text regarding new applications from the preceding part of the paragraph would help.  I would then also reorder the paragraphs.  Perhaps something along the lines of:

    Wildcard link attributes can be advertised using zero length
    Application Identifier Bit Masks for both standard applications and
    user defined applications.  All Standard and User Defined
    Applications are permitted to use any of the wildcard link attributes.

    An application specific advertisement (Application Identifier Bit
    Mask with a matching Application Identifier Bit set) for a link
    Attribute MUST always be preferred over any wildcard advertisement of
    the link attribute on the same link.

    If support for a new application is introduced on any node in a
    network in the presence of wildcard advertisements, these wildcard
    advertisements are permitted to be used by the new application.
    If this is not what is intended, then existing advertisements MUST be
    re-advertised with an explicit set of applications specified before
    the new application is introduced.

If you prefer your proposed text, or want to wordsmith my proposed text that is fine with me ...

One further comment:  I presume that it is allowed to split the advertisement of wildcard link attributes into multiple ASLA TLVs, as long as they don't overlap.  I don't think that you need to say anything about this, unless my presumption is wrong.

Regards,
Rob

---

From: Peter Psenak <[email-masked]>
Date: Fri, 12 Jun 2020 11:40:19 +0200

Hi Rob,

On 12/06/2020 11:21, Rob Wilton (rwilton) wrote:
> One further comment:  I presume that it is allowed to split the advertisement of wildcard link attributes into multiple ASLA TLVs, as long as they don't overlap.  

sure.

> I don't think that you need to say anything about this, unless my presumption is wrong.

I don't see a need to say anything about that.

thanks,
Peter