Subject: [alto] Reviews on draft-gao-alto-fcs-02

From: Kai Gao <[email-masked]>
Date: Tue, 27 Jun 2017 21:09:38 +0800

Dear authors of draft-gao-alto-fcs,

I have roughly reviewed the latest FCS draft. Please see the comments below.

Regards,
Kai

Terminology:

sX: section X
ppX: page X
pX: paragraph X
lX: line X

pp3, s1, p1
l2: scenario -> scenarios
l3-4: "... the P2P application ..." -> "... P2P applications ..."

pp3, s1
p2, l2: central -> centralized
p2 & p3: I think these two paragraphs can benefit from some restructuring to
make the motivations for flow-based query more explicit and also easier 
to read.

pp3, s1, p5
l1: consider -> describe
l3: schema -> schemas

pp4, s1, p1
l2: "... 5-tuples of ..." -> Use the explicit form of the 5-tuple: "... 
5-tuple <ip-protocol, ...>"
l6: "SHOULD support" -> "supports"
l7: "to satisfy" -> "and satisfies"

pp5, s3.1.2, p5
l4: "appeared" -> "appear"

pp6, s3.2
title: "Extend" -> "Extended"
p2, l2: "... to measure the cost" -> "... to specify the request"
p2, l3: "SHOULD" -> "MUST"
p2, l4: "be" -> "have"
p2, l5: "format" -> "formats"
p4, l2: Extra space after "TypedEndpointAddr"

pp6, s3.2.1, p1
l1: "contain" -> "can be"
l4: "specified" -> remove it.
l5: "be conflict" -> "conflict"
l6: "is conflict" -> "conflicts"

pp7, s3.2.3, p1: Use "Protocol" instead of "protocol" to make it 
explicit that
it represents the value contained in the query.

pp8
s3.3.1, p1, l1: Remove "then"
s3.3.2, p6, l4: "appeared" -> "appear"
s3.3.3, p1, l2: "exactly" -> "exactly the same"
s4, title: Example -> Examples

pp14, s4.2.3, p4, l3: "value types" -> "value type"

pp15, s4.2.5.1, p1, l5: ", the servers" -> ". The servers"

pp16, s4.2.5, p3, l3: "TypedHeadreField" -> "TypedHeaderField"

---

From: Dennis Yu <[email-masked]>
Date: Wed, 28 Jun 2017 00:24:42 +0800

Dear authors of draft-gao-alto-fcs-02,

Yet another review of draft-gao-alto-fcs-02 :)
Here are some opinions that maybe have some help,

1.
Some inconsistency with draft-ietf-alto-multi-cost-10,

the definition of ReqFilteredCostMap in "multi-cost" section 4.1.2 is as
follows,
(the comments are appended)

    object {
        [CostType cost-type;]
        [CostType multi-cost-types<1..*>;]
        [CostType testable-cost-types<1..*>;]
        [JSONString constraints<0..*>;]
        [JSONString or-constraints<1..*><1..*>;]
        [PIDFilter pids];   //<---- likely a typo
    } ReqFilteredCostMap;

and the extended definition in "fcs" section 3.1.2 is,

    object {
        [CostType cost-type;]
        [CostType multi-cost-types<1..*>;]
        [CostType testable-cost-types<1..*>;]
        [JSONString constraints<0..*>;]
        [JSONString or-constraints<0..*><0..*>;]  //  1 --> 0
        [PIDFilter  pids;]
        [PIDFilter  pid-flows<1..*>;]
    } ReqFilteredCostMap;

also see the definition of ReqEndpointCostMap. Although the difference
between "1" and "0" is tiny, but I think it's better to agree on one
definition.

2.
the definition of FlowCostResponse in section 4.2.5 is as follows,

    object {
        FlowCostMap     flow-cost-map;
        [FlowCostMap    flow-cost-confidences;]
    } FlowCostResponse : ResponseEntityBase;

    object-map {
        FlowId -> JSONValue;
    } FlowCostMap;

Firstly, I'm not sure the "JSONValue" is well-defined. According to RFC7159
section 3, "A JSON value MUST be an object, array, number, or string, or
one of the following three literal names: false null true"

Secondly, I think the "flow-cost-confidences" could have its clear type
definition, maybe

    object-map {
        FlowId -> JSONValue;
    } FlowCostConfidence

Thirdly, I personally think that the cost confidence should better be
defined as a JSON fractional number between 0 to 1.

3.
A missing comma in the example of page 19 (l3-flow-aggr)
Two redundant commas in the example of page 19 (cost-type) and page 10
(dependent-vtags)

4.
The change log of section 2 can be composed as an appendix.

5.
Section 8.2, "Openflow" and "openflow" -> "OpenFlow"

Sincerely,
Dennis Yu

On Tue, Jun 27, 2017 at 9:09 PM, Kai Gao <[email-masked]>
wrote:

> make the motivations for flow-based query more explicit and also easier to
> read.
>
> pp3, s1, p5
> l1: consider -> describe
> l3: schema -> schemas
>
>
> pp4, s1, p1
> pp7, s3.2.3, p1: Use "Protocol" instead of "protocol" to make it explicit
> that
> _______________________________________________
> alto mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/alto
>

---

From: Jensen Zhang <[email-masked]>
Date: Fri, 30 Jun 2017 08:13:11 +0000

Kai and Dennis, thanks a lot for your reviews.

I have fixed typos and grammar issues according to your comments. About
some of your suggestions, see my comments inline.

On Wed, Jun 28, 2017 at 12:24 AM Dennis Yu <[email-masked]> wrote:

> Firstly, I'm not sure the "JSONValue" is well-defined. According to
> RFC7159 section 3, "A JSON value MUST be an object, array, number, or
> string, or one of the following three literal names: false null true"
>
>
JSONValue is OK here. See Section 11.2.3.6 and Section 11.5.1.6 of RFC7285.

> Secondly, I think the "flow-cost-confidences" could have its clear type
Maybe.

> defined as a JSON fractional number between 0 to 1.
Actually, I prefer to deprecate flow-cost-confidences from the draft, since
it can be supported by introducing new cost types (cost-mode). Defining an
additional field does not convince me. For example, we can define
cost-mode=range, cost-metric=delay:

meta:
  ...
  cost-type:
    cost-mode: range
    cost-metric: delay
cost-map:
  PID1:
    PID2: [100, 110]
    PID3: [220, 240]

Kai, the original idea of the cost confidence is proposed by you. Do you
have any comments?

3.
> The change log of section 2 can be composed as an appendix.
Not necessary. It is a temporary section and will be removed when the
content is ready.

---

From: Shenshen Chen <[email-masked]>
Date: Fri, 30 Jun 2017 21:19:52 +0800

Hi all,

I'd like to join this discussion.

About the flow-cost-confidences, I prefer the original design.
The mean of cost makes sense (the expected cost if pick path randomly)
and can't be deduced from the range (since it may not be an
equal probability distribution).

Here is a list of typos (comments are marked with ">>>"):

[Page 1]
     3.1.  Flow-based Filtered Cost Map  . . . . . . . . . . . . . .   4
       3.1.1.  Capabilities  . . . . . . . . . . . . . . . . . . . .   4
       3.1.2.  Accept Input Parameters . . . . . . . . . . . . . . .   5

>>> Accept => Acceptable, also see 3.3.2. and 4.2.3.

[Page 1]
     3.4.  Example . . . . . . . . . . . . . . . . . . . . . . . . .   8

>>> Example => Examples

>>> Also, the catalog doesn't include Section 4.2.5.1., 4.2.6.1., etc,
which looks weird.

[Page 1]
   The emergence of new networking datapath capabilities has
   substantially increased the flexibility of networking.  For example,
   OpenFlow has emerged as a major southbound protocol for Software-
   Defined Networking, and OpenFlow allows flexible routing beyond
   simple destination-based routing.  In this document, we define a new
   extention to ALTO, namely the Flow Cost Service, for ALTO clients in

>>> extention => extension

   an OpenFlow-enabled network to query ALTO network information.

[Page 6]
   The Protocol field is REQUIRED.  The available values contain
   different protocols including layer two protocols (e.g.  "eth") and
   layer three protocols (e.g.  "ipv4", "ipv6").  It can also be
   specified upper-layer protocols (e.g. "udp", "tcp", "ssh", "http" and
   "ftp").  The source and destination protocols MUST NOT be conflict.

>>> be conflict => be conflicted

   In every EndpointFilter object of either "endpoints" field or
   "endpoint-flows" field, if the source protocol is conflict with the

>>> is conflict with => has conflict with

   destination protocol, this endpoint pair is invalid.  For different
   protocols, some additional constraints are defined in Section 3.2.3.

[Page 7]
   protocols:  Defines a list of JSONString indicating the supported
      Protocol values of the EndpointURI in the request.  The ALTO
      server does not have to claim "ipv4" and "ipv6" in this field
      explictly, because they are supported by default.  If not present,

>>> explictly => explicitly

      this field MUST be interpreted as if it is specified the default
      supported protocols "ipv4" and "ipv6".

[Page 16]
   The ALTO servers can provide more information to the clients when
   requests have errors.  The FlowCostErrorMap below can provide basic
   information about two most common errors for the flow cost service.
   The ALTO servers MAY include it as the data component of an ALTO
   error response.  If multiple errors are identified, the ALTO server
   MUST return exactly one error code according to Section 8.5.2 of
   [RFC7285] .

[Page 20]
  Security considerations:  Security considerations are identical to
      those specified in Section 15 of [RFC7285] .

>>> remove the redundent space between [RFC7285] and full stop

[Page 16, 17]
   Some header fields may have conflicts.  For example, IPv4 fields and
   IPv6 fields can never appear in the same packet, nor can TCP and UDP
   ports.  These header fields MUST not be included in the same flow
   filter, otherwise the ALTO server MUST return an ALTO error response,
   with the error code "E_INVALID_FIELD_VALUE".  As specified in

>>> , otherwise => . Otherwise,
    remove the comma before "with"

   Section 8.5.2 of [RFC7285], the ALTO server MAY include the "field"
   and the "value" in the "meta" field.  In this case, the ALTO server
   MUST use the flow ID as the "field" and the flow filter as the
   "value".  However, the recommended approach is to use the
   FlowCostErrorMap, where the server CAN provide the conflicting typed
   header fields in the "conflicts" field of the FlowFilterError
   associated with the corresponding flow ID.

Best Regards,
Shenshen

---

From: Jensen Zhang <[email-masked]>
Date: Fri, 30 Jun 2017 13:56:22 +0000

Shenshen,

Thanks for your reply. Fixed the typos.

Jensen