----- Original Message ----- > From: "Paul Wouters" <[email protected]> > To: [email protected] > Cc: [email protected] > 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 _______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
