Re: rtg dir review of draft-ietf-l2tpext-sbfd-discriminator

"Carlos Pignataro (cpignata)" <[email protected]> Sun, 3 Jan 2016 12:55:12 +0000
Newsgroups gmane.ietf.l2tpext
Message-ID <[email protected]>
--===============0353157483363396667==
Content-Language: en-US
Content-Type: multipart/signed;
 boundary="Apple-Mail=_2D26E11F-8E52-40B0-BD27-5C03417028C6";
 protocol="application/pgp-signature"; micalg=pgp-sha256

--Apple-Mail=_2D26E11F-8E52-40B0-BD27-5C03417028C6
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

Hi Loa,

Thanks much for your review!

I will be posting a new revision addressing all your comments =E2=80=94 =
in the meantime, please see inline.

> On Dec 18, 2015, at 9:22 AM, Loa Andersson <[email protected]> wrote:
>=20
> Hello,
>=20
> I have been selected as the Routing Directorate reviewer for this =
draft. The Routing Directorate seeks to review all routing or =
routing-related drafts as they pass through IETF last call and IESG =
review, and sometimes on special request. The purpose of the review is =
to provide assistance to the Routing ADs. For more information about the =
Routing Directorate, please see =E2=80=8B =
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>=20
> Although these comments are primarily for the use of the Routing ADs, =
it would be helpful if you could consider them along with any other IETF =
Last Call comments that you receive, and strive to resolve them through =
discussion or by updating the draft.
>=20
> Document: draft-ietf-l2tpext-sbfd-discriminator-01.txt
> Reviewer: Loa Andersson
> Review Date: 2015-12-18
> IETF LC End Date: date-if-known
> Intended Status: Proposed Standard (ID says Standards track)
>=20
> Summary:
>=20
>    I have some minor concerns about this document that I think should =
be resolved before publication.
>=20
> Comments:
>=20
>     I have considerable problems reading the draft, first it does not
> really follow RFC 7322 in some important details, also the format =
figure
> (as I understand it) is misleading. The document need a facelift.
>=20
>=20

Will work on it. Thanks. See below.

>=20
> Major Issues:
>=20
>    "No major issues found."
>=20
> Minor Issues:
>=20
>    Even though the nit-picking below is pretty massive, it is purely
> editorial and should be fixed before going to IETF Last Call. I have
> no real concerns about the technical content

Ack =E2=80=94 thanks!

>=20
> Abstract:
> ---------
>    I used often "my immediate manager" as a reference and said that
> the abstract should give her/him a good idea about what the draft is
> about. I don't think the abstract meet that standard. Could you please
> flesh out.

I updated the Abstract a bit, but have in mind it is also very =
consistent with draft-ietf-isis-sbfd-discriminator and =
draft-ietf-ospf-sbfd-discriminator

>=20
>    RFC 7322 says that "Similarly, the Abstract should be complete
> in itself.  It will appear in isolation in publication announcements
> and in the online index of RFCs." If I encounter this and is not up to
> speed on l2tp and bfd, this does not give a good idea what it is =
about.
>=20

Someone not up to speed with the basicmost concepts of BFD and L2TP as =
used in the abstract, should do some additional reading before this =
document...

> Abbreviations
>=20
> RFC 7322 says:
>   Abbreviations should be expanded in document titles and upon first
>   use in the document.  The full expansion of the text should be
>   followed by the abbreviation itself in parentheses.  The exception =
is
>   an abbreviation that is so common that the readership of RFCs can be
>   expected to recognize it immediately; examples include (but are not
>   limited to) TCP, IP, SNMP, and HTTP.  The online list of
>   abbreviations [ABBR] provides guidance.  Some cases are marginal, =
and
>   the RFC Editor will make the final judgment, weighing obscurity
>   against complexity.
>=20
> The abbreviations are not expanded in title, abstract, and some other
> places, nor are they "well-known=E2=80=9D.

Agreed. Good point. Added a Terminology section, and expanded some =
abbrevs on first use.

>=20
> Examples
> AVP - the RFC Editor abbreviation list gives two expansions, since =
this
> is l2tp I come to the conclusion that this is "attribute-value pair" =
and
> not the wellknow "Audio-Visual Profile (AVP)"
>=20
> S-BFD - BFD is not well-known, and S-BFD is not even in the RFC =
Editors
> abbreviations list.
>=20
> L2TPv3 - L2TP or L2TPv3 are not well-know. There is no RFC that does
> not expand the abbreviation in the title.
>=20
> ICRQ, ICRP, OCRQ, and OCRP are sued but not expanded, the pointer to =
where to find the expansions (RFC 3931) are nit give until the third
> time the quartet is mentioned
>=20
> LCCE - used but not expanded. nor well-known.
> The RFC Editor abbreviations has two expansions
> LCCE       - Logical Cluster Computing Environment (LCCE) or
>           - L2TP Control Connection Endpoint (RFCs 3931 and 4719)
> Since this is is L2TP context, it is the latter that is correct, but
> since we have an ambiguity we need to expand.
>=20

Ack. Thanks.

> Section 2
>=20
> I have a problem parsing this sentence:
>=20
>   This AVP is exchanged during session negotiation (ICRQ, ICRP, OCRQ,
>   OCRP).
>=20
> Do you mean to say
>=20
>   The "S-BFD Target Discriminator ID" AVP is exchanged using the ICRQ,
>   ICRP, OCRQ, and OCRP control messages during session negotiations.
>=20

Yes, fixed.

> Section 2.1
>   There is a TBD in the first sentence of this paragraph. While I
>   agree that TDB is "well-know" I prefer using [TBA by IANA],
>   where TBA stands for To Be Assigned.
>   If you change this you also need to change in the IANA section.
>=20

OK =E2=80=94 it really does not matter as this should be assigned before =
publication.

> Excuse me if I don't understand this figure
>=20
>                                                     No. of octets
>                 +-----------------------------+
>                 | Discriminator Value(s)      |     4/Discriminator
>                 :                             :
>                 +-----------------------------+
>=20
> First I think you say that a Discriminator is 4 octets
> Second there can be a variable number of discriminators per attribute
> value field
> The box in your figure seems to 29 bits wide, this is unorthodox.
>=20

It=E2=80=99s the same as in draft-ietf-isis-sbfd-discriminator. The =
figure is clear. It does not show numbers of bits.

>=20
>                  0       1       2       3
>                  01234567012345670123456701234567
>                 +-----------------------------+
>                 | Discriminator Value(s)      |
>                 :                             :
>                 +-----------------------------+
>=20
> Is this what you mean?
>=20
>                  0       1       2       3
>                  01234567012345670123456701234567
>                 +--------------------------------+
>                 | Discriminator Value (1)        |
>                 +--------------------------------+
>                 :                                :
>                 +--------------------------------+
>                 | Discriminator Value (n-1)      |
>                 +--------------------------------+
>                 | Discriminator Value (n)        |
>                 +--------------------------------+
>=20
> Discriminator - a 4 octet value

But sure, I can make it even prettier :-)


>=20
> IANA consideration
>=20
>     There is a practice - which I disagree with - to assume that IANA
> and readers know where to the registries. Please point it out so no
> mistakes are possible.
>=20
> OLD TEXT
> This number space is managed by IANA as per [RFC3438].
>=20
> NEW TEXT
> IANA maintain a sub-registry "Message Type AVP (Attribute Type 0)
> Values" in the "Control Message Attribute Value Pairs" as per
> [RFC3438]. IANA is requested to assign the first free value from this
> sub-registry as the Message typ AVP for "S-BFD Discriminators".
>=20
>=20

Thanks for this text

> Nits:
>=20
> The nits tool does only give us the date warning.
>=20
> /Loa
>=20

Thanks again, Loa!

=E2=80=94 Carlos.


>=20
>=20
> --
>=20
>=20
> Loa Andersson                        email: [email protected]
> Senior MPLS Expert                          [email protected]
> Huawei Technologies (consultant)     phone: +46 739 81 21 64


--Apple-Mail=_2D26E11F-8E52-40B0-BD27-5C03417028C6
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="signature.asc"
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Message signed with OpenPGP using GPGMail

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIcBAEBCAAGBQJWiRovAAoJEIXgpQGOZny97LQP/0YO1/4xVDcVOofAX7EoISxx
CnnlKNOGnZSHZPHhYvOapgyl+9hmK5kmLTHFuzLMWNK86vYIEOQlZEZ3RdpR0tHa
+wCHv5Bn4K9fBXfPF43MvZ4c4ZyCpHhUNTgUrw5zknpOKN28ji6Uj83asPPYVxnE
XoegFxGlg8DgB/iufbYoJrxqXmRn/ZOWX1Hv30iHPOTaVz4JeV6aTLd3aGaqVz48
PX0Rjv0uNtxaZcAMlpkSIe22jym0jhHUlft0/F8xmd+VW+h8mQFs39Q9SbjEWo71
f36erCqH7qld9XMgvueWg+SxPqdLZrH9r4iMBVgjRXIHNulaNmauGPmZPfe0RZCe
69EdjwhBR4MKYoBm0Kgu8uZVKXNikMsRDtm0qFpAdXJSx84A4yQVNCH62IutYeKK
8fGaXy6W7mFigXfXmh2/fq8E15JjQOu9KUj27TjMk9TSy/7ukPcdzhXocu8cMegI
IhBCwVOuFgxe2npBp7Ws2fv8GDEdmBUljBh9GPAXtTybd5AktAKuX7ocq0QslOKz
QK2z6XOLd4QzSIUZrySsRBDuf+7k0w5LC/u8nTHpwCzHudwo4esdQrcWa4sQ/5h3
Hw182GeuBW75rkShEMB2+Fw35nunnyHoFIsP1t7k5n3xqfiT/eqFqxbeUgGghzDQ
KqWtLN9lMbgffJmCn3Vi
=nP2h
-----END PGP SIGNATURE-----

--Apple-Mail=_2D26E11F-8E52-40B0-BD27-5C03417028C6--


--===============0353157483363396667==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
L2tpext mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/l2tpext

--===============0353157483363396667==--