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

[Christian Hopps <[e-mail]>]

> On Sep 22, 2020, at 8:19 AM, Martin Björklund <[e-mail]> wrote:
> 
> Hi,
> 
> Sorry I missed this question.
> 
> I think it probably can be solved; but see below.
> 
> 
> "Rob Wilton (rwilton)" <[e-mail] <mailto:[e-mail]>> wrote:
>> Hi draft authors, Chris,
>> 
>> Can we also please try and close on this issue raised by Chris.
>> 
>> Chris, I don’t think that there is any great way to solve this issue
>> using YANG features, but presumably the constraint could be enforced
>> with a must statement, or groupings could be used to copy parts of the
>> ipsec structure into an sdn specific ipsec tree structure.
>> 
>> I understand that there isn't any great desire to delay these drafts
>> by trying to generalize the ipsec YANG model contained within it.
>> However, I think that means that the modules should have "-sdn-" in
>> their names to indicate that they are intended specifically for the
>> SDN use case, and should not be confused with the more generic ipsec
>> YANG modules that have been proposed.
>> 
>> Regards,
>> Rob
>> 
>> 
>>> -----Original Message-----
>>> From: yang-doctors <[e-mail]> On Behalf Of
>>> Christian
>>> Hopps
>>> Sent: 24 August 2020 18:08
>>> To: Martin Björklund <[e-mail]>
>>> Cc: [e-mail]; draft-ietf-i2nsf-sdn-ipsec-flow-
>>> [e-mail]; [e-mail]; [e-mail]; yang-
>>> [e-mail]
>>> Subject: Re: [yang-doctors] Yangdoctors last call review of
>>> draft-ietf-
>>> i2nsf-sdn-ipsec-flow-protection-08
>>> 
>>> [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.
> 
> Are we talking about /ipsec-ikeless/spd and /ipsec-ikeless/sad?  If
> these are moved to another module, ipsec-ikeless becomes empty (if
> also the related notifs are moved).

It will still contain the notification, which is used for managing ikeless ipsec in the SDN controller case.

> 
> Why do you want to move them?  It is b/c they are under
> "ipsec-ikeless"?  If so, perhaps a simpler solution is to use another
> (more generic) name for the module and top-level container.

Yes, b/c IPsec always has an SA database (both ike or ikeless). The reason, according to the authors, it was not put in common and was only included in ikeless was b/c the authors felt that for SDNs the SDN controller didn't care about the SA database since IKE was managing it. That doesn't mean it doesn't exist, just that their use case didn't care, and didn't want to force people to implement the YANG for it if they only supported the IKE module for SDN use.

I don't see why the want to not implement the SA database YANG for SDN+IKE couldn't be handled with a feature (or some other YANG mechanism) instead and then have a more correct model organization

The problem is that this common ipsec module (and the ikeless and ike even) are easily augmented and usable by IPsec in general if the SA and policy database were moved to common and out of ikeless. The only other way to re-use it would be to augment a duplicate SA database under the IKE module, but there is only a single SA database on a server, so now we have 2 SA databases in YANG (under ikeless and ike namespaces) to represent the same SA database. It seems wrong to go this route since the change I was suggesting seems pretty trivial.

In my case we have an ipsec extension in development along with a YANG module (https://tools.ietf.org/html/draft-fedyk-ipsecme-yang-iptfs-00) that want's to augment the SA database (for operational state like packet counters among other things), but of course we want this to be for IKE and IKE-less -- in particular IKE is the vastly common use case for IPsec.

Thanks,
Chris.

> 
> If they are moved to ietf-ipsec-common, an implementation that
> implements ietf-ipsec-ike can still import ietf-ipsec-common, but
> choose to not implement it (it will show up as an 'import-only-module'
> in yang library).
> 
> 
> /martin
> 
> 
>>> 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
>>> <[e-mail]> wrote:
>>>> 
>>>> 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
>>>> 
>>>> 
>>>> _______________________________________________
>>>> yang-doctors mailing list
>>>> [e-mail]
>>>> https://www.ietf.org/mailman/listinfo/yang-doctors