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

Isis show route paths json #16182

Merged
merged 7 commits into from
Jun 25, 2024

Conversation

pguibert6WIND
Copy link
Member

Add json support for 'show isis route' , paths part.

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.

Ignoring (all other old fields not using this format also) camelCase for JSON, LGTM.

@ton31337
Copy link
Member

ton31337 commented Jun 7, 2024

Could you fix the tests also?

@pguibert6WIND pguibert6WIND marked this pull request as draft June 10, 2024 07:28
@pguibert6WIND pguibert6WIND force-pushed the isis_show_route_paths_json branch from 9a66d51 to 216b8af Compare June 13, 2024 15:36
@github-actions github-actions bot added size/XXL and removed size/L labels Jun 13, 2024
@pguibert6WIND pguibert6WIND force-pushed the isis_show_route_paths_json branch 5 times, most recently from a88e98c to 760b793 Compare June 18, 2024 12:33
@pguibert6WIND pguibert6WIND marked this pull request as ready for review June 18, 2024 12:36
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

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

looks good ... waiting on ci fixes

@vjardin
Copy link
Contributor

vjardin commented Jun 18, 2024

code is ok, please, @pguibert6WIND , can you fix the commit log's title of:
760b793
so it does not wrap.

@riw777
Copy link
Member

riw777 commented Jun 18, 2024

need to look at ietf yang model

@pguibert6WIND pguibert6WIND force-pushed the isis_show_route_paths_json branch from 760b793 to b4e3159 Compare June 18, 2024 20:01
@github-actions github-actions bot added the rebase PR needs rebase label Jun 18, 2024
@pguibert6WIND pguibert6WIND force-pushed the isis_show_route_paths_json branch from b4e3159 to 3fe7706 Compare June 18, 2024 20:03
@pguibert6WIND
Copy link
Member Author

code is ok, please, @pguibert6WIND , can you fix the commit log's title of: 760b793 so it does not wrap.

done

@pguibert6WIND
Copy link
Member Author

need to look at ietf yang model

Lets us see if CI passes.
For the naming convention, I need the caml support on table to be approved before going on.
so, high chance that ietf yang model will be on separate pull requests.

@pguibert6WIND
Copy link
Member Author

ci:rerun lets rerun

@pguibert6WIND
Copy link
Member Author

need to look at ietf yang model

Lets us see if CI passes. For the naming convention, I need the caml support on table to be approved before going on. so, high chance that ietf yang model will be on separate pull requests.

@riw777 , I found that, but it appears it does not seem to match our model at all.
looks like it should be a separate task, no ?

https://datatracker.ietf.org/doc/html/draft-ietf-isis-yang-isis-cfg

grouping route-content {
       description
         "IS-IS protocol-specific route properties grouping.";
       leaf metric {
         type uint32;
         description "IS-IS metric of a route.";
       }
       leaf-list tag {
         type uint64;
         description
           "List of tags associated with the route.
            This list provides a consolidated view of both
            32-bit and 64-bit tags ([RFC5130](https://datatracker.ietf.org/doc/html/rfc5130)) available for the prefix.";
       }
       leaf route-type {
         type enumeration {
           enum l2-intra-area {
             description "Level 2 internal route. As per [RFC5302](https://datatracker.ietf.org/doc/html/rfc5302),
                          the prefix is directly connected to the
                          advertising router. It cannot be
                          distinguished from an L1->L2 inter-area
                          route.";
           }
           enum l1-intra-area {
             description "Level 1 internal route. As per [RFC5302](https://datatracker.ietf.org/doc/html/rfc5302),
                          the prefix is directly connected to the
                          advertising router.";
           }
           enum l2-external {
             description "Level 2 external route. As per [RFC5302](https://datatracker.ietf.org/doc/html/rfc5302),
                          such a route is learned from other IGPs.
                          It cannot be distinguished from an L1->L2
                          inter-area external route.";
           }
           enum l1-external {



Litkowski, et al.        Expires April 17, 2020                [Page 44]


Internet-Draft                  isis-cfg                    October 2019


             description "Level 1 external route. As per [RFC5302](https://datatracker.ietf.org/doc/html/rfc5302),
                          such a route is learned from other IGPs.";
           }
           enum l1-inter-area {
             description "These prefixes are learned via L2 routing.";
           }
           enum l1-inter-area-external {
             description "These prefixes are learned via L2 routing
                          towards an l2-external route.";
           }
         }
         description "IS-IS route type.";
       }
     }

@pguibert6WIND pguibert6WIND force-pushed the isis_show_route_paths_json branch 2 times, most recently from e5c5be1 to 13d350d Compare June 19, 2024 20:19
@pguibert6WIND
Copy link
Member Author

ci:rerun

This is a preliminary commit, so that route paths are visible from json.

Before:
> IS-IS paths to level-1 routers that speak IPv6
> Vertex               Type         Metric Next-Hop             Interface Parent
> rt1
> 2001:db8:1000::1/128 IP6 internal 0                                     rt1(4)
> rt2                  TE-IS        10     rt2                  eth-rt2   rt1(4)
> rt3                  TE-IS        10     rt3                  eth-rt3   rt1(4)
> 2001:db8:1000::2/128 IP6 internal 20     rt2                  eth-rt2   rt2(4)
> 2001:db8:1000::3/128 IP6 internal 20     rt3                  eth-rt3   rt3(4)

After:
> Vertex                Type          Metric  Next-Hop  Interface  Parent
>  -------------------------------------------------------------------------
>  rt1
>  2001:db8:1000::1/128  IP6 internal  0                            rt1(4)
>  rt2                   TE-IS         10      rt2       eth-rt2    rt1(4)
>  rt3                   TE-IS         10      rt3       eth-rt3    rt1(4)
>  2001:db8:1000::2/128  IP6 internal  20      rt2       eth-rt2    rt2(4)
>  2001:db8:1000::3/128  IP6 internal  20      rt3       eth-rt3    rt3(4)

Signed-off-by: Philippe Guibert <[email protected]>
The 'show isis route json' command never displays the list of
paths. Add the json support for this sub-part.

> # show isis route json
> [..]
> "ipv6-paths":[
>  {
>   "Vertex":"rt1",
>   "Type":"",
>   "Metric":0,
>   "Next-Hop":"",
>   "Interface":"",
>   "Parent":""
>  },
>  {
>    "Vertex":"2001:db8:1000::1\/128",
>    "Type":"IP6 internal",
>    "Metric":0,
>    "Next-Hop":"",
>    "Interface":"",
>    "Parent":"rt1(4)"
>  },

Signed-off-by: Philippe Guibert <[email protected]>
Add the json keyword for dumping isis topology.

Signed-off-by: Philippe Guibert <[email protected]>
Add the json support from ISIS vty command.

Signed-off-by: Philippe Guibert <[email protected]>
… command

Add the json support from ISIS vty command.
> show isis vrf vrf1 topology json

Signed-off-by: Philippe Guibert <[email protected]>
The json output for isis route paths should use caml format.

Signed-off-by: Philippe Guibert <[email protected]>
The json format for json routes should be compliant with caml format.

Before:

> "Prefix|Metric|Interface|Nexthop|SID|LabelOp|Algo":
> "Prefix|Metric|Interface|Nexthop|Label(s)");

After:

> "prefix|metric|interface|nextHop|segmentIdentifier|labelOperation|Algorithm":
> "prefix|metric|interface|nextHop|label(s)");

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND pguibert6WIND force-pushed the isis_show_route_paths_json branch from 13d350d to 611f83f Compare June 21, 2024 13:40
@aceelindem
Copy link
Collaborator

need to look at ietf yang model

Lets us see if CI passes. For the naming convention, I need the caml support on table to be approved before going on. so, high chance that ietf yang model will be on separate pull requests.

With respect to this comment, the ietf-isis.yang model only includes read-only retrieval of the IS-IS local RIB. There is no collary for the SPF tree - refer to https://datatracker.ietf.org/doc/rfc9130/ (see grouping local-rib).

@pguibert6WIND
Copy link
Member Author

need to look at ietf yang model

Lets us see if CI passes. For the naming convention, I need the caml support on table to be approved before going on. so, high chance that ietf yang model will be on separate pull requests.

With respect to this comment, the ietf-isis.yang model only includes read-only retrieval of the IS-IS local RIB. There is no collary for the SPF tree - refer to https://datatracker.ietf.org/doc/rfc9130/ (see grouping local-rib).

Thanks @aceelindem for feedback.
I understand it was not matching because I was comparing a route entry with a SPF path entry.

This pull request has an ultimate commit that modified the route entry json format. (see [0]).
when I read local-rib, I have a doubt about the next-hop object. in caml format, do we keep next-hop or can we move to nextHop ?

[0] 611f83f

@riw777 riw777 merged commit cc3519f into FRRouting:master Jun 25, 2024
10 checks passed
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.

5 participants