Subject: [OSPF] Picky nits in draft-ietf-ospf-te-node-addr-04.txt

From: "Adrian Farrel" <[email-masked]>
Date: Mon, 17 Dec 2007 23:56:59 -0000

Hi,

A few picky nits with your excellent draft.

Cheers,
Adrian

---

Abstract

I have a couple of issues with the Abstract. The first sentence is
ambiguous - seems to say that it enhances the OSPF TE extensions that
already exist to advertise a router's local addresses. And the order
of presentation of material is back to front.

Can I suggest...

OLD
   This document describes procedures that enhance OSPF Traffic
   Engineering (TE) extensions for advertising a router's local
   addresses.  This is needed to enable other routers in a network to
   compute traffic engineered MPLS LSPs to a given router's local
   addresses.  Currently, the only addresses belonging to a router that
   are advertised in TE LSAs are the local addresses corresponding to TE
   enabled links and the local address corresponding to the Router ID.
NEW
   OSPF Traffic Engineering (TE) extensions are used to advertise TE
   Link State Advertisements (LSAs) containing information about TE-
   enabled links. The only addresses belonging to a router that are
   advertised in TE LSAs are the local addresses corresponding to TE-
   enabled links, and the local address corresponding to the Router ID.

   In order to allow other routers in a network to compute Multiprotocol
   Label Switching (MPLS) traffic engineered Label Switched Paths (TE
   LSPs) to a given router's local addresses, those addresses must also
   be advertised by OSPF TE.

   This document describes procedures that enhance OSPF TE to advertise
   a router's local addresses.
===
Section 2
s/to setup/to set up/
s/MPLS TE LSPs/Multiprotocol Label Switching (MPLS) Traffic Engineered Label 
Switched Paths (TE LSPs)/
===
Section 3
Suggest you rename this section "Rejected Potential Solution"
===
Section 4
Assuming you intend this to become an RFC, suggest you rename this section 
"Solution" and make the same change in the text.

s/node attribute TLV. node attribute TLV./node attribute TLV/

s/anticipated that a node attribute TLV/anticipated that the node attribute 
TLV/
===
Section 4.1

Your figure does not match the text. Assuming that the text is correct (i.e. 
you want to allow prefixes, not just full /32 addresses), and assuming that 
you want to use the full 32 bits for each address regardless of the prefix 
length, your figure should look like...

       0                   1                   2                   3
       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |              1                |             Length            |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |   Addr Len 1  |          IPv4 Address 1                       |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |IPv4 Addr cont.|                                               :
      +-+-+-+-+-+-+-+-+                                               ~
      :                               .                               :
      ~                               .               +-+-+-+-+-+-+-+-+
      :                               .               |   Addr Len n  |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                          IPv4 Address n                       |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

But perhaps you need to clarify the text and the figures for IPv4 and IPv6 
to indicate:
- whether Length is in octets
- whether Address is always octet-aligned
- whether Address is always 4/16 octet aligned (IPv4/6)
- whether Length is always a multiple of 5/17 (IPv4/6)

===
Section 4.2
s/SHOULD not/SHOULD NOT/

Having said "SHOULD NOT" don't you need to say what happens if, or why an 
implementation MAY vary this?

You need to clarify:
- What happens if > 1 Node Attribute TLV in an OSPF TE LSA
- What happens if a router advertises two OSPF TE LSAs each with a Node 
Attribute TLV
- What happens if > 1 sub-TLV of the same type in a Node Attribute TLV

===
Section 5

This is a brave section given the current climate :-)

Aren't you advertising information that was not previously advertised? 
Doesn't that increase the risk if the protocol is compromised?
===
Section 6

Suggest you point the IANA at the registry and subregistry by name, to make 
sure they understand where to make the assignment.

Don't you need IANA to manage a new registry for the sub-TLV type values?
===

_______________________________________________
OSPF mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/ospf

---

From: Acee Lindem <[email-masked]>
Date: Tue, 18 Dec 2007 12:54:36 -0500

Hi Rahul, Kireeti,
See one comment inline.
On Dec 17, 2007, at 6:56 PM, Adrian Farrel wrote:

> s/MPLS TE LSPs/Multiprotocol Label Switching (MPLS) Traffic  
> Engineered Label Switched Paths (TE LSPs)/
> ===
> Section 3
> Assuming you intend this to become an RFC, suggest you rename this  
> section "Solution" and make the same change in the text.
>
> s/anticipated that a node attribute TLV/anticipated that the node  
> attribute TLV/
> ===
> Section 4.1
>
> Your figure does not match the text. Assuming that the text is  
> correct (i.e. you want to allow prefixes, not just full /32  
> addresses), and assuming that you want to use the full 32 bits for  
> each address regardless of the prefix length, your figure should  
> look like...
>
>       0                   1                   2                   3
> But perhaps you need to clarify the text and the figures for IPv4  
> and IPv6 to indicate:

If the draft is enhanced to support prefixes (which would satisfy the  
ASON requirement), then I'd suggest the Node IPv6 Local Address Sub- 
TLV use the OSPFv3 method of packing OSPFv3 prefixes into even  
multiples of 32-bit words.

A.4.1.  IPv6 Prefix Representation

    IPv6 addresses are bit strings of length 128.  IPv6 routing
    protocols, and OSPF for IPv6 in particular, advertise IPv6 address
    prefixes.  IPv6 address prefixes are bit strings whose length ranges
    between 0 and 128 bits (inclusive).

    Within OSPF, IPv6 address prefixes are always represented by a
    combination of three fields: PrefixLength, PrefixOptions, and  
Address
    Prefix.  PrefixLength is the length in bits of the prefix.
    PrefixOptions is an 8-bit field describing various capabilities
    associated with the prefix (see Appendix A.4.2).  Address Prefix is
    an encoding of the prefix itself as an even multiple of 32-bit  
words,
    padding with zero bits as necessary.  This encoding consumes
    ((PrefixLength + 31) / 32) 32-bit words.

Thanks,
Acee

>
> ===
> Section 4.2
> s/SHOULD not/SHOULD NOT/
>
> Having said "SHOULD NOT" don't you need to say what happens if, or  
> why an implementation MAY vary this?
>
> You need to clarify:
> - What happens if a router advertises two OSPF TE LSAs each with a  
> Node Attribute TLV
> Aren't you advertising information that was not previously  
> advertised? Doesn't that increase the risk if the protocol is  
> compromised?
> ===
> Section 6
>
> Suggest you point the IANA at the registry and subregistry by name,  
> to make sure they understand where to make the assignment.
>

---

From: Rahul Aggarwal <[email-masked]>
Date: Wed, 26 Dec 2007 12:05:25 -0800 (PST)

Hi Adrian,

Thanks for the comments. Please see below:

>    a router's local addresses.

Done.

> Switched Paths (TE LSPs)/

Done.

> Suggest you rename this section "Rejected Potential Solution"

Done.

> "Solution" and make the same change in the text.

Done.

> s/node attribute TLV. node attribute TLV./node attribute TLV/

Done.

> s/anticipated that a node attribute TLV/anticipated that the node attribute

Done.

> you want to allow prefixes, not just full /32 addresses),

That is correct.

> and assuming that

That is correct too.

Done.

> - whether Address is always 4/16 octet aligned (IPv4/6)

Done.

> - whether Length is always a multiple of 5/17 (IPv4/6)

I don't that is necessary once all the other changes are made.

> s/SHOULD not/SHOULD NOT/

Ok.

> implementation MAY vary this?

I am open to changing this to MUST NOT if there are no objections as this
would just be duplicate information.

> - What happens if > 1 sub-TLV of the same type in a Node Attribute TLV

Will clarify these cases in the next revision.

> Doesn't that increase the risk if the protocol is compromised?

I am open to any suggested text.

> sure they understand where to make the assignment.

Ok.

> Don't you need IANA to manage a new registry for the sub-TLV type values?

That is a good idea. Will add text for that.

rahul

---

From: Rahul Aggarwal <[email-masked]>
Date: Wed, 26 Dec 2007 12:06:21 -0800 (PST)

Hi Acee,

<snipped>

> multiples of 32-bit words.

Thanks for the suggestion. I have made the change.

rahul