Re: Yangdoctors early review of draft-ietf-bfd-unsolicited-01

["Reshad Rahman (rrahman)" <[e-mail]>]

And this is what the changes would look like.

On 2019-08-21, 4:14 PM, "Reshad Rahman (rrahman)" <[e-mail]> wrote:

    Hi Jeff,
    
    Yes, to me it makes sense to do the change suggested by Martin (add "default tx-rx-intervals;" to the choice statement). BFD YANG co-authors, please respond asap if you disagree.
    
    Regards,
    Reshad.
    
    On 2019-08-21, 4:11 PM, "Jeffrey Haas" <[e-mail]> wrote:
    
        Reshad,
        
        If procedures permit it (I'm unclear on the detail), does it make sense to
        pull the BFD yang module for a fix from the editor queue?
        
        -- Jeff
        
        On Mon, Aug 19, 2019 at 07:31:27PM +0000, Reshad Rahman (rrahman) wrote:
        > I was looking at an old copy of the doc which didn't have default. So yes, mandatory doesn't make sense with the default statements.
        > 
        > Your assumption below wrt the intention is correct. I don't know how feasible it is to add this while it's in the editor q.
        > 
        > Regards,
        > Reshad.
        > 
        > On 2019-08-19, 3:18 PM, "Martin Bjorklund" <[e-mail]> wrote:
        > 
        >     "Reshad Rahman (rrahman)" <[e-mail]> wrote:
        >     > Thanks Martin and Mahesh.
        >     > 
        >     > I believe we should add a mandatory statement to the choic (speaking
        >     > as BFD YANG co-author,)
        >     
        >     But then it is not clear why all leafs in the cases have default
        >     statements.
        >     
        >     Since the 'single-interval' case is optional with a if-feature (which
        >     BTW is weird since it is trivial to implement), and the only other
        >     case has default values on both its leafs, I would have assumed that
        >     the intention was that if nothing is configured, the server should use
        >     1000000 microseconds for the intervals.  If this is the intention,
        >     perhaps a statement:  "default tx-rx-intervals;" can be added to the
        >     module, even though the doc is in the RFC ed q.
        >     
        >     
        >     /martin
        >     
        >     
        >     
        >     > 
        >     > Just created https://github.com/bfd-wg
        >     > 
        >     > Regards,
        >     > Reshad.
        >     > 
        >     > 
        >     > On 2019-08-19, 2:45 PM, "Mahesh Jethanandani" <[e-mail]> wrote:
        >     > 
        >     >     [Adding the authors of BFD YANG module]
        >     >     
        >     >     Martin brings up a good point. But since the document that contains ietf-bfd-types is sitting in RFC Ed Queue, this will have to go into a bis document.
        >     >     
        >     >     Chairs, could you create a bfd-wg in GitHub for us to track this as an issue to be fixed as part of a bis document?
        >     >     
        >     >     > On Aug 19, 2019, at 4:29 AM, Martin Björklund via Datatracker <[e-mail]> wrote:
        >     >     > 
        >     >     > Reviewer: Martin Björklund
        >     >     > Review result: Ready with Nits
        >     >     > 
        >     >     > I have reviewed this document from a YANG model perspective only.
        >     >     > 
        >     >     > My only comment is actually for a grouping defined in ietf-bfd-type, but used
        >     >     > in this module.  There is a choice "interval-config-type":
        >     >     > 
        >     >     >  +--rw unsolicited {bfd-unsol:unsolicited-params-global}?
        >     >     >       +--rw enable?                           boolean
        >     >     >       +--rw local-multiplier?                 multiplier
        >     >     >       +--rw (interval-config-type)?
        >     >     >          +--:(tx-rx-intervals)
        >     >     >          |  +--rw desired-min-tx-interval?    uint32
        >     >     >          |  +--rw required-min-rx-interval?   uint32
        >     >     >          +--:(single-interval) {single-minimum-interval}?
        >     >     >             +--rw min-interval?               uint32
        >     >     > 
        >     >     > This choice is not mandatory and doesn't have a default case, so the question
        >     >     > is what happens if no nodes from the choice has been configured?   I would
        >     >     > expect the choice to have a default case (but this then would apply to
        >     >     > ietf-bfd-types, not this document.)
        >     >     > 
        >     >     > 
        >     >     
        >     >     Mahesh Jethanandani
        >     >     [e-mail]
        >     >     
        >     >     
        >     >     
        >     >     
        >     > 
        >     
        >