Subject: [yang-doctors] Yangdoctors last call review of draft-ietf-i2nsf-sdn-ipsec-flow-protection-08

From: Martin Björklund via Datatracker <[email-masked]>
Date: Mon, 24 Aug 2020 07:37:35 -0700

Reviewer: Martin Björklund
Review result: Ready with Nits

I did an early YANG Doctor's review of this draft.  Most of my
comments then have been addressed in this version.

Comments:

o  As I wrote in my early review, the RFC editor enforces a common
   format of YANG modules, so it is better to adhere to this format
   before sending the draft to the RFC editor.  Use

     pyang -f yang --yang-line-length 69 <FILE>

   to get a consistent look-and-feel for your module.

   (You will have to manually re-flow description statements after
   this.)

o  There are some leafs that are optional in the model, but w/o a
   default value and w/o an explanation of what happens if that leaf
   is not set.  You should find those and either make them mandatory,
   add a default value, or explain what it means when it isn't set.
   As an example,
   /ipsec-ike/pad/pad-entrypeer-authenticatin/pre-shared/secret
   is optional.  I suspect that this leaf needs to be mandatory.
   Another example is the leaf espencap.

/martin

---

From: Christian Hopps <[email-masked]>
Date: Mon, 24 Aug 2020 13:08:05 -0400

[adding in ipsec@]

Hi,

This draft was discussed in ipsecme at the last IETF, and there was a desire to look closer at a couple changes that would make these models usable by ipsec generally rather than only for SDNs. Otherwise we will end up with 2 models that look very similar and duplicate almost all the functionality. This was going to be done during the next yang doctor review, but it looks like that happened in the meantime (ships in the night).

At minimum the module names should include "-sdn-" if no other changes are made to indicate that they are only for sdn use; however, this is not the optimal solution.

A better solution would be to move the containers currently under ikeless (for SA and Policy databases) under ipsec-common.

The feedback I received from the authors was that the SDN controllers didn't care about the actual SAs and policies when using IKE so they didn't want to require someone implementing ike+common modules to have to support them.

The YANG question I suppose is, is there an easy way to move these containers from ipsec-ikeless to ipsec-common, but still allow for them to be empty and/or unimplemented for the SDN IKE use case? If they were made features, is there a proper YANG way to indicate that if the ikeless module is present then those features must also be supported thus matching the functionality as defined by the current draft?

Thanks,
Chris.

> On Aug 24, 2020, at 10:37 AM, Martin Björklund via Datatracker <[email-masked]> wrote:
> 
> _______________________________________________
> yang-doctors mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/yang-doctors