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

[Martin Björklund <[e-mail]>]

Christian Hopps <[e-mail]> wrote:
> 
> 
> > 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.

Ok.


> > 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.

Then I agree w/ you that moving them to common and adding a feature
would work.  If they don't want to implement them just don't implement
the feature.


/martin

> 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
>