On Wed, Aug 21, 2019 at 10:33:01PM +0000, Reshad Rahman (rrahman) wrote: > And this is what the changes would look like. That looks fine to me. I don't expect it to be controversial. -- Jeff > > On 2019-08-21, 4:14 PM, "Reshad Rahman (rrahman)" <rrahman@xxxxxxxxx> 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" <jhaas@xxxxxxxx> 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" <mbj@xxxxxxxxxx> wrote: > > > > "Reshad Rahman (rrahman)" <rrahman@xxxxxxxxx> 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" <mjethanandani@xxxxxxxxx> 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 <noreply@xxxxxxxx> 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 > > > mjethanandani@xxxxxxxxx > > > > > > > > > > > > > > > > > > > > > > > > <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> > <!-- saved from url=(0049)https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht --> > <html xmlns="http://www.w3.org/1999/xhtml"><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> > > <meta http-equiv="Content-Style-Type" content="text/css"> > <title>Diff: draft-ietf-bfd-yang.txt - draft-ietf-bfd-yang-17.txt</title> > <style type="text/css"> > body { margin: 0.4ex; margin-right: auto; } > tr { } > td { white-space: pre; font-family: monospace; vertical-align: top; font-size: 0.86em;} > th { font-size: 0.86em; } > .small { font-size: 0.6em; font-style: italic; font-family: Verdana, Helvetica, sans-serif; } > .left { background-color: #EEE; } > .right { background-color: #FFF; } > .diff { background-color: #CCF; } > .lblock { background-color: #BFB; } > .rblock { background-color: #FF8; } > .insert { background-color: #8FF; } > .delete { background-color: #ACF; } > .void { background-color: #FFB; } > .cont { background-color: #EEE; } > .linebr { background-color: #AAA; } > .lineno { color: red; background-color: #FFF; font-size: 0.7em; text-align: right; padding: 0 2px; } > .elipsis{ background-color: #AAA; } > .left .cont { background-color: #DDD; } > .right .cont { background-color: #EEE; } > .lblock .cont { background-color: #9D9; } > .rblock .cont { background-color: #DD6; } > .insert .cont { background-color: #0DD; } > .delete .cont { background-color: #8AD; } > .stats, .stats td, .stats th { background-color: #EEE; padding: 2px 0; } > span.hide { display: none; color: #aaa;} a:hover span { display: inline; } tr.change { background-color: gray; } > tr.change a { text-decoration: none; color: black } > </style> > <script> > var chunk_index = 0; > var old_chunk = null; > > function format_chunk(index) { > var prefix = "diff"; > var str = index.toString(); > for (x=0; x<(4-str.length); ++x) { > prefix+='0'; > } > return prefix + str; > } > > function find_chunk(n){ > return document.querySelector('tr[id$="' + n + '"]'); > } > > function change_chunk(offset) { > var index = chunk_index + offset; > var new_str; > var new_chunk; > > new_str = format_chunk(index); > new_chunk = find_chunk(new_str); > if (!new_chunk) { > return; > } > if (old_chunk) { > old_chunk.style.outline = ""; > } > old_chunk = new_chunk; > old_chunk.style.outline = "1px solid red"; > window.location.replace("#" + new_str) > window.scrollBy(0,-100); > chunk_index = index; > } > > document.onkeydown = function(e) { > switch (e.keyCode) { > case 78: > change_chunk(1); > break; > case 80: > change_chunk(-1); > break; > } > }; > </script> > </head> > <body> > <table border="0" cellpadding="0" cellspacing="0"> > <tbody><tr id="part-1" bgcolor="orange"><th></th><th><a href="https://tools.ietf.org/rfcdiff?url2=draft-ietf-bfd-yang.txt" style="color:#008; text-decoration:none;"><</a> <a href="https://tools.ietf.org/html/draft-ietf-bfd-yang.txt" style="color:#008">draft-ietf-bfd-yang.txt</a> </th><th> </th><th> <a href="https://tools.ietf.org/html/draft-ietf-bfd-yang-17.txt" style="color:#008">draft-ietf-bfd-yang-17.txt</a> <a href="https://tools.ietf.org/rfcdiff?url1=draft-ietf-bfd-yang-17.txt" style="color:#008; text-decoration:none;">></a></th><th></th></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr id="part-1" class="change"><td></td><th><small>skipping to change at</small><a href="https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht#part-1"><em> page 26, line 26<span class="hide"> ??</span></em></a></th><th> </th><th><small>skipping to change at</small><a href="https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht#part-1"><em> page 26, line 26<span class="hide"> ??</span></em></a></th><td></td></tr> > <tr><td class="lineno"></td><td class="left"> }</td><td> </td><td class="right"> }</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> }</td><td> </td><td class="right"> }</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> <CODE ENDS></td><td> </td><td class="right"> <CODE ENDS></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left">2.13. BFD types YANG Module</td><td> </td><td class="right">2.13. BFD types YANG Module</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> This YANG module imports typedefs from [RFC6991], [RFC8177] and the</td><td> </td><td class="right"> This YANG module imports typedefs from [RFC6991], [RFC8177] and the</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> "control-plane-protocol" identity from [RFC8349].</td><td> </td><td class="right"> "control-plane-protocol" identity from [RFC8349].</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr id="diff0001"><td></td></tr> > <tr><td class="lineno"></td><td class="lblock"><CODE BEGINS> file "ietf-bfd-types@201<span class="delete">8-08-0</span>1.yang"</td><td> </td><td class="rblock"><CODE BEGINS> file "ietf-bfd-types@201<span class="insert">9-08-2</span>1.yang"</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left">module ietf-bfd-types {</td><td> </td><td class="right">module ietf-bfd-types {</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> yang-version 1.1;</td><td> </td><td class="right"> yang-version 1.1;</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> namespace "urn:ietf:params:xml:ns:yang:ietf-bfd-types";</td><td> </td><td class="right"> namespace "urn:ietf:params:xml:ns:yang:ietf-bfd-types";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> prefix "bfd-types";</td><td> </td><td class="right"> prefix "bfd-types";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> // RFC Ed.: replace occurences of XXXX with actual RFC number and</td><td> </td><td class="right"> // RFC Ed.: replace occurences of XXXX with actual RFC number and</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr id="part-2" class="change"><td></td><th><small>skipping to change at</small><a href="https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht#part-2"><em> page 27, line 49<span class="hide"> ??</span></em></a></th><th> </th><th><small>skipping to change at</small><a href="https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht#part-2"><em> page 27, line 49<span class="hide"> ??</span></em></a></th><td></td></tr> > <tr><td class="lineno"></td><td class="left"> to the license terms contained in, the Simplified BSD License</td><td> </td><td class="right"> to the license terms contained in, the Simplified BSD License</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> set forth in Section 4.c of the IETF Trust's Legal Provisions</td><td> </td><td class="right"> set forth in Section 4.c of the IETF Trust's Legal Provisions</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> Relating to IETF Documents</td><td> </td><td class="right"> Relating to IETF Documents</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> (http://trustee.ietf.org/license-info).</td><td> </td><td class="right"> (http://trustee.ietf.org/license-info).</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> This version of this YANG module is part of RFC XXXX; see</td><td> </td><td class="right"> This version of this YANG module is part of RFC XXXX; see</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> the RFC itself for full legal notices.";</td><td> </td><td class="right"> the RFC itself for full legal notices.";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> reference "RFC XXXX";</td><td> </td><td class="right"> reference "RFC XXXX";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr id="diff0002"><td></td></tr> > <tr><td class="lineno"></td><td class="lblock"> revision 201<span class="delete">8-08-0</span>1 {</td><td> </td><td class="rblock"> revision 201<span class="insert">9-08-2</span>1 {</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> description "Initial revision.";</td><td> </td><td class="right"> description "Initial revision.";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> reference "RFC XXXX: YANG Data Model for BFD";</td><td> </td><td class="right"> reference "RFC XXXX: YANG Data Model for BFD";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> }</td><td> </td><td class="right"> }</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> /*</td><td> </td><td class="right"> /*</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> * Feature definitions</td><td> </td><td class="right"> * Feature definitions</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> */</td><td> </td><td class="right"> */</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> feature single-minimum-interval {</td><td> </td><td class="right"> feature single-minimum-interval {</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> description</td><td> </td><td class="right"> description</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> "This feature indicates that the server supports configuration</td><td> </td><td class="right"> "This feature indicates that the server supports configuration</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> of one minimum interval value which is used for both transmit and</td><td> </td><td class="right"> of one minimum interval value which is used for both transmit and</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr id="part-3" class="change"><td></td><th><small>skipping to change at</small><a href="https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht#part-3"><em> page 31, line 43<span class="hide"> ??</span></em></a></th><th> </th><th><small>skipping to change at</small><a href="https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht#part-3"><em> page 31, line 43<span class="hide"> ??</span></em></a></th><td></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> grouping base-cfg-parms {</td><td> </td><td class="right"> grouping base-cfg-parms {</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> description "BFD grouping for base config parameters.";</td><td> </td><td class="right"> description "BFD grouping for base config parameters.";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> leaf local-multiplier {</td><td> </td><td class="right"> leaf local-multiplier {</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> type multiplier;</td><td> </td><td class="right"> type multiplier;</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> default 3;</td><td> </td><td class="right"> default 3;</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> description "Multiplier transmitted by local system.";</td><td> </td><td class="right"> description "Multiplier transmitted by local system.";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> }</td><td> </td><td class="right"> }</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"></td><td> </td><td class="right"></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> choice interval-config-type {</td><td> </td><td class="right"> choice interval-config-type {</td><td class="lineno"></td></tr> > <tr id="diff0003"><td></td></tr> > <tr><td class="lineno"></td><td class="lblock"></td><td> </td><td class="rblock"><span class="insert"> default tx-rx-intervals;</span></td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> description</td><td> </td><td class="right"> description</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> "Two interval values or one value used for both transmit and</td><td> </td><td class="right"> "Two interval values or one value used for both transmit and</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> receive.";</td><td> </td><td class="right"> receive.";</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> case tx-rx-intervals {</td><td> </td><td class="right"> case tx-rx-intervals {</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> leaf desired-min-tx-interval {</td><td> </td><td class="right"> leaf desired-min-tx-interval {</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> type uint32;</td><td> </td><td class="right"> type uint32;</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> units microseconds;</td><td> </td><td class="right"> units microseconds;</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> default 1000000;</td><td> </td><td class="right"> default 1000000;</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> description</td><td> </td><td class="right"> description</td><td class="lineno"></td></tr> > <tr><td class="lineno"></td><td class="left"> "Desired minimum transmit interval of control packets.";</td><td> </td><td class="right"> "Desired minimum transmit interval of control packets.";</td><td class="lineno"></td></tr> > > <tr><td></td><td class="left"></td><td> </td><td class="right"></td><td></td></tr> > <tr id="end" bgcolor="gray"><th colspan="5" align="center"> End of changes. 3 change blocks. </th></tr> > <tr class="stats"><td></td><th><i>2 lines changed or deleted</i></th><th><i> </i></th><th><i>3 lines changed or added</i></th><td></td></tr> > <tr><td colspan="5" align="center" class="small"><br>This html diff was produced by rfcdiff 1.47. The latest version is available from <a href="http://www.tools.ietf.org/tools/rfcdiff/">http://tools.ietf.org/tools/rfcdiff/</a> </td></tr> > </tbody></table> > > > </body></html>