[Swan-dev] revival, was Fwd: [Swan-commit] Changes to ref refs/heads/master

[Paul Wouters]

On Fri, 13 Dec 2019, Andrew Cagney wrote:

> Yea, there's a lot of overlap between this code and pending (the
> latter has a reference to the connection).
>
>>> However, for instances, things seem more interesting.  Is there a test
>>> I should look at?  My assumption was that by the time revival occurred
>>> the connection instance should be long gone?
>>
>> Look at ikev2-allow-narrow-04. It installs a narrowed down IPsec SA, so
>> using an instance.
>
> If I shutdown east, west does this:
>
> Things start out ok:
>
> "westnet-eastnet-ikev2"[1] 192.1.2.23 #1: deleting state
> (STATE_IKESA_DEL) aged 37.088s and NOT sending notification
> ...
> "westnet-eastnet-ikev2"[1] 192.1.2.23 #1: deleting IKE SA but
> connection is supposed to remain up; schedule EVENT_REVIVE_CONNS
> | add revival: connection 'westnet-eastnet-ikev2' added to the list
> and scheduled for 0 seconds
> | global one-shot timer EVENT_REVIVE_CONNS scheduled in 0 seconds
>
> and as I suspected, the instance gets deleted:
>
> #1: deleting connection "westnet-eastnet-ikev2"[1] 192.1.2.23
> instance with peer 192.1.2.23 {isakmp=#0/ipsec=#0}
>
> however, as part of that it calls flush_revival() unconditionally:
>
> | flush revival: connection 'westnet-eastnet-ikev2' revival flushed
>
> and consequently:
>
> | processing global timer EVENT_REVIVE_CONNS
> | spent 0 milliseconds in global timer EVENT_REVIVE_CONNS
>
> which means it never worked :-(
>
> So flush_revival() shouldn't be called when an instance?

If this was a connection with narrowing=yes and right=staticip and
auto=start, we _should_ revive. This is an instance.

If this was a connection with auto=add and right=%any then
we _should not_ revive. This is an instance too.

But all those items should be clear when calling add_revival() or not.

Paul