Subject: Re: [PCN] FW: New Version Notification - draft-ietf-tsvwg-rsvp-pcn-03.txt

From: Bob Briscoe <[email-masked]>
Date: Thu, 15 Nov 2012 21:34:39 +0000

Georgios & Anurag,

Thank you for this draft. Below is my review, as:
- a member of the RSVP directorate
- one of the PCN design team
- a co-author of draft-lefaucheur-rsvp-ecn, on which this draft is based.

I have already sent my main technical concerns in two separate 
emails, summarised below (but I suggest we use the separate threads 
to discuss):

1. It would be easy to allow for an off-path policy-decision point 
(PDP), as per RFC2753 (policy-based admission control (PBAC) framework)

2. PCN already reduces reservation state and processing to nothing on 
interior nodes. Adding aggregate reservations to PCN adds 3 
disadvantages and no advantages:
   - it requires more processing and state in interior nodes and boundary-nodes
   - it unnecessarily pins routes to interior nodes and
   - it adds unnecessary latency.

This email reviews comprehensibility and raises a large number of 
lesser technical concerns (26 to be precise), as well as editorial 
nits (many of which may be irrelevant if I'm right about the major 
technical concerns).

==General style==
I didn't find the style useful at all. Sections 1, 2 & 3 defer 
everything meaningful into pointers to other RFCs. This makes it 
incomprehensible, even if you know all the references intimately. 
Lixia's suggestion that you refer to the specific sections of other 
RFCs won't be enough.

An RFC should be understandable without re-reading its references 
(readers should have read them, but they shouldn't be expected to 
remember every detail).

I have to say we are moving very slowly, and backwards. When Francois 
Le Faucheur first wrote the ancestor to this draft (7 years ago!), I 
could understand it immediately, and in half the pages it covered 
nearly everything in this draft and covered the gaps I have listed 
below. [In the interests of full disclosure: I became a co-author of 
draft-lefaucheur-tsvwg-rsvp-ecn-00, nonetheless Francois's pre-00 
draft was more comprehensible and complete than this one, even before 
the co-authors did the first reviews.]

This draft says very little itself, but ends up much longer. For 
example Section 3 tediously repeats content-free text like:
"<Title>
In this document it is considered that for <Title> the same methods 
can be used as the ones described in <reference>".

Not only section 3; sections 1 & 2 also rely wholly on references, 
making them just as incomprehensible. I also agree with the other 
reviewers' suggestions to focus the introduction initially on what 
this draft adds, and move the background material after that.

In summary: avoid phrases like "the same methods as" or "standard 
RSVP aggregation [...] procedures are used". Instead just say 
directly how it works.

==Normative & Technical Points==

==Gaps==

* How an aggregate reservation is created vs. how one is used if it 
is already present

* How next hop addresses are discovered (a PCN domain is only one 
RSVP hop unlike with RSVP aggregation, so although the process is 
similar, this needs to be described explicitly)

* How PCN interior nodes ignore RSVP messages:
   - by configuration?
   - or by the PCN-ingress switching aggregate PATH messages to IP 
protocol number RSVP-E2E-IGNORE (which isn't strictly the right name 
but would have the right effect)?

* Error conditions, for instance when messages don't contain the 
expected objects, e.g. [draft-lefaucheur-tsvwg-rsvp-ecn-01, page 6 bullet 1].

* How per-node state is maintained and the interaction with RSVP soft 
state refresh messages is not discussed. PCN-related state is unlike 
other RSVP state, in that it continually varies (it is controlled by 
a varying signal). Therefore it needs to be clear that the fields of 
the PCN object reflect the current value when sent, not their 
original value, which is what an RSVP implementer would normally expect.

==Specific sections==

1. Introduction

"   This document, and according to [RFC4860] MAY also be used end-to-end
    directly by end-systems attached to a Diffserv network.
"
No. See RFC5559 for PCN applicability - there is insufficient 
aggregation for PCN to work effectively e2e. The PCN applicability 
statement in RFC5559 should be stated here instead: PCN is only 
applicable to links with aggregation levels where the bit-rate of the 
largest flow is tens of times smaller than the smallest available 
link rate in the PCN region.

OLD:
    In this document it is considered that the PCN-nodes MUST be able to
    support the functionality specified in [RFC5670], [RFC5559],
    [RFC6660], [RFC6661], [RFC6662].
NEW:
    To comply with this specification, PCN-nodes MUST be able to
    support the functionality specified in [RFC5670], [RFC5559]
    [RFC6660], and either [RFC6661] or [RFC6662].

Section 2.1 first para:
"   In addition, in this document it is considered that the PCN-boundary
    nodes are able to distinguish and process (1) RSVP SESSIONS for
    generic aggregated sessions and their messages according to
    [RFC4860], (2) e2e RSVP sessions and messages according to [RFC2205].
"
Say how (by the addresses that the RSVP messages are addressed to).

"   Furthermore, it is considered that the PCN-interior-nodes are not
    able to distinguish neither RSVP generic aggregated sessions and
    their associated messages [RFC4860], nor e2e RSVP sessions and their
    associated messages [RFC2205].
"
Say how (see two possibilities in 'Gaps' above). This is also not 
explained in Sections 2.1.7., 3.2 & 3.7, which are all meant to be about this.

Section 2.1 bottom of p11:
"The RSVP SESSION object for generic
    aggregate reservations is based on the RSVP SESSION object specified
    in [RFC4860] augmented with the following information:

     o) the IPv4 DestAddress, IPv6 DestAddress SHOULD be set to the IPv4
        or IPv6 destination addresses, respectively, of the Deaggregator
       (PCN-egress-node)
"
Say how - see 'Gaps' above - the whole process of RFC4860 (e2e PATH, 
e2e PathErr, aggregate PATH) finds the right address, but this needs 
to be described because in PCN the RSVP hop is across multiple IP 
nodes (the whole PCN-domain), unlike with RSVP aggregation.

Section 2.1.2.
"   The PCN-traffic (e.g., e2e microflows) belonging to an ingress-
    egress-aggregate can be classified only at the PCN-boundary-nodes
    using the combination of (1) PCN-BA (i.e., combination of the DSCP
    and ECN fields), (2) IP addresses of the specific pair of PCN-
    boundary-nodes used by a ingress-egress-aggregate.
[...]
    Moreover, the PCN-traffic (e.g., e2e microflows)
    belonging to a RSVP generic aggregated reservation can be classified
    only at the PCN-boundary-nodes (i.e., Aggregator and Deaggregator) by
    using the RSVP SESSION object for RSVP generic aggregated
    reservations, see [RFC4860].
"
Are you assuming a tunnel? You haven't said so? Otherwise the traffic 
doesn't contain the PCN-boundary-node addresses.

Section 2.1.8. Inter-domain Routes

PCN charter scope precludes inter-domain considerations. I personally 
would happy to have discussion of inter-domain matters in here, but 
it will not be complete, because we would need to discuss security & 
integrity of PCN objects, but the PCN charter precluded that.

Section 2.1.10. Multi-level Aggregation
"   PCN does not consider multi-level aggregations within the PCN domain.
"
You have referenced generic aggregate reservations throughout 
[RFC4680], rather than just aggregate reservations [RFC3175]. I 
understand the difference is that RFC4860 supports e.g. multiple 
levels of precedence. So why say we don't consider multi-level aggregation?

Section 3.1
"     o) The e2e RSVP reservation session associated with an e2e Path
         message that arrives at the external interface of the PCN-
         ingress-node is mapped/matched onto an existing RSVP generic
         aggregation reservation state.
"
You haven't said how the existing aggregate is created in the first 
place (see 'Gaps' above).

Section 3.1 (twice)
"                                               A new error code
         "PCN-domain rejects e2e reservation" MUST be augmented to the
         RSVP error codes to inform the sender that a PCN domains rejects
         the e2e reservation request.
"
Disagree. The regular error code "01: Admission Control failure" 
should be used, so that end-system implementations understand that 
the call has been blocked.
It should also use the regular "Sub-code = 2: Requested bandwidth unavailable".

If the PCN-ingress uses a different code, it will imply to other RSVP 
nodes including end-systems that admission control did not fail. If 
it uses the 01 error code but a different sub-code it will imply that 
AC failed, but not because of insufficient bandwidth.

PCN is one (of many) mechanisms to determine whether a reservation 
should be blocked. For other nodes, it is not relevant to describe 
the mechanism (PCN) that was used to determine that there was 
insufficient bandwidth.

Section 3.1
"        o) If for the same ingress-egress-aggregated and the same RSVP
             generic aggregated reservation then (1) the PCN-admission-
             state and/or (2) the state for the RSVP generic aggregated
             reservation are/is "block", the flow SHOULD NOT be
             admitted
[...]
    The way of how the PCN-admission-state is maintained is specified in
    [RFC6661] and [RFC6662]. The way of how the RSVP generic aggregated
    reservation state is maintained is specified in [RFC4860].
"
As discussed in my email arguing against aggregate reservations, the 
e2e RESV message can carry an up-to-date PCN object for the 
PCN-ingress to decide whether to admit the flow signalled by the same 
message, given the PCN-ingress already handles e2e RESV messages. 
There is no need to rely on admission state determined earlier, given 
the PCN-egress already sends an e2e RESV message to the PCN-ingress 
at admission time.

Section 3.2 and 3.7 (and 2.1.7).
Neither section says how interior nodes are arranged to ignore RSVP 
messages (see 'Gaps' earlier which suggests two possible approaches).

Section 3.8 (last sentence)
"       The address of the PCN-ingress-
         node is the one specified in the same ingress-egress-aggregate."
    I cannot work out how

Section 3.11
Separation of the policy enforcement role of the PCN-ingress from a 
policy decision role (that may be off-path) will need to be added to 
the first bullet of this section (see other email).

Section 3.11
"        However, based on a local policy, the Aggregator could use
         other procedures of terminating microflows.
"
Please delete. Policy determines /which/ flows are terminated, not 
/how/ the termination procedure works.

Section 3.12
I don't understand the purpose of this section - section 3.11 seems 
to have already said everything that section 3.12 says.

Section 3.13
Say why an aggregate reservation would need to be removed:
- no e2e reservations left in the aggregate?
- such severe pre-congestion that every flow in the aggregated has to 
be terminated (!!!???)

Section 4. Protocol Elements
"   The protocol elements in this document are using the protocol
    Elements defined in [RFC4860], augmented with the following rules:
"
This doc uses a number of protocol elements from the base RSVP spec, 
not just the extras defined in RFC4860.

BTW, protocol elements are generally called classes in RSVP documents 
(and instances of them are called objects). Also the grammar and 
capitalisation in this sentence is pretty poor.

Section 4. Protocol Elements
The first two bullets are inappropriate here and more appropriate to section 3.

Section 4.1 PCN Object

I don't see why the PCN object needs to contain the IP addresses of 
the PCN-ingress and PCN-egress. These will be in the SESSION object 
earlier in the list of RSVP objects, and RFC2205 says that the 
SESSION object defines the session for the other objects that follow. 
This also means we don't need different PCN objects for IPv4 & IPv6.

Section 4.1 PCN Object

s/rate/fraction/
I don't think you mean rate, which implies with respect to time. I 
think you mean the fraction of bytes in packets with each marking 
relative to the total bytes in PCN marked packets.

Section 4.1 PCN Object

I suggest that the PCN object always reports the fractions of all 
three non-zero values of the PCN (aka ECN) field, even for single 
marking (SM) mode that doesn't use one of the codepoints. Then 
PCN-egress implementations and the PCN object on the wire can be the 
same for both SM & CL modes (I think), and only the ingress has to be 
different.

Section 4.1 PCN Object (and IANA Considerations)

IANA need to be told the high order 2 bits of the Class-Num, to 
exploit the Unknown Class rules of [RFC2205, section 3.10]. I suggest 
the Class-Num is 10bbbbbb, so that any node outside a PCN domain that 
receives a PCN object and doesn't understand it will strip it off and 
forward the remainder of the RSVP message. Ideally we would want the 
RSVP message (with or without the PCN object) to be forwarded and an 
error message raised, but that option is not available; RSVP only 
allows an error message to be raised when it discards a whole message.

Section 4.1 PCN Object

The section defines a PCN CL Flow IDs object. It's not clear whether 
it is meant to be a different object from the PCN object, or an 
extension to it. I think it would be clearer to make it a separate 
RSVP object, although in theory a PCN-ingress could work out that a 
PCN CL Flow IDs object is included, by a calculation from the object 
length (because the base PCN object is always a fixed length).

I don't see the point of the LENGTH field (for the number of flow ID 
objects), given RSVP gives the length of an object in octets, from 
which the number of flow ID objects can be derived (divide by 16 for 
IPv4 or divide by 40 for IPv6). Also RSVP objects have to align on 4 
octet boundaries.

==Editorial Nits==

Globally:
s/In this document it is considered that.../
  /To comply with this specification.../

Abstract:
s/specifies the extensions to/
  /specifies extensions to/

1. Intro
s/collocated/
  /colocated/
s/extensions to the Generic Aggregated RSVP [RFC4860] for the support of/
  /extensions to Generic Aggregated RSVP [RFC4860] for support of/

OLD
    Furthermore, this document and according to [RFC4860], in absence of
    e2e RSVP flows, a variety of policies (not defined in this document)
    can be used at the Aggregator to set the DSCP of packets passing into
    the aggregation region and how they are mapped onto generic aggregate
    reservations. These policies are not described in this document but
    are a matter of local configuration.
NEW
    In a similar way to [RFC4860], the Aggregator can detect flows 
based on policy
    (not defined in this document) rather than using e2e RSVP flow 
signalling. Then
    it can map them onto aggregate reservations and set the DSCP of 
the packets of
    these flows as they pass into the aggregation region.

Section 1.1. Terminology

"  Ingress-egress-aggregate (IEA):
[...]
                     combination of (1) fields), (2) IP addresses of the
"
Some text appears to be missing here.

Section 1.1. Terminology
OLD:
    t-recvFail
                     An ingress-egress-aggregate timer that is used at
                     The Decision point (in this document at the PCN-
                     ingress-node) which when expires raises an alarm to
                     management, and activates the PCN-ingress-node to
                     block the admission of new PCN-flows. This timer
                     expires when it value is equal to T-fail and is
                     reset when a report, i.e., RSVP aggregated RESV
                     message, is received for a RSVP generic aggregated
                     reservation (which is matched to one
                     ingress-egress-aggregate).
NEW:
    t-recvFail
                     A timer per ingress-egress-aggregate that the 
PCN-ingress-node
                     sets every time it receives an RSVP RESV message for that
                     ingress-egress-aggregate. When its value reaches T-fail
                     it is assumed that the PCN-ingress has lost 
contact with the
                     PCN-egress. Therefore the PCN-ingress blocks 
admission of new
                     PCN-flows into that aggregate and raises a 
management alarm.
REASONING:
    The order in which the clauses were written made it hard to 
understand, there was nothing to say what it measured, and it wasn't 
clear what scope was implied by "it blocks PCN-flows".

Section 1.1 Terminology

Underscores (e.g t_meas) are often used in the terminology section, 
whereas hyphens (e.g. t-meas) are used in the body text.

Section 2.1 Overview

There is no Section 2.2, so all the sub-sections of 2.1.X could be promoted.

Section 2.1.1
s/used, SHOULD/used SHOULD/

Section 2.1.2.
OLD:
    The PCN-traffic is marked using PCN-marking and is classified using
    The PCN-BA (i.e., combination of the DSCP and ECN fields).
NEW:
    The PCN-ingress marks a PCN-BA useing PCN-marking (i.e., 
combination of the DSCP and ECN fields), which interior nodes use to 
classify PCN-traffic.
REASONING:
    Avoid passive verbs - ambiguous.

Section 2.1.3
s/for the determination of/to determine the address of/

Section 2.1.4
"   o) PCN-ingress-node MUST use one or more policies to estimate whether
       an e2e RSVP reservation session associated with an e2e Path
       message that arrives at the external interface of the PCN-ingress-
       node can be mapped onto an existing RSVP generic aggregation
       reservation state.
"
This is a straightforward mapping function, it shouldn't involve policy.
Also mappings are deterministic - the word estimate is wrong.

Section 3.1
"    o) If the timer t-recvFail expires [...]
      o) If the timer t-recvFail does NOT expire [...]
"
Switch the order of these two bullets, to describe the normal 
(successful) behaviour first.

Section 3.1

s/ingress-egress-aggregated/
  /ingress-egress-aggregate/
s/then (1)/
  /(1)
[However, I suggest earlier that the technical sense should be 
substantially changed, possibly making these editorial changes irrelevant.]

Section 3.8.
OLD:
         The PCN object is specified in
         this document and is used for the report of the data measured by
         the PCN-egress-node, for a particular ingress-egress-aggregate,
         see [RFC6661], and [RFC6662].
NEW:
         The PCN-egress-node reports the marking probabilities it measures for
         a particular ingress-egress-aggregate in a PCN object, as specified in
         Section 4 (see also [RFC6661], and [RFC6662]).
REASONING:
    Poor sentence order.

Section 3.11.
s/associated to one ingress-egress-aggregate/
  /associated with one ingress-egress-aggregate/

Section 3.15
Repeats Section 2.1.9. Unnecessary.

Section 4.
s/by an PCN-egress node/
  /by a PCN-egress node/

Regards

Bob

At 19:55 11/10/2012, [email-masked] wrote:
>Hi all,
>
>Please note that draft-ietf-tsvwg-rsvp-pcn-03.txt has just been submitted!
>All comments received during the tsvwg meeting in Vancouver and 
>during the tsvwg and pcn mailing lists have been worked out!
>
>If you have any other comments please send them to the tsvwg mailing list!
>
>Best regards,
>Georgios
>
>
>________________________________________
>Van: [email-masked] [[email-masked]]
>Verzonden: donderdag 11 oktober 2012 21:46
>Aan: [email-masked]; 
>[email-masked]; [email-masked]
>Onderwerp: New Version Notification - draft-ietf-tsvwg-rsvp-pcn-03.txt
>
>A new version (-03) has been submitted for draft-ietf-tsvwg-rsvp-pcn:
>http://www.ietf.org/internet-drafts/draft-ietf-tsvwg-rsvp-pcn-03.txt
>
>
>The IETF datatracker page for this Internet-Draft is:
>https://datatracker.ietf.org/doc/draft-ietf-tsvwg-rsvp-pcn/
>
>Diff from previous version:
>http://www.ietf.org/rfcdiff?url2=draft-ietf-tsvwg-rsvp-pcn-03
>
>IETF Secretariat.
>_______________________________________________
>PCN mailing list
>[email-masked]
>https://www.ietf.org/mailman/listinfo/pcn

________________________________________________________________
Bob Briscoe,                                BT Innovate & Design