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 protected]> wrote: > 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" > > 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 > definition, maybe > > object-map { > FlowId -> JSONValue; > } FlowCostConfidence > > Maybe. > Thirdly, I personally think that the cost confidence should better be > 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. > 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. > > Not necessary. It is a temporary section and will be removed when the content is ready. > 5. > Section 8.2, "Openflow" and "openflow" -> "OpenFlow" > > Sincerely, > Dennis Yu > > On Tue, Jun 27, 2017 at 9:09 PM, Kai Gao <[email protected]> > wrote: > >> 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" >> >> >> _______________________________________________ >> alto mailing list >> [email protected] >> https://www.ietf.org/mailman/listinfo/alto >> > > _______________________________________________ > alto mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/alto >
_______________________________________________ alto mailing list [email protected] https://www.ietf.org/mailman/listinfo/alto
