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

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Alvaro,


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

Yes, hopefully.

> 
> 
> Thanks!
> 
> 
> Alvaro.
> 
> 
> > ----------------------------------------------------------------------
> > 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.
> 
> That sounds like a simple addition.
> 
> 
> 
> > 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.
> 
> 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".
[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 *).

> 
> 
> 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.
[RW] 

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



> 
> 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.
[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.



> 
> 
> 
> > 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.
> 
> 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|          ...
>             +-+-+-+-+-+-+-+-+...
> 
[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.


> 
> 
> 
> > 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?
> 
> 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.
> 
[RW] 

I agree.


> 
> 
> > 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:
> 
> 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...
[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