Christian Hopps <chopps@xxxxxxxxxx> wrote: > > > > On Sep 22, 2020, at 8:19 AM, Martin Björklund <mbj+ietf@xxxxxxx> > > wrote: > > > > Hi, > > > > Sorry I missed this question. > > > > I think it probably can be solved; but see below. > > > > > > "Rob Wilton (rwilton)" <rwilton@xxxxxxxxx <mailto:rwilton@xxxxxxxxx>> > > 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 <yang-doctors-bounces@xxxxxxxx> On Behalf Of > >>> Christian > >>> Hopps > >>> Sent: 24 August 2020 18:08 > >>> To: Martin Björklund <mbj+ietf@xxxxxxx> > >>> Cc: i2nsf@xxxxxxxx; draft-ietf-i2nsf-sdn-ipsec-flow- > >>> protection.all@xxxxxxxx; ipsec@xxxxxxxx; last-call@xxxxxxxx; yang- > >>> doctors@xxxxxxxx > >>> 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 > >>> <noreply@xxxxxxxx> 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 > >>>> yang-doctors@xxxxxxxx > >>>> https://www.ietf.org/mailman/listinfo/yang-doctors > -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
