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

[Andrew Cagney]

On Fri, 13 Dec 2019 at 09:45, Paul Wouters <paul at nohats.ca> wrote:
>
> On Thu, 12 Dec 2019, Andrew Cagney wrote:
>
> >> Aliased connections with auto=start must get revived. Similar for narrowing=yes connections which become instances.
> >
> > The revival code uses c->name as the connection identifier, not
> > c->connalias.  So if a connection brought up using an alias goes down
> > then c->name should re-find it.
>
> Ah right. I dont like us depending on c->name for anything but yeah I
> guess it does safe us here.

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?

> > (As an aside, conn_by_name() re-orders the connection list - so if
> > both the conn and its instance were in the list then luck would
> > determine which was found).
>
> Hmm.
>
> Paul