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)" <[email protected]> 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" <[email protected]> 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" <[email protected]> wrote: > > > > "Reshad Rahman (rrahman)" <[email protected]> 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" > <[email protected]> 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 <[email protected]> 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 > > > [email protected] > > > > > > > > > > > > > > > > > > > > > > > > <!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>
