Skip to content
This repository has been archived by the owner on Sep 17, 2019. It is now read-only.

Enhance get_route_to #174

Open
mirceaulinic opened this issue Dec 23, 2016 · 13 comments
Open

Enhance get_route_to #174

mirceaulinic opened this issue Dec 23, 2016 · 13 comments

Comments

@mirceaulinic
Copy link
Member

Two proposed additions:

  • arg vrf
  • next_hop_type key inside the output dict
@dbarrosop
Copy link
Member

Remind me, what is next_hop_type?

@mirceaulinic
Copy link
Member Author

As the name states, it is the type of the next hop. E.g. NULL routes have the next hop type "discard". Also, the type of the next hop can be an interface etc.

@dbarrosop
Copy link
Member

dbarrosop commented Jun 3, 2017

To me it seems we are encoding arbitrary information but maybe I am misunderstanding it.

NULL routes have the next hop type "discard"

Examples from different platforms?

the type of the next hop can be an interface

Is the type "interface" or an interface? If it's the latter this is clearly the wrong construct. "Ethernet1" is not a type, it's an interface. If you are using arbitrary interface names as type, how can I distinguish that the next hop is of type interface?

To me it seems more natural to have next_hop_interface and next_hop. So I can do:

# discard route
next_hop_interface: null
next_hop: None

# regular route with no interface
next_hop_interface: None
next_hop: 1.1.1.1

# next_hop and interface
next_hop_interface: "et1"
next_hop: 1.1.1.1

@mirceaulinic
Copy link
Member Author

Indeed, having more specific names rather than a generic next_hop_type would be more intuitive.
Let me think a bit about other types of fields, so if we are applying this API change right now we can introduce few more fields that have explicit next_hop_<type>: <name of the next hope type>.

But how would you represent: next_hop_interface: null vs. next_hop_interface: None and distinguish between them?
Perhaps having a field like null_route: True better, or something similar?

@dbarrosop
Copy link
Member

I don't think having a field null_route is necessary as that's a use case of a particular configuration.

But how would you represent: next_hop_interface: null vs. next_hop_interface: None and distinguish between them?

Sorry, the "data" on my previous comment had a "bug". null is not really an object but the string "null".

@sincerywaing
Copy link

I recently got a similar use case so circirling back on this one. I mean for junos it's fine we can use key to filter table name if we want specific vrf. But for ios it seems we don't even have this api imported. Any reason? Also for ios, we most use vrf kw or we won't get table in specific vrf. adding @ktbyers to further comment.

@mirceaulinic
Copy link
Member Author

Sorry, I dropped the ball here, I will try to revisit this issue during the week of 17th.

But for ios it seems we don't even have this api imported. Any reason?

Yes, the reasoning is that nobody submitted any code to add this method. I know you have the skills required, so please go ahead and provide an implementation, if you're willing to @sincerywaing :-)

@sincerywaing
Copy link

I guess the difficult part for ios is that you need parse everything!@mirceaulinic I'll start looking into this :)

@sincerywaing
Copy link

sincerywaing commented Jul 10, 2017

I spent some time looking into this and found it is really complicated to list every attribute for ios -

def get_route_to(self, vrf='', destination=''):
    """
            Returns a dictionary of dictionaries containing details of all available routes to a
            destination.
            :param destination: The destination prefix to be used when filtering the routes.
            :param vrf (optional): Retrieve the routes only for a specific protocol.
            Each inner dictionary contains the following fields:
                * protocol (string)
                * age (string)
                * next_hop (string)
                * routing_table (string)
                * distance (string)
                * metric (string)
    """
    if vrf = '':
        command = 'show ip route ' + destination
    elif:
        command = 'show ip route vrf ' + vrf + ' ' + destination
    output = self.device.send_command(command)
    re_dict = {'protocol': r'Known\svia\s\"(.+?)\"', 'metric': r'metric\s(\d+)', 'distance': r'distance\s(\d+)', 'next_hop':  r'(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})\,\sfrom\s(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})', 'age': r'([\w:]+)\sago', 'routing_table': r'Routing\sTable\:\s([\w:]+)'}
    for k, v in re_dict.iteritems():
        pat = re.compile(v)
        re_dict[k] = pat.search(output).group(1).split(' ')[0]
    return re_dict

e.g. no interface shown here

Routing Table: test
Routing entry for 189.222.33.0/28
  Known via "bgp 65777", distance 20, metric 0
  Tag 65777, type external
  Redistributing via ospf 20
  Advertised by ospf 20 metric-type 1 subnets route-map foh-bgp-to-ospf
  Last update from 56.44.3.3 13:23:13 ago
  Routing Descriptor Blocks:
  * 56.44.3.3, from 56.44.3.3, 13:23:13 ago
      Route metric is 0, traffic share count is 1
      AS Hops 2
      Route tag 65777
      MPLS label: none

{'distance': '20', 'metric': '0', 'next_hop': '56.44.3.3', 'protocol': 'bgp', 'age': '13:23:13', 'routing_table': 'test'}
        * protocol (string) - Supported
        * current_active (True/False) - N/A
        * last_active (True/False)  - N/A
        * age (int) - supported (**string**)
        * next_hop (string) - supported
        * outgoing_interface (string) - N/A, not displayed here but displayed in below show .
        * selected_next_hop (True/False) - N/A
        * preference (int) - N/A
        * inactive_reason (string) - N/A
        * routing_table (string) - Supported
        * protocol_attributes (dictionary) - N/A

However when there is load balancing and interface, from same device but different protocol

Routing Table: test
Routing entry for 189.222.33.0/28
  Known via "ospf 10", distance 110, metric 0, type extern 2, forward metric 10
  Redistributing via bgp 65777
  Advertised by bgp 65666 route-map ospf-to-bgp
  Last update from 45.32.3.2 on TenGigabitEthernet1/0/0.405, 7w0d ago
  Routing Descriptor Blocks:
  * 45.32.3.2, from 65.5.5.5, 7w0d ago, via TenGigabitEthernet1/0/0.405
      Route metric is 0, traffic share count is 1
    45.32.3.3, from 65.5.5.6, 7w0d ago, via TenGigabitEthernet1/0/0.405
      Route metric is 0, traffic share count is 1
{'distance': '110', 'metric': '0', 'next_hop': '45.32.3.2', 'protocol': 'ospf', 'age': '7w0d', 'routing_table': 'test'}
  1. only the current best * one is selected, or next_hop would be a list
  2. age is with the last updated one

@sincerywaing
Copy link

sincerywaing commented Jul 10, 2017

so a better implementation would be separate the output to two parts -

Part I start from beginning to 'last update' to parse protocol, distance, metric, routing_table;

Part II strat from 'Routing descriptor blocks' to the end to return a list of dict including next_hop, age, interface(if any) and protocol_spec arguments.

Finally it will look like -

    * protocol (string)
    * routing_table (string)
    * distance (string)
    * protocol_attributes (list of dictionary, each item in the list contains): 
             - next_hop (string): 
             - outgoing_interface (string)
             - metric (string)
             - protocol_specifics (dictionary) 

Thoughts? If we are comfortable with this proposal, I'll start look into this.

@dbarrosop
Copy link
Member

@mirceaulinic what's your take on this? I think doing backwards compatible changes like the one you suggested is fine but I don't think we should do backwards incompatible changes as @sincerywaing is suggesting.

I am not suggesting his changes don't make sense (which they might) but I think that if we are planning to make backwards incompatible changes we should:

  1. Use a model from the openconfig or the ietf communities, not something designed by us. Unless the openconfig/ietf fails to deliver on time, of course.
  2. Use napalm-yang instead of a getter.

@mirceaulinic
Copy link
Member Author

mirceaulinic commented Jul 22, 2017

Use a model from the openconfig or the ietf communities, not something designed by us.

This is a sound idea, but I wasn't able to identify if there's any model available yet for the routes state. But sure, that's the long term solid solution.

For the short term, given that the method already exists, and @sincerywaing's question is specifically about IOS, we can implement this one easily.
I have two comments: we should fill in the fields as much as possible. If the information is not exposed at all by the device, that field will be empty (depending on the case, e.g. empty string for int, 0 or -1 for int etc.). This is also the case with EOS where, IIRC, there are two empty fields.

Some fields can be returned though:

current_active (True/False) - N/A

This information is available -- the * tells you that.

outgoing_interface (string) - N/A, not displayed here but displayed in below show .

No problem: as long as the device is able to give you this information, we can execute another request to retrieve this data.

protocol_attributes (dictionary) - N/A

This is another request, depending on the protocol.

Thoughts?

@dbarrosop
Copy link
Member

Ok, somehow I misunderstood @sincerywaing and I thought he was suggesting changing the current model.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants