-
Notifications
You must be signed in to change notification settings - Fork 1.3k
added managed redistribution in ISIS #15617
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
Conversation
e669fe5
to
aebd626
Compare
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.
This is a good addition to FRR, but the code must be improved. I didn't look into the actual redistribution implementation yet, but I have some comments from the configuration point of view. Please, take a look.
isisd/isis_cli.c
Outdated
@@ -85,13 +85,13 @@ void cli_show_router_isis(struct vty *vty, const struct lyd_node *dnode, | |||
{ | |||
const char *vrf = NULL; | |||
|
|||
vrf = yang_dnode_get_string(dnode, "vrf"); | |||
vrf = yang_dnode_get_string(dnode, "./vrf"); |
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.
Please, remove all these unrelated changes.
isisd/isis_cli.c
Outdated
/* | ||
* XPath: /frr-isisd:isis/instance/route_leanking | ||
*/ | ||
DEFPY_YANG(isis_leanking, isis_leanking_cmd, |
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.
First, this should be leaking
, not leaNking
. This needs to be fixed everywhere in the code.
Second, I think it should be a part of the regular redistribute
command. For example, that's how you configure this on Cisco:
redistribute isis ip level-2 into level-1 route-map r1
IMO we should just use the same command, there's no need to introduce new commands and new terminology.
isisd/isis_redist.c
Outdated
@@ -585,6 +690,85 @@ void isis_redist_area_finish(struct isis_area *area) | |||
} | |||
|
|||
#ifdef FABRICD | |||
DEFUN(isis_leanking, isis_leanking_cmd, |
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.
Same comments about the command here.
@@ -104,6 +104,16 @@ module frr-isisd { | |||
description | |||
"This enum indicates capability for both levels."; | |||
} | |||
enum "level2_to_level1" { |
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.
There are no such levels in IS-IS, it shouldn't be modeled like this. More comments below.
yang/frr-isisd.yang
Outdated
@@ -2078,6 +2109,45 @@ module frr-isisd { | |||
} | |||
} | |||
|
|||
container route_leanking { |
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.
We don't need this new container to model IS-IS to IS-IS redistribution. It is enough to modify the existing redistribute
container:
- remove
must ". != \"isis\"";
fromleaf protocol
to allow redistribution from IS-IS - add a new
from-level
leaf to configure from which level to redistribute:leaf from-level { type level; when "../protocol = \"isis\""; mandatory true; must "(. != \"level-1-2\") and ((../../../is-type = \"level-1-2\") and (. != ../level))"; description "IS-IS level from which the routes should be redistributed."; }
These two changes would be enough to model what you need without introducing a lot of new NB callbacks.
bb9750a
to
8cfa3b5
Compare
9aabe4a
to
4b6cff3
Compare
5050c31
to
78a577c
Compare
1) added command "redistribute ipv4/ipv6 isis level-m into level-n route-map name" in cli according to RFC5302 2) added adding prefixes with the best metric from the level-1 db to the level-2 db on l1_l2 end system Signed-off-by: squirrelking57 <1248756005hfh@gmail.com>
There is no need for a review at the moment, it needs improvement |
created a new beautiful and good pull request #15863 |
added command "route_leaking ipv4/ipv6 level's route-map name" in cli according to RFC5302