WIP: GRE tunnel plugin#3554
Conversation
|
Could we use |
No idea, never tested it. Will try it out |
ipspace
force-pushed
the
feature/tunnel.gre
branch
from
c286b38 to
ef0986f
Compare
ipspace
force-pushed
the
feature/tunnel.gre
branch
from
6dc2921 to
a5833ef
Compare
The plugin implements GRE tunnel logic, including: * Tunnel attributes * Setting tunnel type on links with tunnel attributes * Figuring out tunnel source and destination IP addresses Shared code is implemented in a utility module, potentially optimizing other tunnel implementations. Included in the PR: * Plugin, attributes, utility functions * Coverage transformation test * GRE and GRE-in-VRF integration tests * Cisco IOS implementation * Documentation
ipspace
force-pushed
the
feature/tunnel.gre
branch
from
a5833ef to
45392ba
Compare
|
@jbemmel, @DanPartelly -- The tunnel.gre plugin is ready. Based on @jbemmel suggestion, I changed the source attribute names to tunnel.source.link.something. Will figure out how to put 'tunnel.gre' plugin in 'tunnel/gre' directory as a separate PR. |
| link: | ||
| tunnel: | ||
| af: # Transport address family | ||
| { type: str, valid_values: [ ipv4, ipv6] } |
There was a problem hiding this comment.
Dang, missed it before merging. Maybe we have to update yamllint defaults ;)
| # Second pass -- after computing tunnel source for all GRE interfaces, set tunnel destinations | ||
| # | ||
| for node in node_iflist: # Process only nodes with tunnel interfaces | ||
| ndata = topology.nodes[node] # Get node data (we might need it) |
There was a problem hiding this comment.
As I said, we might need it ;) Interestingly, none of the code checking tools complain about setting a variable value and not using it afterwards; they only complain about unused variable.
| continue | ||
|
|
||
| if t_name and t_name != intf.get('name',None): | ||
| continue # Mismatch in interface name |
There was a problem hiding this comment.
link name - that's the confusing part I noted earlier
There was a problem hiding this comment.
Yeah, of course I agree, but that ship has sailed years ago...
There was a problem hiding this comment.
The comment should say "Mismatch in link name"
There was a problem hiding this comment.
IIRC I tested and you can specify 'name' on an individual interface, which would make this exactly what it says
| if t_af not in intf: # Is the interface enabled for the desired tunnel transport AF | ||
| continue | ||
|
|
||
| if not isinstance(intf[t_af],str): # Cannot run tunnels from unnumbered interfaces |
There was a problem hiding this comment.
Should generate a (suppressible) warning?
There was a problem hiding this comment.
Let's see how this evolves. We could either generate errors in here or in the caller function. We could be more precise here, but would have more context in the caller.
jbemmel
left a comment
jbemmel
left a comment
There was a problem hiding this comment.
Some minor tweaks, I think we will find that much of the code is applicable to other tunnel types as well but we can refactor as we go
Already done:
Still missing: