yang: models for route map and filters #4733

rzalamena · 2019-07-25T20:34:09Z

Requirement

This PR depends on #4649 .

Summary

Here is my suggestion of YANG models for the FRR route-map, access-list and prefix list implementation.

I've read and got inspiration from the following sources:

Source code;
IETF's route policy model: https://tools.ietf.org/html/draft-ietf-rtgwg-policy-model-06#section-10 ;
Our documentation
- Route map: http://docs.frrouting.org/en/latest/routemap.html
- Filters: http://docs.frrouting.org/en/latest/filter.html
Cisco route map YANG model: https://github.com/YangModels/yang/blob/master/vendor/cisco/xe/1671/Cisco-IOS-XE-route-map.yang
Cisco access list YANG model: https://github.com/YangModels/yang/blob/master/vendor/cisco/xe/1661/Cisco-IOS-XE-acl.yang

The current models ONLY cover the functionality implemented by lib, but I've enabled the models to expect and receive augmentations later to support code outside of libfrr. For an example: the current access list only covers IPv4, IPv6 and MAC, however BGP implements access lists for AS, communities and ext communities. You can find clues in the YANG models, here is a snippet:

            /*
             * Protocol YANG models should augment the parent node to
             * contain the routing protocol specific value. The protocol
             * must also augment `condition-value` to include its specific
             * values or expand the `when` statement on the existing cases.
             */
            enum routing-protocol-specific {
              description "Match a routing protocol specific type";
              value 100;
            }

Developers would add another leaf to signal its custom type (we can't augment enumerations, so we need another leaf) and then augment the value or condition-value choices to include their custom choice values.

My plan now is to implement northbound for EIGRP, filters and lastly route maps and see how they integrate. Please let me know if you need more clarifications.

NOTE: there is an IETF ACL YANG model, but its intent is to support firewalls and not exactly routing protocols access-lists ( https://tools.ietf.org/html/rfc8519 ).

Components

yang

LabN-CI · 2019-07-25T21:12:14Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/4733 `11bc133`
Date	07/25/2019
Start	16:50:28
Finish	17:12:13
Run-Time	21:45
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-07-25-16:50:28.txt
Log	autoscript-2019-07-25-16:51:15.log.bz2
Memory	428 427 361

For details, please contact louberger

NetDEF-CI · 2019-07-26T22:59:15Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8447/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8447/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: changelog-should-mention-nmu

CLANG Static Analyzer Summary

Github Pull Request 4733, comparing to Git base SHA 51e75ed

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8447/artifact/shared/static_analysis/index.html

riw777 · 2019-07-30T15:04:10Z

This looks okay to me -- it might be good to have a file showing the model themselves, and documentation?

rzalamena · 2019-07-30T15:59:40Z

This looks okay to me -- it might be good to have a file showing the model themselves, and documentation?

@riw777 I couldn't get JSON representation because that would require instance data and we don't have it yet, but I got a tree representation of the YANG models that should help...

https://gist.github.com/rzalamena/109f6372bd93bef0b466822bb8c27c8f

If you want to generate it yourself later, you can use the following command:

yanglint -f tree /path/to/frr/yang/frr-route-map.yang

donaldsharp · 2019-07-30T23:42:31Z

LGTM

rwestphal

Looks good to me, but please see my review comments.

Also, some daemons support ACLs but not prefix-lists. So I think it would be better to move prefix-lists to a separate module of their own. This would also simplify the code as ACLs and prefix-lists are implemented on different files.

yang/frr-filter.yang

rwestphal · 2019-08-01T20:40:08Z

yang/frr-filter.yang

+      enum permit {
+        description "Accept an entry";
+        value 1;
+      }


Looks good. The code also contains a FILTER_DYNAMIC enum value but it isn't being used anywhere. I think you can remove it later when converting the code.

yang/frr-filter.yang

rwestphal · 2019-08-01T20:46:19Z

yang/frr-filter.yang

+           Extended access list: source value to match.";
+        mandatory true;
+
+        case host {


This and the other case statements from here can be omitted for simplicity.

RFC 7950 says the following (section 7.9.2):

As a shorthand, the "case" statement can be omitted if the branch
contains a single "anydata", "anyxml", "choice", "container", "leaf",
"list", or "leaf-list" statement.

I agree... I don't expect to ever change this to add more cases.

yang/frr-filter.yang

rwestphal · 2019-08-01T21:39:44Z

yang/frr-filter.yang

+  typedef access-list-legacy {
+    description "Standard/Extended IPv4 access list";
+    type uint16 {
+      range "1..199 | 1300..2699";


This range overlaps with "100..199 | 2000..2699" from access-list-extended. Is this correct?

Yes. They overlap because it is the union of standard and extended legacy access lists (using numbers instead of name).

New model based on FRR's CLI and data structures. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

This model contains the description of access-list, prefix-list and other lists used by route map and other filtering interfaces. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

Import the new YANG model filter and use its types. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

@rwestphal

Based on @rwestphal feedback, lets remove `case`s where we don't expect to add more items or items with more than one `leaf`. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

LabN-CI · 2019-08-01T23:22:31Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/4733 `bec0aa8`
Date	08/01/2019
Start	19:00:25
Finish	19:22:30
Run-Time	22:05
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-08-01-19:00:25.txt
Log	autoscript-2019-08-01-19:01:26.log.bz2
Memory	436 431 360

For details, please contact louberger

NetDEF-CI · 2019-08-02T00:36:53Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8498/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8498/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: changelog-should-mention-nmu

CLANG Static Analyzer Summary

Github Pull Request 4733, comparing to Git base SHA cf347ba

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-8498/artifact/shared/static_analysis/index.html

riw777

LGTM

rzalamena added review & merge me look at me! libfrr yang labels Jul 25, 2019

rwestphal reviewed Aug 1, 2019

View reviewed changes

rzalamena added 4 commits August 1, 2019 19:58

yang: initial route-map YANG model import

0f28135

New model based on FRR's CLI and data structures. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

yang: initial filter YANG model import

2332428

This model contains the description of access-list, prefix-list and other lists used by route map and other filtering interfaces. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

yang: use filter types in route-map

04ef6e7

Import the new YANG model filter and use its types. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

yang: simplify filter choice by removing cases

bec0aa8

Based on @rwestphal feedback, lets remove `case`s where we don't expect to add more items or items with more than one `leaf`. Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

rzalamena force-pushed the route-map-yang branch from 11bc133 to bec0aa8 Compare August 1, 2019 22:59

rzalamena marked this pull request as ready for review August 2, 2019 19:54

riw777 approved these changes Aug 6, 2019

View reviewed changes

riw777 merged commit 112fed9 into FRRouting:master Aug 6, 2019

rzalamena deleted the route-map-yang branch October 9, 2019 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

yang: models for route map and filters #4733

yang: models for route map and filters #4733

rzalamena commented Jul 25, 2019

LabN-CI commented Jul 25, 2019

NetDEF-CI commented Jul 26, 2019

riw777 commented Jul 30, 2019

rzalamena commented Jul 30, 2019

donaldsharp commented Jul 30, 2019

rwestphal left a comment

rwestphal Aug 1, 2019

rwestphal Aug 1, 2019

rzalamena Aug 1, 2019

rwestphal Aug 1, 2019

rzalamena Aug 1, 2019

LabN-CI commented Aug 1, 2019

NetDEF-CI commented Aug 2, 2019

riw777 left a comment

yang: models for route map and filters #4733

yang: models for route map and filters #4733

Conversation

rzalamena commented Jul 25, 2019

Requirement

Summary

Components

LabN-CI commented Jul 25, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Jul 26, 2019

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

riw777 commented Jul 30, 2019

rzalamena commented Jul 30, 2019

donaldsharp commented Jul 30, 2019

rwestphal left a comment

Choose a reason for hiding this comment

rwestphal Aug 1, 2019

Choose a reason for hiding this comment

rwestphal Aug 1, 2019

Choose a reason for hiding this comment

rzalamena Aug 1, 2019

Choose a reason for hiding this comment

rwestphal Aug 1, 2019

Choose a reason for hiding this comment

rzalamena Aug 1, 2019

Choose a reason for hiding this comment

LabN-CI commented Aug 1, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Aug 2, 2019

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

riw777 left a comment

Choose a reason for hiding this comment