-
Notifications
You must be signed in to change notification settings - Fork 759
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
DS-Lite Support (WIP) #2382
DS-Lite Support (WIP) #2382
Conversation
|
Hi @noctarius, I would like to see if we can integrate this a little bit more loosely coupled, although I understand that most of the legacy code around dhcp looks a lot similar to this. Is there some documentation of the steps needed to get this working? We have quite some hooks in place to add custom features, it would be good if we can detach this more. Thanks for working on this, Best regards, Ad |
|
I think all the steps necessary are in the forum https://forum.opnsense.org/index.php?topic=7788.msg35863#msg35863. Anyhow I think it is valid to have it as an option for the IPv4 configuration of the interface. Obviously you need to do quiet a few things in the background, like retrieving the AFTR address via DHCPv6, the creation of a tunnel (and possibly re-creation), setting up the dynamic, default gateway (192.0.0.1) and some more things. I'd refrain from making it too complicated for users to activate / configure, as this is not an easy thing (especially the DHCP part), as well as tunnel configuration stuff. Happy to jump on a discussion though, hanging out in the IRC ;-) edit: maybe I misunderstand what you want to have loosely coupled though ;-) here's a script I'm using so far (with a patch to interfaces.inc to make call it after a dhcp prefix is retrieved): |
|
Here is a bit more of a timeline description: Tbh I don't think there is too much option since I actually need the response from DHCPv6 (using the new raw-option patch) to find an AFTR. I guess I'd be able to use the newwanip hook but I have to make sure the tunnel is created before the default gateway is set, since it'll fail otherwise (since 192.0.0.1 won't be available yet). As said, happy for any hint to make it nicer, but I doubt there's too much option, especially in the UI, I really really want it to be an option for IPv4 on WAN, everything else is too complicated / unintuitive :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first thoughts... nice work so far, thanks!
src/www/interfaces.php
Outdated
| @@ -2466,7 +2545,7 @@ function toggle_allcfg() { | |||
| </td> | |||
| </tr> | |||
| <tr class="dhcpv6_basic"> | |||
| <td><a id="help_for_dhcp6prefixonly" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Request only a IPv6 prefix"); ?></td> | |||
| <td><a id="help_for_dhcp6prefixonly" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Request only an IPv6 prefix"); ?></td> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably best merged in a different PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw it, the reason it is a separate commit, easy to extract and merge before ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very forward-thinking... cherry-picked via 932038d
| @@ -275,12 +275,28 @@ | |||
| endif; | |||
| if ($ifinfo['status'] != "down"): | |||
| if ($ifinfo['dhcplink'] != "down" && $ifinfo['pppoelink'] != "down" && $ifinfo['pptplink'] != "down"): | |||
| if ($ifinfo['ipaddr']):?> | |||
| if ($ifinfo['ipaddr']): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should split this up as well, not sure if this is the best approach to add conditionals where none existed before
| @@ -127,7 +127,13 @@ function interface_widget_update(sender, data) | |||
| <?=empty($ifinfo['media']) ? htmlspecialchars($ifinfo['cell_mode']) : htmlspecialchars($ifinfo['media']);?> | |||
| </td> | |||
| <td> | |||
| <?=htmlspecialchars($ifinfo['ipaddr']);?> | |||
| <? if ($ifinfo['ipaddr'] == "dslite") { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
| if (is_dslite_enabled($ifdescr)) { | ||
| $gifif = dslite_find_tunnel_interface($ifdescr); | ||
| $aftr = dslite_find_aftr_address($ifdescr); | ||
| $ifinfo['ipaddr'] = 'dslite'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the tunnel does not have a local ipv4 tunnel address?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is always 192.0.0.2 -> 192.0.0.1 as per spec, therefore the ip doesn't really make sense to be shown. You share a public IPv4 with multiple customers using the AFTR CNATing.
src/etc/rc.newwanipv6
Outdated
| // If on DS-Lite, try to find a valid / usable AFTR address (either from DHCPv6 or manually configured) | ||
| if (is_dslite_enabled($interface)) { | ||
| // Try to read dhcp option 64 (AFTR name) from the environment | ||
| $aftr_addr = trim(getenv("raw_dhcp_option_64")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very cool solution :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just extended the raw option patch which is already around ;-)
|
|
||
| require_once("interfaces.inc"); | ||
| require_once("util.inc"); | ||
| require_once("system.inc"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cyclic dependencies
ok for merge, but it would be less confusing to move these into interfaces.inc and use a consistent interfaces_dslite_ prefix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure can do, wasn't quite sure since few things (like gateway stuff) has its own files, others don't.
| log_error(sprintf("Starting DS-Lite tunnel with AFTR '%s' on interface %s with external IP '%s'", $aftr, $parent, $ip)); | ||
| } | ||
|
|
||
| $gifif = legacy_interface_create('gif'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this generates a gif interface with an arbitrary index. better rename the interface to be able to guess the name later when we need it, e.g. dslite_wan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which is ok, I save the gif name into the state file, as with multiple wan interfaces you might have multiple dslite tunnels ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, wan is the unique internal identifier... the others are lan and opt0 - opn(n-1)
between saving state to a file and avoiding it it's always better to avoid if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough, but there's no "wan" anywhere hardcoded ;-) it'll just work with any $interface :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's $parent here as fas as I can see
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh right, for any $parent 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what I could do is build something like "parentname_dslite", whatya think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd lead with dslite_ to make it easier to spot (like a "driver" name), but both are fine, since we don't need to track them in the config.xml and can always modify later if needed :)
src/etc/inc/interfaces.inc
Outdated
| @@ -2357,6 +2357,87 @@ function interface_virtual_create($interface) | |||
| } | |||
| } | |||
|
|
|||
| function interface_dslite_configure($verbose, $parent = 'wan', $aftr) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your copyrights to the file, obviously
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/etc/inc/interfaces.inc
Outdated
| $gifif = legacy_interface_create('gif', "{$parent}_dslite"); | ||
|
|
||
| $pmtu = ''; | ||
| if (!empty($mtu)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to move/check !empty($config['interfaces'][$parent]['dslite_tunnel_mtu']) over here and use it when not empty, in the sprintf(), so the $wanconfig and $mtu can be dropped.
src/etc/inc/interfaces.inc
Outdated
| $gifif = legacy_interface_create('gif', "{$parent}_dslite"); | ||
|
|
||
| $pmtu = ''; | ||
| if (!empty($mtu)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to check !empty($config['interfaces'][$parent]['dslite_tunnel_mtu']) over here and use the contents in the sprintf() when that's the case. $wanconfig and $mtu can be dropped then
src/etc/inc/interfaces.inc
Outdated
| @file_put_contents($dslite_tunnel_file, $gifif); | ||
|
|
||
| $dslite_aftr_file = sprintf("/tmp/dslite_%s_aftr.state", $parent); | ||
| @file_put_contents($dslite_aftr_file, $aftr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity, can't we read the aftr tag back from ifconfig? It's usually a bit more resilient to check ifconfig directly then to use temporary files. If I understand the workflow correctly, the gif interface should always have a predictable name now, so if it's there, we can validate it. right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be possible to read it back from ifconfig as long as there is a connection. If the tunnel drops for any reason I'm not sure the AFTR address would still be readable. I think a safe fallback would be to try to read it from ifconfig and if not available recreate the tunnel using the AFTR address in the file. Not everything I want to implement is done yet, for example tunnel monitoring and possibly recreating it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way we can check if the address is and stays in ifconfig? If it does, I can add the option to legacy_interfaces_details() so we can always stick to the interface and prevent files from getting async.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, my AFTR connection seems to be pretty stable. Normally when a reconnection (in the Fritzbox) happens you get a new external IP address (which is still shared by multiple people but is "new" to you). I didn't have this situation yet, at least not that I realized. I think setting up the gateway monitoring (for 192.0.0.1) might give some hint if we actually need to recreate the interface at any time. Whenever I apply changes to the/a wan interface, I should get a new DHCPv6 request, shouldn't I?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect so, yes. If you can send me an example ifconfig -m output of the interface, I can see if we can fit the aftr tag in legacy_interfaces_details() so you can fetch it from there. Saves temp files :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing nothing easier than that, however I'd imagine it is already available, since the 4in6 is most probably just a gif tunnel :-)
Anyhow here it is:
gif0: flags=8051<UP,POINTOPOINT,RUNNING,MULTICAST> metric 0 mtu 1460
options=80000<LINKSTATE>
capabilities=80000<LINKSTATE>
tunnel inet6 2a02:908::b863 --> 2a02:908::13:4000
inet 192.0.0.2 --> 192.0.0.1 netmask 0xfffffff8
nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
groups: gif
Just changed a few bits in my IPv6 ;-) and the 2a02:908::13:4000 is the AFTR address in my case. I guess you might not be able to reach / ping that address though. As far as I know, it's only reachable from the inside of the Unitymedia network.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, the tunnel wasn't parsed yet, with 0ac2af1 you can easily extract it from legacy_interfaces_details()
[tunnel] => Array ( [proto] => inet6 [src_addr] => 2a02:908:0000:0000:0000:0000:b863 [dest_addr] => 2a02:908::13:4000 )
src/etc/inc/interfaces.inc
Outdated
| @unlink($dslite_aftr_file); | ||
| } | ||
|
|
||
| function interface_dslite_find_tunnel_interface($parent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the interface is static, it's probably better to ditch this and refer to the static value.
src/etc/inc/interfaces.inc
Outdated
| } | ||
|
|
||
| $gifif = interface_dslite_find_tunnel_interface($parent); | ||
| if (!empty($gifif)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could probably check for existence of the interface using !empty(legacy_interfaces_details("{$parent}_dslite"))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After ditching find_tunnel, doesn't it make more sense to do a if (does_interface_exist($gifif)) { ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're probably right, both do an ifconfig call.
src/etc/rc.newwanipv6
Outdated
| log_error("Got AFTR from DHCPv6 response: {$aftr_addr}"); | ||
| } | ||
|
|
||
| if (!empty(trim($config['interfaces'][$interface]['dslite_aftr_addr']))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if dslite_aftr_addr takes precedence over getenv("raw_dhcp_option_64") it's better to make it explicit in the call flow. if dslite_aftr_addr then use this, else something like.
$aftr_addr = !empty(trim(getenv("raw_dhcp_option_64"))) ? pack("H*", trim(getenv("raw_dhcp_option_64"))) : null;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually think it's pretty ugly to use the same code over and over again like the getenv( .... ) but well, you asked for it. Wait for typos :D
|
@noctarius I missed the forum posting, but thanks for explaining. Your point is clear about intuitiveness, we should indeed try to integrate it in the interface settings. I left some remarks in the code which might help to simplify things a bit further. |
|
You're welcome. I'm using the very hacky shellscript for now which works™ ;-) But I'd be better in the system as more and more people will have the DS-Lite setup. Especially with the freedom of selecting your own modem/router/hardware in Germany (Europe?!?). Fritzboxen shouldn't be the only ones (apart from the hacky implementation in openwrt) to support DS-Lite :D |
|
PS: this all depends on the new raw-option patch for dhcp6c, just to remember :) |
src/etc/inc/gwlb.inc
Outdated
| @@ -5,6 +5,7 @@ | |||
| Copyright (C) 2008 Bill Marquette <bill.marquette@gmail.com> | |||
| Copyright (C) 2008 Seth Mos <seth.mos@dds.nl> | |||
| Copyright (C) 2010 Ermal Luçi | |||
| Copyright (C) 2018 Christoph Engelbert <me@noctarius.com> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree with this one as a means of substantial copyrightable changes. interfaces.inc and interfaces.php are ok though because I expected that copyright to merely shift from dslite.inc to interfaces.inc when asking for the merge and noting now the copyright was missing.
src/etc/inc/interfaces.inc
Outdated
|
|
||
| $mtu = ''; | ||
| if (!empty($config['interfaces'][$parent]['dslite_tunnel_mtu'])) { | ||
| $mtu = sprintf('mtu %s', $config['interfaces'][$parent]['dslite_tunnel_mtu']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be:
$mtu = exec_safe('mtu %s ', $config['interfaces'][$parent]['dslite_tunnel_mtu']);
src/etc/inc/interfaces.inc
Outdated
| $mtu = sprintf('mtu %s', $config['interfaces'][$parent]['dslite_tunnel_mtu']); | ||
| } | ||
|
|
||
| mwexecf('/sbin/ifconfig %s inet6 tunnel %s %s %s -accept_rtadv ifdisabled', array($gifif, $ip, $aftr, $mtu)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here $mtu will be escaped as a word instead of two parsable arguments for ifconfig, so that doesn't work. Use this instead:
mwexecf("/sbin/ifconfig %s inet6 tunnel %s %s {$mtu} -accept_rtadv ifdisabled", array($gifif, $ip, $aftr));
(cherry picked from commit 0ac2af1)
(cherry picked from commit 0ac2af1)
…l as adding a dynamic gateway
…s widget and page.
…ed DHCPv6 AFTR option response back to a string. Related commit for dhcp6c is noctarius/dhcp6c@d3d0585
5319ad5
to
50d7b82
Compare
|
Lots of conflicts after over a year, need to redo this. |
This is still WIP but I'd like to share it already to get feedback as early as possible, since C is a looooong time back and PHP too (apart from that fact it's not exactly my favorite language ;-)).
This patch series adds support for DS-Lite (https://tools.ietf.org/html/rfc6333) which is one of the most commonly used transition techniques for IPv6-only provider networks but offering IPv4 access for customers using a so-called CNAT (carrier grade NAT). The NAT capability is provided by the provider given an IPIP 4in6 tunnel to a remote endpoint called AFTR.
The AFTR address can either be configured manually or is discovered using DHCPv6 option 64 (using the new raw-option support in dhcp6c, opnsense/dhcp6c#1) as described in https://tools.ietf.org/html/rfc6334.
Looking forward to feedback.