Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSPFD yang #6055

Closed
wants to merge 42 commits into from
Closed

OSPFD yang #6055

wants to merge 42 commits into from

Conversation

Spantik
Copy link
Member

@Spantik Spantik commented Mar 20, 2020

Initial version of OSPF configuration Yang.

chiragshah6 and others added 28 commits December 9, 2019 14:20
module: frr-vrf
  +--rw lib
     +--rw vrf* [name]
        +--rw name      string
        +--ro id?       uint32
        +--ro active?   boolean <false>
        +--rw netns {netns}?
           +--rw name?   string

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
lib: Adding frr-vrf yang module to common place
so it can be accessed from all frr modules.

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
module: frr-vrf
  +--rw lib
     +--rw vrf* [name]
        +--rw name      string
        +--ro id?       uint32
        +--ro active?   boolean <false>
        +--rw netns {netns}?
           +--rw name?   string

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
lib: Adding frr-vrf yang module to common place
so it can be accessed from all frr modules.

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
module: frr-interface
  +--rw lib
     +--rw interface* [name vrf]
        +--rw name           string
        +--rw vrf            frr-vrf:vrf-ref
        +--rw description?   string

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
yang: add vrf ref to interface model
Yang files for staticd to use northbound APIs

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: vishaldhingra <vdhingra@vmware.com>
Signed-off-by: vishaldhingra <vdhingra@vmware.com>
Yang files for basic frr-routing used by other
daemons like staticd and pim

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: vishaldhingra <vdhingra@vmware.com>
Signed-off-by: vishaldhingra <vdhingra@vmware.com>
A common nexthop and group nexthop yang data model
for all protocols in FRR.

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: Vishaldhingra <vdhingra@vmware.com>
Signed-off-by: Santosh P K <sapk@vmware.com>
lib: FRR next-hop yang data model.
lib: yang defination for basic routing frr-routing
module: frr-vrf
  +--rw lib
     +--rw vrf* [name]
        +--rw name      string
        +--ro id?       uint32
        +--ro active?   boolean <false>
        +--rw netns {netns}?
           +--rw name?   string

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
lib: Adding frr-vrf yang module to common place
so it can be accessed from all frr modules.

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
module: frr-interface
  +--rw lib
     +--rw interface* [name vrf]
        +--rw name           string
        +--rw vrf            frr-vrf:vrf-ref
        +--rw description?   string

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
A common nexthop and group nexthop yang data model
for all protocols in FRR.

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: Vishaldhingra <vdhingra@vmware.com>
Signed-off-by: Santosh P K <sapk@vmware.com>
Yang files for staticd to use northbound APIs

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: vishaldhingra <vdhingra@vmware.com>
Signed-off-by: vishaldhingra <vdhingra@vmware.com>
Yang files for basic frr-routing used by other
daemons like staticd and pim

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: vishaldhingra <vdhingra@vmware.com>
Signed-off-by: vishaldhingra <vdhingra@vmware.com>
module: frr-vrf
  +--rw lib
     +--rw vrf* [name]
        +--rw name      string
        +--ro id?       uint32
        +--ro active?   boolean <false>
        +--rw netns {netns}?
           +--rw name?   string

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
lib: Adding frr-vrf yang module to common place
so it can be accessed from all frr modules.

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
module: frr-interface
  +--rw lib
     +--rw interface* [name vrf]
        +--rw name           string
        +--rw vrf            frr-vrf:vrf-ref
        +--rw description?   string

Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
A common nexthop and group nexthop yang data model
for all protocols in FRR.

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: Vishaldhingra <vdhingra@vmware.com>
Signed-off-by: Santosh P K <sapk@vmware.com>
Yang files for staticd to use northbound APIs

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: vishaldhingra <vdhingra@vmware.com>
Signed-off-by: vishaldhingra <vdhingra@vmware.com>
Yang files for basic frr-routing used by other
daemons like staticd and pim

Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: vishaldhingra <vdhingra@vmware.com>
Signed-off-by: vishaldhingra <vdhingra@vmware.com>
@Spantik
Copy link
Member Author

Spantik commented Mar 20, 2020

OSPF Yang tree.

Screenshot 2020-03-20 at 12 48 46 PM

Screenshot 2020-03-20 at 12 49 20 PM

Screenshot 2020-03-20 at 12 49 50 PM

Screenshot 2020-03-20 at 12 50 16 PM



container timers {
leaf dead-interval {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation here as well

leaf out {
type empty;
description
"In direction.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out?

leaf in {
type empty;
description
"Out direction.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In?

description
"Stable IP address of the advertising router.";
}
container inrer-as {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inrer?

enum "cisco" {
value 1;
description
"Alternative ABR, cisco implementation.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two spaces?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cisco (capital first maybe)?

leaf rfc1583compatibility {
type empty;
description
"Enable the RFC1583Compatibility flag.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enable the RFC1583 compatibility flag.

leaf opaque-lsa {
type empty;
description
"Enable the Opaque-LSA capability (rfc2370)";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use RFC or rfc, but not mixed everywhere?

range "10..1800";
}
description
"Timer value in seconds.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what timer? :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a poor description indeed.

Also, please use units seconds;.

description
"Prefix SID.";
}
leaf no-php-flag {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

list neighbor {
key "as";
leaf as {
type uint16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 is valid AS as well?

Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just recommend reviewing all the typos, whitespaces, indentation.

Copy link
Member

@rwestphal rwestphal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Spantik Thanks for starting this work!

A few preliminary review comments:

  • For configuration data, please use boolean leafs instead of empty leafs. That's necessary for the show configuration <running|candidate|...> with-defaults command to work properly. Using empty leafs there's no way to know if the configuration option is enabled by default or not.
  • This module makes mixed use of hyphens and underscores (e.g. leaf key_id, then leaf mds-key a few lines below). I think it would be better to consolidate on hyphens only (that's what the IETF modules seem to be doing, even though there doesn't seem to be any guideline in that regard).
  • This module contains lots of indentation and whitespace issues. Examples: trailing whitespaces, redundant new lines, lines that are too long, etc. You can use yanglint to format this module automatically for you.
  • Finally, you could reuse definitions from ietf-ospf.yang whenever possible. That would give us nice node descriptions for free and make the native<->IETF module translation process easier in the future.

"A list of area objects";

leaf area_id {
type inet:ipv4-address;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the CLI we can specify the OSPF area using either an IPv4 address or an unsigned integer, and the CLI retains the chosen format when displaying the running configuration. With this YANG module we won't be able to preserve that behavior, so we need to discuss if that's acceptable. There are a few options to preserve the old behavior, like using YANG unions, choices, or extending the NB to have the ability to associate metadata to data nodes. But all solutions imply an extra complexity that might not be worth the effort.

range "10..1800";
}
description
"Timer value in seconds.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a poor description indeed.

Also, please use units seconds;.

range "1..4294967";
}
description
"The reference bandwidth in terms of Mbits per second.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use units Mbits; like in the IETF module.

}
}
leaf retransmit-interval {
type uint16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and other timers are missing ranges ("1..65535" in this case).

mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jun 19, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by  : Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jun 19, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jun 19, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
@Spantik
Copy link
Member Author

Spantik commented Jun 19, 2020

A new PR has been raised with review comments addressed #6616. Closing this.

@Spantik Spantik closed this Jun 19, 2020
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jun 30, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jun 30, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jul 2, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jul 14, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jul 14, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jul 14, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jul 21, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Jul 22, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Aug 6, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Revision History:
1. Fixed review comments.
2. Removed ospf list with id as key, name can be used as key
3. Corrected the alignment

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
mobash-rasool added a commit to mobash-rasool/frr that referenced this pull request Aug 6, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Revision History:
1. Fixed review comments.
2. Removed ospf list with id as key, name can be used as key
3. Corrected the alignment

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
maduri111 pushed a commit to maduri111/frr that referenced this pull request Aug 9, 2020
Initial version of OSPF configuration Yang.

Raised new PR with review comment fixes on top of PR
FRRouting#6055

Revision History:
1. Fixed review comments.
2. Removed ospf list with id as key, name can be used as key
3. Corrected the alignment

Co-authored-by : Santosh P K <sapk@vmware.com>
Co-authored-by : Mobashshera Rasool <mrasool@vmware.com>
Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants