Fix issue DAN-169 #11
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| yang/vyatta-protocols-frr-link-params-v1.yang usr/share/configd/yang/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| { | ||
| "/interfaces/@element/link-params/@enter": "link-params", | ||
| "/interfaces/@element/link-params/@exit": "exit-link-params", | ||
| "/interfaces/@element/link-params/enable": "enable", | ||
| "/interfaces/@element/link-params/admin-grp": "admin-grp {/@text}", | ||
| "/interfaces/@element/link-params/ava-bw": "ava-bw {/@text}", | ||
| "/interfaces/@element/link-params/delay": "delay {/@text}", | ||
| "/interfaces/@element/link-params/delay-variation": "delay-variation {/@text}", | ||
| "/interfaces/@element/link-params/max-bw": "max-bw {/@text}", | ||
| "/interfaces/@element/link-params/max-rsv-bw": "max-rsv-bw {/@text}", | ||
| "/interfaces/@element/link-params/metric": "metric {/@text}", | ||
| "/interfaces/@element/link-params/packet-loss": "packet-loss {/@text}", | ||
| "/interfaces/@element/link-params/res-bw": "res-bw {/@text}", | ||
| "/interfaces/@element/link-params/unrsv-bw": "unrsv-bw {/@text}", | ||
| "/interfaces/@element/link-params/use-bw": "use-bw {/@text}" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| module vyatta-protocols-frr-link-params-v1 { | ||
| namespace "urn:vyatta.com:mgmt:vyatta-protocols-frr-link-params:1"; | ||
| prefix vyatta-protocols-frr-link-params-v1; | ||
|
|
||
| import vyatta-types-v1 { | ||
| prefix types; | ||
| } | ||
| import configd-v1 { | ||
| prefix configd; | ||
| } | ||
| import vyatta-interfaces-v1 { | ||
| prefix if; | ||
| } | ||
| import vyatta-interfaces-dataplane-v1 { | ||
| prefix interfaces-dataplane; | ||
| } | ||
| import vyatta-interfaces-loopback-v1 { | ||
| prefix interfaces-loopback; | ||
| } | ||
| import vyatta-interfaces-switch-v1 { | ||
| prefix interfaces-switch; | ||
| } | ||
|
|
||
| organization "Niral Networks Private Limited"; | ||
| contact | ||
| "Niral Networks | ||
| Postal: Sigma Soft Tech Park, Whitefield Main Road, | ||
| Bengaluru, India 560066 | ||
| Web: www.niralnetworks.com"; | ||
|
|
||
| description | ||
| "Copyright (c) 2019-2020, Niral Networks Intellectual Property. | ||
| All rights reserved. | ||
|
|
||
| Redistribution and use in source and binary forms, with or | ||
| without modification, are permitted provided that the following | ||
| conditions are met: | ||
|
|
||
| 1. Redistributions of source code must retain the above copyright | ||
| notice, this list of conditions and the following disclaimer. | ||
| 2. Redistributions in binary form must reproduce the above | ||
| copyright notice, this list of conditions and the following | ||
| disclaimer in the documentation and/or other materials provided | ||
| with the distribution. | ||
| 3. Neither the name of the copyright holder nor the names of its | ||
| contributors may be used to endorse or promote products derived | ||
| from this software without specific prior written permission. | ||
|
|
||
| THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS | ||
| 'AS IS' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT | ||
| LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS | ||
| FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE | ||
| COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, | ||
| INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, | ||
| BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; | ||
| LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER | ||
| CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | ||
| LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN | ||
| ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE | ||
| POSSIBILITY OF SUCH DAMAGE. | ||
|
|
||
| SPDX-License-Identifier: BSD-3-Clause | ||
| The YANG module package for vyatta-protocols-frr-link-params-v1"; | ||
|
|
||
| revision 2021-03-02 { | ||
| description | ||
| "Added link-params option."; | ||
|
There was a problem hiding this comment. 'Initial revision' would seem more appropriate. There was a problem hiding this comment. The description has been modified to "Initial revision" |
||
| } | ||
|
|
||
| grouping link-params { | ||
| container link-params { | ||
| configd:help "Configure interface link parameters"; | ||
| leaf admin-group { | ||
| type string { | ||
| pattern "0[Xx][a-fA-F0-9]+"; | ||
| } | ||
| must "(../enable)" { | ||
|
There was a problem hiding this comment. Does this not require an 'enable' leaf in this container, which is missing? Did you mean '../../enable' or would you be better removing all these musts, and having it so any link-param configuration here implicitly enables link-params? Or make it a presence container so configuration of the container with no child nodes means link-params are enabled? There was a problem hiding this comment. The must statement has been removed in the updated code. There was a problem hiding this comment. Given that enable is gone, these must statements don't seem right (on all of the leaf nodes below as well). There was a problem hiding this comment. I've removed the must statement as enable was gone |
||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Administrative group membership"; | ||
| description "Administrative group membership"; | ||
| } | ||
| leaf available-bandwidth { | ||
| type uint32; | ||
|
There was a problem hiding this comment. Much cleaner. Please add units statements to all of these so it's clear what units are being used. Maybe also mention the units in the configd:help as units aren't exposed via the CLI help. There was a problem hiding this comment. added units statements in each of the leaf nodes and configd:help There was a problem hiding this comment. So you're never going to exceed ~4GB? Would a uint64 be safer here? Same applies to all bandwidth fields There was a problem hiding this comment. uint64 would be able to allocate more bandwidth than uint32.. It has been modified There was a problem hiding this comment. Please push changes - presumably you will be modifying ALL bandwidth nodes to uint64? There was a problem hiding this comment. I've pushed my changes. In all the bandwidth I've modified to uint64 |
||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Unidirectional Available Bandwidth"; | ||
|
There was a problem hiding this comment. You might want to give the user a clue about units? bits? Gigabytes? There was a problem hiding this comment. The units have already been added i.e (Bytes/second). |
||
| description "Unidirectional Available Bandwidth"; | ||
| } | ||
| leaf delay { | ||
| type uint32 { | ||
| range "0..16777215"; | ||
|
There was a problem hiding this comment. The units have already been added in the previous commit i.e (micro-second). |
||
| } | ||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Unidirectional Average Link Delay"; | ||
| description "Unidirectional Average Link Delay"; | ||
| } | ||
| leaf delay-variation { | ||
| type uint32 { | ||
| range "0..16777215"; | ||
|
There was a problem hiding this comment. The units have already been added in the previous commit i.e (micro-second). |
||
| } | ||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Unidirectional Link Delay Variation"; | ||
| description "Unidirectional Link Delay Variation"; | ||
| } | ||
| leaf max-bandwidth { | ||
| type uint32; | ||
|
There was a problem hiding this comment. The units have already been added in the previous commit i.e (Bytes/second). |
||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Maximum bandwidth that can be used"; | ||
| description "Maximum bandwidth that can be used"; | ||
| } | ||
| leaf max-reserved-bandwidth { | ||
|
There was a problem hiding this comment. Can this exceed max-bandwidth? If not then perhaps a must statement enforcing this would be wise. There was a problem hiding this comment. How do these 2 relate to available-bandwidth, residual, unreserved and utilised? There was a problem hiding this comment.
There was a problem hiding this comment. What determines the max bandwidth that can be used? Is this an inherent property of the link, or, like ADSL, depends on how well the link is working at a given time? There was a problem hiding this comment. I am not sure about it. In frr the max-bandwidth is set upon setting link-param however in Danos CLI user can provide max-bandwidth according to the need. There was a problem hiding this comment. How can you code a feature when you don't know what it means??? |
||
| type uint32; | ||
|
There was a problem hiding this comment. The units have already been added in the previous commit i.e (Bytes/second). |
||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Maximum bandwidth that may be reserved"; | ||
| description "Maximum bandwidth that may be reserved"; | ||
| } | ||
| leaf metric { | ||
|
There was a problem hiding this comment. Could you ever have other metrics? Maybe mpls-te-metric would be a better name? There was a problem hiding this comment. It seems better.. It has been changed |
||
| type uint32 { | ||
| range "1..4294967295"; | ||
|
There was a problem hiding this comment. I think this is equivalent '1..max' which might read better? There was a problem hiding this comment. max would be better to read. It has been updated. |
||
| } | ||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Link metric for MPLS-TE purpose"; | ||
| description "Link metric for MPLS-TE purpose"; | ||
| } | ||
| leaf neighbor { | ||
| type types:ipv4-address; | ||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Configure remote ASBR information (Neighbor IP address and AS number)"; | ||
| description "Configure remote ASBR information (Neighbor IP address and AS number)"; | ||
| } | ||
| leaf packet-loss { | ||
| type uint32; | ||
|
There was a problem hiding this comment. The units have already been added in the previous commit i.e (Percentage). |
||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Configure remote ASBR information (Neighbor IP address and AS number)"; | ||
| description "Configure remote ASBR information (Neighbor IP address and AS number)"; | ||
|
|
||
| } | ||
| leaf residual-bandwidth { | ||
| type uint32; | ||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Unidirectional Residual Bandwidth"; | ||
| description "Unidirectional Residual Bandwidth"; | ||
|
There was a problem hiding this comment. What is residual bandwidth? Description can (and often should) be multi-line to give a fuller description than configd:help. There was a problem hiding this comment. Fuller description has been provided for residual bandwidth following draft-ietf-isis-te-metrics-extension-03. |
||
| } | ||
| leaf unreserved-bandwidth { | ||
| type uint32; | ||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Unreserved bandwidth at each priority level"; | ||
| description "Unreserved bandwidth at each priority level"; | ||
| } | ||
| leaf utilised-bandwidth { | ||
|
There was a problem hiding this comment. Is this config or state? Please provide a better description. There was a problem hiding this comment. Fuller description has been provided for utilised bandwidth following draft-ietf-isis-te-metrics-extension-03. There was a problem hiding this comment. This still sounds more like state than config - 'as measured in the router' is state not config AFAICT. There was a problem hiding this comment. This similar command is there in vtysh shell where I can use this to adjust utilized-bandwidth in Bytes/second and was req in Danos CLI and in the draft-ietf-isis-te-metrics-extension-03 the above description is given of the same therefore I've provided that description. |
||
| type uint32; | ||
| must "(../enable)" { | ||
| error-message "link-params enable must be set."; | ||
| } | ||
| configd:help "Unidirectional Utilised Bandwidth"; | ||
| description "Unidirectional Utilised Bandwidth"; | ||
|
|
||
| } | ||
| } | ||
| } | ||
|
|
||
| augment /if:interfaces/interfaces-dataplane:dataplane { | ||
| uses link-params; | ||
| } | ||
| augment /if:interfaces/interfaces-dataplane:dataplane/interfaces-dataplane:vif/interfaces-dataplane:ip { | ||
| uses link-params; | ||
| } | ||
| augment /if:interfaces/interfaces-loopback:loopback { | ||
| uses link-params; | ||
| } | ||
| augment /if:interfaces/interfaces-switch:switch/interfaces-switch:vif { | ||
| uses link-params; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this just be 2021 if a new file? Certainly should list 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2021 has been listed i.e 2019-2021