Apologies for the delay in responding, Paul. This addresses most of my
concerns. I note that some new redundancy has been introduced since -16,
notably for source-port/destination-port, which could be further
consolidated according to the DRY principle ("don't repeat yourself"). It's
probably worthwhile to scrub the entire ecosystem for this kind of
redundancy, but I wouldn't say it's a blocker.

Thanks,
Kyle

On Sat, Jan 22, 2022 at 8:55 AM Mr. Jaehoon Paul Jeong <
[email protected]> wrote:

> Hi Kyle and Tom,
> Here is the revision of I2NSF NSF-Facing Interface YANG Data Model Draft:
>
> https://datatracker.ietf.org/doc/html/draft-ietf-i2nsf-nsf-facing-interface-dm-17
>
> I attach a revision letter to explain how Patrick and I have addressed
> your comments.
>
> Thanks.
>
> Best Regards,
> Paul
>
>
> On Tue, Nov 23, 2021 at 2:57 AM Kyle Rose via Datatracker <
> [email protected]> wrote:
>
>> Reviewer: Kyle Rose
>> Review result: Has Issues
>>
>> I have reviewed this document as part of the security directorate's
>> ongoing
>> effort to review all IETF documents being processed by the IESG.  These
>> comments were written primarily for the benefit of the security area
>> directors.
>>  Document editors and WG chairs should treat these comments just like any
>> other
>> last call comments.
>>
>> This document Has Issues.
>>
>> I don't actually have a lot to say about this document from a security
>> perspective: its purpose is to describe, using YANG, a data model
>> intended as
>> the basis for configuration schemas developed for implementations of the
>> Interface to Network Security Functions (I2NSF) framework. Security
>> considerations for the most part should be addressed in documents
>> describing
>> system architecture or normatively detailing how implementations are to
>> make
>> use of the data model described here. I'm not going to relitigate any such
>> issues here.
>>
>> The main issues I found in this document are ones that I, as someone not
>> terribly familiar with YANG, found troubling from a general engineering
>> perspective. These are best illustrated by example:
>>
>>  * Why are `ipvX-prefix`, `-range`, and (the misleadingly-named)
>> `-address`
>>  defined here? These concepts are generic enough that they should be in
>> modules
>>  of their own to minimize variation among data models and the errors that
>> will
>>  inevitably result from capturing the same concept in slightly different
>> ways
>>  that are not obvious to the user. * Overall, I have to imagine that much
>> of
>>  the `nsfintf` data model is generic enough that it should be captured
>>  externally. For instance, `tcp-flags`, `port-range`, `flow-label`,
>> `dscp`,
>>  etc. are generally useful elements of an abstract transport data model
>> that
>>  they shouldn't be defined here, but rather incorporated from an external
>> data
>>  model that is maintained by those in (for example) the transport area.
>>
>> Am I just commenting on the YANG ecosystem in general? If these are
>> standard
>> practices, then the overall ecosystem has major latent problems. Ideally,
>> a
>> particular YANG module seems like it should describe only those elements
>> defined at a particular layer, in this case rules and actions, and use
>> reference external modules for elements that are defined at lower layers.
>>
>> Also some nits:
>>
>>  * `ipvX-address` describes a subspace of the global IPvX address space,
>> not a
>>  single address. The name is going to cause problems. * The descriptions
>> given
>>  are often (usually?) just restatements of the entity name. Example is
>>  `identity priority-by-order` described as "Identity for priority by
>> order".
>>  The description should provide some value for the user beyond simply
>> restating
>>  the name. * The headings in section 5 should be clearly labeled with the
>> word
>>  "example", such as "Example Security Requirement 1". * IPv6 addresses in
>> text
>>  MUST be represented in lowercase, according to RFC 5952 section 4.3.
>>
>>
>> _______________________________________________
>> I2nsf mailing list
>> [email protected]
>> https://www.ietf.org/mailman/listinfo/i2nsf
>>
>
_______________________________________________
I2nsf mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/i2nsf

Reply via email to