Re: [netconf] New Version Notification - draft-ietf-netconf-notification-capabilities-09.txt

[Andy Bierman <[e-mail]>]

On Thu, Jan 9, 2020 at 3:54 AM Martin Bjorklund <[e-mail]> wrote:

> Andy Bierman <[e-mail]> wrote:
> > Hi,
> >
> > I think the new system capabilities module is good enough.
> > The node-instance-identifier type allows a key to be missing, which
> > provides some entry reuse.
> >
> > My concerns are with the notification capabilities module.
> >
> >   - notification-support:  why is this an enumeration instead of bits?
> >
> > suggest:
> >
> >      typedef notification-support {
> >         type bits {
> >           bit notifications-for-config-changes-supported {
> >             description "The publisher is capable of sending
> >               notifications for config=true nodes, but not
> >               for config=false nodes for the relevant scope
> >               and subscription type." ;
> >           }
> >           bit notifications-for-state-changes-supported {
> >             description "The publisher is capable of sending
> >               notifications for config=false nodes, but not
> >               for config=true nodes for the relevant scope
> >               and subscription type." ;
> >           }
> >         }
> >
> >    - can the names be shorter (config-changes, state-changes)?
> >     these names seem redundant and verbose
>
> +1 for bits and the suggested shorter names
>
>
>
> >   - update-period
> >     this object seems implementation-specific.
> >     The description should be less normative and use 2119 language
> >
> >             A periodic subscription to the selected data nodes must
> >             specify a value that is at least as large or greater than
> >             this
> >
> >       Suggest:  s/must/SHOULD/
>
> I don't think either is correct.  This leaf informs the client what an
> acceptable 'period' value is.  A client that sends a shorter period
> will get a 'period-unsupported' error back, as defined in RFC 8641.
> Perhaps rephrase the sentence to:
>
>   A request for a periodic subscription to the selected data nodes
>   with a smaller period than what this leaf specifies will result in
>   a 'period-unsupported' error.
>
>
>
I agree this is better.



> >   - supported-update-period
> >     - this is very implementation-specific, especially as a leaf-list
> > instead of a
> >       leaf with a range
> >     - also has 'must' language that is inappropriate; s/must/SHOULD/
>
> See above.
>
> >  - max-objects-per-update
> >    - extremely implementation-specific; Does this assume a server
> generates
> > updates
> >      by counting objects and stopping when this value is reached?
> >    - the text is not clear if it means objects or object instances? Do
> > child nodes count
> >      as instances? IMO this object should be removed or changes to max.
> > bytes per update
> >      (also implementation-specific)
>
> I agree that this is implementation-specific, and a byte-count as
> well.  It assumes that the implementation uses very static
> datastructures.   But I think it can be ok - an implementation that
> doesn't have such static limits will not instantiate this leaf.
>
>
Then there should be text in the description-stmt that this leaf will not
be instantiated if the server implementation does not restrict an update
to a specific number of objects.

What about the supported-update-period leaf-list?
If a server allows any centi-second value from 1 sec to 1 hour then this
will
be a leaf-list with 5900 instances in it.  Since there is no reason to
limit the upper-bound
(e.g., client wants 1 statistics update per day) this leaf-list will likely
have millions
of instances in it.

IMO this leaf-list should be removed. The minimum-update-period is
sufficient.



> /martin
>


Andy


>
>
>
> >  - supported-excluded-change-type
> >    - this union has overlapping enumeration implied values
> >     suggest assigning value -2; and value -1; to none and all enums
> >
> >
> > Andy
> >
> >
> >
> > On Tue, Jan 7, 2020 at 5:04 AM Kent Watsen <[e-mail]> wrote:
> >
> > >
> > > Dear WG,
> > >
> > > This is a fairly substantial update that defines a generic
> > > "ietf-system-capabilities’
> > > module and a separate "ietf-notification-capabilities” module that
> > > augments into it.
> > >
> > > We could start WGLC #2 now, but it would be good to get a few
> high-level
> > > reactions
> > > from the WG before doing so.
> > >
> > > Please provide comments as to if you believe the draft is ready for
> WGLC
> > > #2.
> > >
> > > Thanks,
> > > Kent // as shepherd
> > >
> > >
> > > > On Jan 7, 2020, at 3:28 AM, [e-mail] wrote:
> > > >
> > > >
> > > > A new version (-09) has been submitted for
> > > draft-ietf-netconf-notification-capabilities:
> > > >
> > >
> https://www.ietf.org/internet-drafts/draft-ietf-netconf-notification-capabilities-09.txt
> > > >
> > > >
> > > > The IETF datatracker page for this Internet-Draft is:
> > > >
> > >
> https://datatracker.ietf.org/doc/draft-ietf-netconf-notification-capabilities/
> > > >
> > > > Diff from previous version:
> > > >
> > >
> https://www.ietf.org/rfcdiff?url2=draft-ietf-netconf-notification-capabilities-09
> > > >
> > > > Please note that it may take a couple of minutes from the time of
> > > submission
> > > > until the diff is available at tools.ietf.org.
> > > >
> > > > IETF Secretariat.
> > > >
> > >
> > > _______________________________________________
> > > netconf mailing list
> > > [e-mail]
> > > https://www.ietf.org/mailman/listinfo/netconf
> > >
>