Feature #10392: GRE: Tunnels cannot have IPv6 and IPv4 addresses …

[xrm]

…at the same time. This PR tries to change that.

• Redmine Issue: https://redmine.pfsense.org/issues/10392
• Ready for review

This commit (only) patches the GRE part. GIF interfaces have to be patched separately; and while that patch should mostly consist of directly applying this commit to the gif-file/functions as well, I have no way of testing it.

Patch was tested on 2.4.4-RELEASE-p3. Since I did not come across any issues when merging this back onto master, I expect this to work on the latest master as well.

First commit to the pfSense project ever, so I'm neither sure if the "fallback mode for older configurations" is the right way to do it not if the code fulfils the quality standard in all points.

[jim-p]

Rather than have any kind of fallback detection, the configuration should be upgraded directly to the new style. See the other functions in /etc/inc/upgrade_config.inc and so on.

Why make the user manually pick IPv4/IPv6? If they fill in just the v4 info, then it's v4. If they fill in just the v6 info, it's v6. If they fill in both, it's both. Input validation should allow one or the other to be empty (but not both).

[xrm]

Hi jim-p,

I removed the selection box and added a possible upgrade code to upgrade_config.php. I tested the new code (except for the upgrade_config.php-part since I don't know how) on 2.4.5.

There is one small issue, though: if using IPv4 and IPv6 at the same time, the newly added IPv6 gateway shows the buttons to delete and to edit it - something the IPv4 gateway does not. It doesn not seem to me as if this is related to my changes but I wanted to mention it.

Thanks
Sebastian

[jim-p]

One more thing: You need to increase the value of latest_config in /etc/inc/globals.inc to match the new upgrade code version.

To test that, you can start the PHP shell (pfSsh.php) and run convert_config(); followed by exec after updating globals.inc.

[xrm]

The original code outputs gre['remote-network'] which is not defined. Since the user put in the network themselves anyway (and should therefore know it without the error message telling them), I decided to just output which of the two possibly entered networks (v4/v6) already exists.

[xrm]

Thanks for the detailled feedback. I revised the complete wording on the GRE edit page and it should (hopefully) be more consistent now. I also used the opportunity to improve a few minor things such as limiting the IP-input fields to v4 and v6, respectively.

The link1-variable still confuses me a bit but I changed it as proposed.

[vktg]

works as expected, converting old-style GRE interfaces to new is OK

# ifconfig gre0
gre0: flags=8051<UP,POINTOPOINT,RUNNING,MULTICAST> metric 0 mtu 1476
	description: GREd
	options=80000<LINKSTATE>
	tunnel inet 192.168.3.4 --> 192.168.3.6
	inet 192.168.46.4 --> 192.168.46.6 netmask 0xffffff00
	inet6 fe80::edd:aeff:fe8a:1d00%gre0 prefixlen 64 scopeid 0xd
	inet6 fc00:46::4 --> fc00:46::6 prefixlen 128
	groups: gre
	nd6 options=21<PERFORMNUD,AUTO_LINKLOCAL>
# ping6 fc00:46::6
PING6(56=40+8+8 bytes) fc00:46::4 --> fc00:46::6
16 bytes from fc00:46::6, icmp_seq=43 hlim=64 time=1.021 ms
16 bytes from fc00:46::6, icmp_seq=44 hlim=64 time=0.944 ms
16 bytes from fc00:46::6, icmp_seq=45 hlim=64 time=7.456 ms
16 bytes from fc00:46::6, icmp_seq=46 hlim=64 time=1.055 ms
16 bytes from fc00:46::6, icmp_seq=47 hlim=64 time=1.038 ms
^C
--- fc00:46::6 ping6 statistics ---
48 packets transmitted, 5 packets received, 89.6% packet loss
round-trip min/avg/max/std-dev = 0.944/2.303/7.456/2.577 ms
# ping 192.168.3.6
PING 192.168.3.6 (192.168.3.6): 56 data bytes
64 bytes from 192.168.3.6: icmp_seq=0 ttl=64 time=0.893 ms
64 bytes from 192.168.3.6: icmp_seq=1 ttl=64 time=1.005 ms

It's better to set IPv6 subnet field inactive and add note,
otherwise 'always 128' subnet can confuse users

something like:

))->setHelp('The subnet is used for determining the IPv6 network needs to be 128 as enforced by kernel.')->setAttribute('disabled', true);

tested on two 2.5.0.a.20200512.2320

[xrm]

The subnet is used for the route, though.