isisd: added prefix propogation/leaking #15863
Conversation
added: 1) default prefix propogation from level-1 to level-2 db on level_1_2 edge according to RFC 1195 2) cli command "redistribute ipv4/ipv6 isis level-m into level-n route-map map_name" 3) maneged leaking from level-2 into level-1 accoding to RFC 5302 4) Re-advertisement Flag in SR subtlv in redistributed prefix accoding to RFC 8667 Signed-off-by: squirrelking57 <[email protected]>
| @@ -2078,6 +2125,45 @@ module frr-isisd { | |||
| } | |||
| } | |||
|
|
|||
| container route_leaking { | |||
There was a problem hiding this comment.
Repeating my previous review - this is not needed. We added the level-from leaf to the redistribute container, specifically to cover all of this. Why do you think this separate container is needed?
There was a problem hiding this comment.
I think we need such a container because in callbacks, for example, we parse the young model, and if we don't make a separate container for local redistribution, then we will deal with the same callback that is already provided in ISIS:
return nb_cli_apply_changes(vty, "./route_leaking/%s[protocol='%s'][level='%s']", ip, proto, level_from);
will be the same like
return nb_cli_apply_changes( vty, "./redistribute/%s[protocol='%s'][level='%s']", ip, proto, level);
There was a problem hiding this comment.
I don't understand the example you provided, but that's exactly the point - why do we need any additional callbacks for that?
| @@ -1495,6 +1525,15 @@ module frr-isisd { | |||
| "IS-IS level into which the routes should be redistributed."; | |||
| } | |||
|
|
|||
| leaf level_from { | |||
There was a problem hiding this comment.
The common practice in YANG models is to use dashes (-), not underlines (_). Please, change this to level-from.
| type level; | ||
| must "(. != \"level-1-2\") and ((../../../is-type = \"level-1-2\") or (. = ../../../is-type))"; | ||
| description | ||
| "IS-IS level into which the routes should be redistributed."; | ||
| } | ||
|
|
||
| leaf level_from { |
| @@ -312,6 +322,27 @@ module frr-isisd { | |||
| uses redistribute-attributes; | |||
| } | |||
|
|
|||
| grouping leaking-attributes { | |||
There was a problem hiding this comment.
Regardless of the necessity of route_leaking container, this is not needed because it's a complete copy of grouping redistribute-attributes.
| @@ -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.
As in my previous review - there are no such levels in IS-IS and this change is wrong and unnecessary. That's exactly why we have two leafs - level and level-from.
|
@squirrelking57 any news on this pull-request ? |
|
Sorry for the long reply, I plan to fix all the requested edits in the near future, and I also found a problem with the metric calculation, which I will also fix |
added: