Re: [OPSAWG] Call for adoption: draft-boydseda-ipfix-psamp-bulk-data-yang-model

[Benoit Claise <[e-mail]>]

Dear all,

I finally spent some time reviewing 
draft-boydseda-ipfix-psamp-bulk-data-yang-model-03

A couple of observations to start with:

Let's look at the draft objectives first:
1. Obsoleting RFC 6728 with new IPFIX and PSAMP YANG modules.

    RFC 6728 was the first YANG module produced outside of NETMOD. So
    obviously, we learned a lot in the mean time.
    As an example, the reference to the ifIndex as opposed to ifName in
    ietf-interfaces.
    So starting from a fresh new YANG module, which would follow the new
    YANG guidelines (RFC8407) and NMDA (RFC8342), is obvisously a good idea

2. Removing the SCTP constraint

    The requirement is clearly to use UDP, as opposed to SCTP. This
    makes sense to me, as SCTP did not take off in NetFlow/IPFIX/PSAMP
    world. Now, what would the IESG, and transport ADs in particular,
    say about using UDP in a standard document, while it has been
    imposing congestion-aware protocols? If the answer is "no way", the
    answer might be an experimental RFC.
    For your copious leisure time, some IPFIX history, along with my
    thoughts (*)

3. Bulk-data-export

    It goes beyond the RFC6728 scope
    What's not clear: bulk data is also for non-PSAMP data?

        This coupling and transport requirement
        makes it difficult for a device, which does not support SCTP, to use
        the model for collecting and exporting non-PSAMP bulk data.

Some feedback:

- the abstract doesn't mention the IPFIX YANG module, while 
https://tools.ietf.org/html/draft-boydseda-ipfix-psamp-bulk-data-yang-model-03#section-2 
does

- The Tree Diagrams (currently in appendix) should be moved inside the 
document

- You wrote:

    These are some of the other issues with the current model:

    o  The PSAMP YANG model defines the frequency of export in the PSAMP
       cache.  Bulk data needs the export frequency to be controlled by
       the exporting process.

Well, the IPFIX world, as soon as the flow expires (active and inactive 
timers), it streams out of the device b/c we speak about many flow records.
See https://tools.ietf.org/html/rfc5470#section-5.1.1

- Terminology.
You chose to cut/paste each definition in this draft. It's your chose, 
even if RFC6728 decided to use this format:

    o  Definitions adopted from [RFC5101  <https://tools.ietf.org/html/rfc5101>]:
       *  Collection Process
       *  Collector
       *  Data Record
       *  Exporter
       *  Flow
       *  Flow Key
       *  Flow Record
       *  Information Element
       *  IPFIX Device
       *  IPFIX Message
       *  Observation Domain
       *  Observation Point
       *  (Options) Template

You rightly added the RFC reference in case of cut/paste.
Now, here is one issue. Metering Process and Exporting Process have 
their own definitions in RFC6728 and in this draft.
The following text, from RFC6728, is still useful IMO.

    The terms Metering Process and Exporting Process have different
    definitions in [RFC5101  <https://tools.ietf.org/html/rfc5101>] and [RFC5476  <https://tools.ietf.org/html/rfc5476>].  In the scope of this
    document, these terms are used according to the following
    definitions, which cover the deployment in both PSAMP Devices and
    IPFIX Devices:


What you decided to do is change the Metering Process definition 
compared to RFC6728 by adding some extra sentences, and more 
importantly, by deleting this sentence:

       Inputs to the process are packets observed at one
       or more Observation Points, as well as characteristics describing
       the packet treatment at these Observation Points.

This came as a surprise. Explain why?
What we really need a section "change since RFC 6728", highlighting 
these type of changes, along with some justifications.
See, as an example https://tools.ietf.org/html/rfc7011#section-1.1

Note: all IPFIX/PSAMP RFCs have, by convention, the first letter 
capitalized for each terms specified in the terminology. Granted, this 
is not of highest importance but ...

- section 3. Structure of the Configuration Data Model.
While https://tools.ietf.org/html/rfc6728#section-3 has a (maybe long) 
explanation of IPFIX/PSAMP, 
https://tools.ietf.org/html/draft-boydseda-ipfix-psamp-bulk-data-yang-model-03#section-3 
is pretty short.
What bothers me is that you seem to pick what you want out of RFC6728... 
up to point where it's sometimes wrong.

        o  A PSAMP/IPFIX metered model where a PSAMP/IPFIX device configures
           a meter that samples packets passing through a device, applies an
           IPFIX template to those packets, and exports IPFIX templates/data
           records to an IPFIX collector.

IPFIX is an exporting protocol
PSAMP is a sampling metering process.
The following sentence is too simplistic/wrong:

       A PSAMP/IPFIX metered model where a PSAMP/IPFIX device configures
       a meter that samples packets passing through a device

You introduce new term: "PSAMP/IPFIX metered model", "PSAMP/IPFIX device"
This YANG module must be able to configure IPFIX without sampling.

- section 3.1, Metering Process Decomposition in Selection Process and Cache
Let me illustrate a general concern with the draft with a diff of this 
section, from RFC6758 to this draft

Note: the generic IETF diff tool https://www6.ietf.org/tools/rfcdiff/ 
was not useful, so I had to cut/paste the respective section in txt file 
as a prerequisite

In the first sentence, you removed "commonly".
In the first sentence, you changed "split into packet Sampling" and 
"Filtering functions performed by Selection Process, and the maintenance 
of the Flow Records and Packet Reports is performed by a Cache" into 
"split into the selection process and cache"
The sentence "The configuration data model adopts the separation of 
Selection Processes and Caches in order to support the flexible 
configuration and combination of these functional blocks." disappeared
The reference "As defined in [RFC5476]" disappeared.
"The action of the Selection Process on a single packet" became "The 
action of the selection-process on a single packet": No only you removed 
the capitalized first letter of the definition but the definition aspect 
is completely lost because you now have "selection-process", which you 
changed in the picture.
Regarding the section starting with "The configuration data model  does 
...", you re-wrote it with your interpretation and btw, you forgot the 
reference
Same remark for the sentence starting with "The configuration data model 
enables the configuration of a Selection Process".
You change the spec: "a Cache MUST NOT aggregate packets observed" => a 
cache must not aggregate packets observed

All in all, while the goal to clarify the text is noble, you took some 
freedom. Some modifications are wrong, some are not ideal, and some 
others need a more careful review by IPFIX experts. You know, we went 
through 11 draft versions, carefully evaluating each sentence, before 
publishing the RFC.
When creating a bis document, the goal is to modify the text minimally 
to facilitate the diff comparison... involving the authors day one to 
understand why the text is written that way.

-  I could make the same remarks for other sections.
Ex: "The ObservationPoint class specifies an Observation Point" => "The 
ObservationPoint class specifies an observation-point"
Clearly you missed the fact that Observation Point refers to a definition.
Ex: "A Selection Process MAY pass the Selected Packet Stream to a 
Cache." => "A selection process may pass the selected packet stream to a 
cache


At this point in time, I've been spending a couple of hours, barely 
arriving at section 4 and I'll stop there...
Reviewing this draft is very time consuming (and I know IPFIX, and maybe 
that's the reason).
This might explain why you did not get much feedback.

Let's start from the objectives.
1. Obsoleting RFC 6728 with new IPFIX and PSAMP YANG modules.
     There are two parts here.
     1.1 The YANG module. Martin Bjorklund did the YANG doctors review. 
As long as he is fine, I'm fine.
     1.2 This draft text obsoleting RFC 6728. For all the reasons 
mentioned above, I believe this draft is NOT a good basis.
2. Removing the SCTP constraint
     Sure, that's important, if the IESG agrees.
3. Bulk-data-export
     While I personally believe that the telemetry future is more into 
model-driven (to use the same data model for configuration and 
operational data), this new bulk-data-export feature does not do any 
harm. No objection here.


Let's keep some other factors in mind.
The BBF members did the absolute right thing to come to the IETF to 
update RFC6728, as opposed to focus this work in BBF. Thanks.
The IETF did not provide much feedback, even if the draft was presented 
a few times. Some reasons why are mentioned above.

In conclusion, my suggestion to the AD is therefore to publish this 
document, as experimental, not obsoleting RFC 6728, as AD sponsor.

Regards, Benoit

========================================

I started with some editorial/idnits, then I stopped
-

    [RFC6728] defines a single YANG module for the IP Flow Information
    Export (IPFIX) and Packet Sampling (PSAMP) protocols.  The PSAMP
    collecting process and the IPFIX exporting process are tightly
    coupled in this module.  Moreover, the exporting process requires a
    device to support SCTP.  This coupling and transport requirement
    makes it difficult for a device, which does not support SCTP, to use
    the model for collecting and exporting non-PSAMP bulk data.
Inline reference to RFC6728.

-

          The transport sessions are modeled such that they can be
          retrieved individually in addition to retrieving the entire
          list (which may be quite large for devices such as an NG-PON2
          OLT).

Describe NG-PON2 OLT

- 
https://datatracker.ietf.org/doc/review-boydseda-ipfix-psamp-bulk-data-yang-model-02-opsdir-lc-ersue-2019-12-01/ 
=> Not Ready

    Also as the document is largely based on RFC 6728, introducing the
    authors of RFC 6728 as co-authors and involving them for review
    would very useful. As a minimum they need to be involved as
    reviewers and mentioned in the Acknowledgments section.

    ...

    In case there is no support in OPSAWG WG for this draft to replace
    the standard track RFC 6728 I believe it would be appropriate to
    publish it as an "AD sponsored Experimental RFC". It can still
    become a standard track RFC after getting implementation reports and
    appropriate community feedback on its usage.

- 
https://datatracker.ietf.org/doc/review-boydseda-ipfix-psamp-bulk-data-yang-model-02-opsdir-lc-clarke-2019-12-20/ 
=> Not Ready

    One other point that struck me as I read this document was that 6728
    is being obsoleted by this, but there are references to things
    defined within it. I would think that anything that this new
    document will use in a normative fashion should be explicitly stated
    in this document. Such examples are found in Section 3 where text
    like "based on [RFC6728]" is used.

========================================

(*) 
https://www.claise.be/from-netflow-to-ipfix-via-psamp-13-years-of-standardization-explained-2/

    The next couple of years were dedicated to IPFIX protocol
    specifications. According to my recollection, the WG spend one year
    or maybe one year and half on transport-related discussions: should
    we use TCP or SCTP as the congestion-aware transport protocol …
    while most customers only cared about UDP when the flow export
    collection is exclusively within their management domain … and where
    the distributed function of forwarding ASICs complicate congestion
    aware transport-requirements.
    The final specifications compromise on: “SCTP [RFC4960] using the
    PR-SCTP extension specified in [RFC3758] MUST be implemented by all
    compliant implementations.  UDP [UDP] MAY also be implemented by
    compliant implementations.  TCP [TCP] MAY also be implemented by
    compliant implementations.”



> Hi all,
>
> I support progression of this draft as it addresses current needs for 
> IPFIX applications within Access Networks. The modular way this draft 
> constructs the configuration models will aid to the longevity of the 
> IPFIX protocol as additional use cases are identified.
>
> As a co-author of the draft, I would also like to address some 
> previous comments raised.
>
> I acknowledge the draft is long, but the content is necessary. In 
> order to address the shortcomings of the existing RFC 6728 data model 
> in the context of these new applications (see section 1.1 of the 
> draft), the data model was rewritten and restructured. As such, the 
> authors felt it was necessary to obsolete RFC 6728 so that there was 
> no confusion over the existence of the two data model approaches. This 
> meant that most of the content from RFC 6728 was carried over with 
> some necessary changes needed to a) align with the new data models and 
> b) modify how functional descriptions are tied to the data model to 
> conform to the latest RFCs which define YANG data models, e.g. use of 
> tree diagrams instead of class diagrams. As other author noted, 
> splitting the document into smaller parts doesn’t really change the 
> amount of text that must be reviewed and actually increases it as some 
> portions will need to be repeated. This opens the door to introduce 
> inconsistencies. As such, I would not be in favor of splitting the draft.
>
> I look forward to working with everyone to progress this draft forward.
>
> Best regards,
>
> Joey
>
>
> _______________________________________________
> OPSAWG mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/opsawg