Hi Rob,
thanks for your comments, please see inline (##PP)
On 10/06/2020 19:18, Rob Wilton (rwilton) wrote:
Hi Alvaro,
-----Original Message-----
From: Alvaro Retana <aretana.i...@gmail.com>
Sent: 10 June 2020 16:49
To: Rob Wilton (rwilton) <rwil...@cisco.com>
Cc: Acee Lindem (acee) <a...@cisco.com>; draft-ietf-ospf-te-link-attr-
re...@ietf.org; lsr-cha...@ietf.org; lsr@ietf.org; Yingzhen Qu
<yingzhen...@futurewei.com>; The IESG <i...@ietf.org>
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.
##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.
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.
##PP
What is complicated? The logic is actually quite simple - more specific
advertisement wins above a wildcard advertisement.
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)
##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."
- 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.
##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 listed for all applications that
support them. But perhaps this doesn't work well in practice.
##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 document clarity if the behaviour was clearly
specified in section 5.
##PP
sure, I have moved the text from 12.2 to 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.
##PP
Added the drawing of bits.
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.
##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."
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: ...
##PP
changed to the above.
Thanks,
Rob
_______________________________________________
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr
