[Swan-dev] more dpd and liveness comments

Paul Wouters paul at nohats.ca
Mon Feb 10 00:50:53 EET 2014


Looking at:

         bool st_liveness;                       /* Liveness checks */
         bool st_dpd;                            /* Peer supports DPD */
         bool st_dpd_local;                      /* If we want DPD on this conn */

I got confused by these hidden_variable names.

st_dpd talks about the peer's capability but st_liveness talks about or
local intention like st_dpd_local.

It seems to me we probably should rename these variables, something
like:

         bool st_liveness_local;                 /* If we want Liveness checks on this conn */
         bool st_dpd_local;                      /* If we want DPD on this conn */
         bool st_peer_supports_dpd;              /* Peer supports DPD - IKEv2 liveness support is mandatory*/

However, the question of "do we want DPD/Liveness" on this connection is
not something we should/need to track in the state. We can simply check
st->st_connection->dpd_action && st->st_connection->dpd_timeout

I think for IKEv1, the "we want DPD on this conn" depends on (or was
supposed to depend on) two things: our local policy and the peer's
capabilities. For IKEv2 we know the peer always supports liveness, as it
is mandatory to implement.

So I'm thinking we should be able to reduce these three variables down
to one:

         bool st_peer_supports_dpd;                            /* Peer supports DPD - IKEv2 liveness support is mandatory*/

We should take a close look at st_dpd_local vs st_dpd as the names could
have very easilly have led to confusion and mixup.

Paul


More information about the Swan-dev mailing list