Dhcp failover config generation buggy
-
I was just wondering, why both my primary and secondary pfsense boxes put "primary" into their dhcpd.conf
files. IMHO there's a bug in /etc/inc/services.inc (pfSense-Full-Update-2.1-RC2-amd64-20130904-2355.tgz):–- services.inc.orig 2013-09-05 13:23:20.000000000 +0200
+++ services.inc 2013-09-05 15:04:00.000000000 +0200
@@ -451,7 +451,7 @@
$a_vip = &$config['virtualip']['vip'];
if(is_array($a_vip)) {
foreach ($a_vip as $vipent) {
- if($int == $real_dhcpif) {
+ if (convert_friendly_interface_to_real_interface_name($vipent['interface']) == $real_dhcpif) {
/* this is the interface! */
if(is_numeric($vipent['advskew']) && ($vipent['advskew'] < "20"))
$skew = 0;with this fix, the secondary really does put "secondary" into dhcpd.conf, without, both configs
wind up as "primary" here, and won't go past "recovery" state.Cheers,
MarkusEDIT: my original fix was wrong, sorry. Don't know whether this is the best approach, but it seems to work.
-
Your diff is not entirely correct either.
I pushed a fix https://github.com/pfsense/pfsense/commit/25f9f3322863eae12f705137d8d414643d7b853f that should do the right thing.Can you confirm it fixes for you?
-
I changed my patch, sorry.
I'm not sure your fix would work here. I did some logging for troubleshooting, and the comparison that
is supposed to match looks like this here. I have to add that this specific firewall is running
2.1-RC0 (i386) built on Sun Jun 2, so in case something in the upstream code changed, your change
might work:php: : Array ( [0] => em1 [1] => Array ( [mode] => carp [interface] => lan [vhid] => 99 [advskew] => 100 [advbase] => 1 [password] => XXXX [descr] => MGMT [type] => single [subnet_bits] => 24 [subnet] => 10.99.1.1 ) [2] => em1 )
with elements being:
[0] $real_dhcpif
[1] $vipent
[2] $intso, the way I see it, guess_interface_from_ip($dhcpifconf['failover_peerip']) yields the physical interface, not
the CARP interface, so the $intip = find_interface_ip($int) will give the IP address of the physical interface
too. Your proposed fix compares $vipent['subnet'] == $intip, but that won't be the case because the VIP
IP is not the same as the IP of the physical interface.Cheers,
Markus -
Another potential issue: the isc dhcpd.conf manual says:
…
The mclt statementmclt seconds;
The mclt statement defines the Maximum Client Lead Time. It must be
specified on the primary, and may not be specified on the secondary.
.....
however, pfsense currently sets mclt on both the primary and secondary, but only
specifies the split value on the primary.Was this intentional, or did the two get mixed up?
-
I think the original intent may have been for guess_interface_from_ip() to return the actual vip interface with that specific IP.
--- interfaces.inc.orig Fri Sep 6 00:27:12 2013 +++ interfaces.inc Fri Sep 6 00:31:27 2013 @@ -4015,6 +4015,14 @@ /* create a route table we can search */ exec("netstat -rnWf inet", $output, $ret); foreach($output as $line) { + if(preg_match("/^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+[ ]+link[#]/", $line)) { + $fields = preg_split("/[ ]+/", $line); + if($ipaddress==$fields[0]) { + return $fields[6]; + } + } + } + foreach($output as $line) { if(preg_match("/^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+\/[0-9]+[ ]+link[#]/", $line)) { $fields = preg_split("/[ ]+/", $line); if(ip_in_subnet($ipaddress, $fields[0])) {
I didn't do anything to the IPv6 side.
-
I don't think guess_interface_from_ip() returning the VIP interface would work, since the carp
interface doesn't appear in the routing table on the BACKUP node, and even on the MASTER
node, the lookup is done on the failover_peerip, which is not the /32 of the MASTER:MASTER: 10.99.1.0/24 link#2 U 0 544689315 1500 em1 10.99.1.1 link#13 UH 0 0 1500 lan_vip99 10.99.1.2 link#2 UHS 0 0 16384 lo0 SLAVE: 10.99.1.0/24 link#2 U 0 2362 1500 em1 10.99.1.3 link#2 UHS 0 0 16384 lo0
Cheers,
-
The fixes made based on this thread were backed out because they broke previously-functional failover setups.
The code that is in place is correct for most setups, perhaps there is something else unique/incorrect about your configuration contributing to the detection failure, but we had some fallout from this while testing the -RELEASE images.
So the code is back to how it was before, and it is confirmed working on several very large CARP+DHCP Failover setups. If it does not work for you on 2.1-RELEASE, I'd take a closer look at your configuration first.
-
The code that is in place is correct for most setups, perhaps there is something else unique/incorrect about your configuration contributing to the detection failure, but we had some fallout from this while testing the -RELEASE images.
So the code is back to how it was before, and it is confirmed working on several very large CARP+DHCP Failover setups. If it does not work for you on 2.1-RELEASE, I'd take a closer look at your configuration first.
Okay. I don't see how the original code could be correct, since the loop variable to
check for "a defined vip" (according to the commentary) isn't actually used for
searching, and I pointed out that your fix was not correct either.
But I understand you don't want to break things just before release. My later
version seems to work correctly for me, so I'll just work with a local patch then.
I suggest to have a look at this again in 2.1-stable .Cheers,
MarkusPS: for illustration (original):
foreach ($a_vip as $vipent) { if($int == $real_dhcpif) { /* this is the interface! */ if(is_numeric($vipent['advskew']) && ($vipent['advskew'] < "20")) $skew = 0; } }
can be transformed into:
if($int == $real_dhcpif) { foreach ($a_vip as $vipent) { /* this is the interface! */ if(is_numeric($vipent['advskew']) && ($vipent['advskew'] < "20")) $skew = 0; } }
and this just doesn't look correct to me.