Skip to content

Refactor initial tunnel implementation#3576

Merged
ipspace merged 4 commits into
devfrom
tunnel-refactor
Jul 4, 2026
Merged

Refactor initial tunnel implementation#3576
ipspace merged 4 commits into
devfrom
tunnel-refactor

Conversation

@ipspace

@ipspace ipspace commented Jul 4, 2026

Copy link
Copy Markdown
Owner
  • Move utils/tunnel.py to extra/tunnel/init.py
  • Return interface list from 'select source interface' function, making if useful in environments with additional requirements
  • Make tunnel.af check optional to support dual-stack-capable tunnels
  • Move most of GRE processing to _p2p.py utility function, assuming it can be used (or adapted) to other tunnel implementations.

Also:

  • Move all link-related utility functions to augment/links and move them to the top of that module

* Move utils/tunnel.py to extra/tunnel/__init__.py
* Return interface list from 'select source interface' function, making
  if useful in environments with additional requirements
* Make tunnel.af check optional to support dual-stack-capable tunnels
* Move most of GRE processing to _p2p.py utility function, assuming it
  can be used (or adapted) to other tunnel implementations.

Also:

* Move all link-related utility functions to augment/links and move
  them to the top of that module

This comment was marked as resolved.

This comment was marked as resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread netsim/augment/links.py
Comment thread netsim/extra/tunnel/gre/__init__.py Outdated
@ipspace ipspace requested a review from jbemmel July 4, 2026 14:14
@ipspace

ipspace commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

@jbemmel -- Looking at your wireguard code, there was a lot of duplication with the GRE code, so I refactored the whole thing and created a bunch of (hopefully useful) _p2p functions.

Also, the "select source interface" now returns a list of viable interfaces (and ignores AF if it's not set) which you can then use to do smarter interface selection.

Comment thread netsim/extra/tunnel/__init__.py

@jbemmel jbemmel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no comment

@ipspace ipspace merged commit 3da5337 into dev Jul 4, 2026
11 checks passed
@ipspace ipspace deleted the tunnel-refactor branch July 4, 2026 15:12
if 'name' in intf and '->' in intf.name and 'GRE' not in intf.name:
intf.name += ' [GRE tunnel]'
node_iflist = _p2p.feature_check(topology,t_mode='gre',t_desc='GRE tunnels')
_p2p.tunnel_source(topology,node_iflist,default_af='ipv4',t_name='GRE')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The current code requires the user to explicitly specify tunnel.af: ipv6 in an IPv6-only setup. I would suggest we infer tunnel.af from the available address families on the selected interface, preferring ipv4 if both are available. This behavior should be consistent across tunnel plugins

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

You know that uneasy feeling I had when looking at #3541?

I get a similar feeling considering auto-AF for GRE tunnels. I can't tell you what could break, but I'll stay with the current "ipv4 is the default" behavior.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I get a similar feeling considering auto-AF for GRE tunnels. I can't tell you what could break, but I'll stay with the current "ipv4 is the default" behavior.

I have no problem with that, just thinking that we might want to do that like this?

attributes:
  link:
    tunnel:
      af:                     # Transport address family
        { type: str, valid_values: [ ipv4, ipv6 ], _default: ipv4 }

rather than bury it inside Python logic?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

And what would happen when someone uses tunnel.gre and tunnel.wireguard in the same topology?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nothing, this section would be under GRE not affecting Wireguard

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Are you absolutely sure about that? Want to try it out first?

snuffy22 pushed a commit to snuffy22/netlab that referenced this pull request Jul 9, 2026
* Move utils/tunnel.py to extra/tunnel/__init__.py
* Return interface list from 'select source interface' function, making
  if useful in environments with additional requirements
* Make tunnel.af check optional to support dual-stack-capable tunnels
* Move most of GRE processing to _p2p.py utility function, assuming it
  can be used (or adapted) to other tunnel implementations.

Also:

* Move all link-related utility functions to augment/links and move
  them to the top of that module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants