overly strict check in normal_binding_state_transition_check?

[Kevan Carstensen]

Hi,

normal_binding_state_transition_check in failover.c will reject
transitions from active to abandoned that are not initiated by the
primary of a failover pair: 

    switch (lease -> binding_state) {
        [...snip...]
        case FTS_ACTIVE:
        /* The secondary can't change the state of an active
           lease. */
        if (state -> i_am == primary) {
            /* Except that the client may send the DHCPRELEASE
               to the secondary, and we have to accept that. */
            if (binding_state == FTS_RELEASED)
                return binding_state;
            new_state = lease -> binding_state;
            goto out;
        }
        [...snip...]

I think that this is overly conservative. There are situations in which
is reasonable for only one of the primary and secondary to know that a
lease should be abandoned -- for example, if a lease is to be abandoned
because of a ICMP echo initiated by one server. Given that, it seems
that the primary should accept these updates from the secondary (as the
secondary does from the primary) instead of rejecting them so that the
primary has a more accurate view of the state of things.

We occasionally see the following messages in the logs from the
secondary of our failover setup:

  Mar 30 14:40:56 gemini dhcpd: bind update on xx.xx.xx.xx from cpp rejected: xx.xx.xx.xx: invalid state transition: active to abandoned

We're not aware of any service issues associated with the messages, so
they're more of a curiosity than anything, but we'd still prefer that
they go away. :-) It looks pretty easy to alter
normal_binding_state_transition_check to accept transitions to abandoned
from the secondary; if no one objects to that change, I can send a
patch.

Thanks,
-- 
Kevan Carstensen                        <kacarstensen at csupomona.edu>
Operating Systems Analyst, I&IT Systems, Cal Poly Pomona