[Swan-dev] more dpd and liveness comments

[Matt Rogers]

----- Original Message -----
> From: "Paul Wouters" <paul at nohats.ca>
> To: mrogers at redhat.com
> Cc: swan-dev at lists.libreswan.org
> Sent: Sunday, February 9, 2014 5:50:53 PM
> Subject: more dpd and liveness comments
> 
> 
> 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
> 

This makes sense to me. We set those local variables after a check for 
for dpd_action and dpd_timeout together anyways. I'll run a patch through
the dpd test cases to be sure.

Matt