<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 19 Apr 2020 at 06:57, Antony Antony <<a href="mailto:antony@phenome.org">antony@phenome.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Dear fellow developers.<br>
<br>
I just noticed the IKEv2 IPsec rekey responder code has regressed beyond <br>
recognition! too many changes after the main regression:) While trying to <br>
figure out I notice logging and debugging lines changed too (possibly old) <br>
some with STATE_ and other without the prefix STATE_.  This make it hard to <br>
follow the regression. <br>
<br>
I suggest we take pause and retrace the steps. Also changing IKEv2 STATE_ is <br>
, as I recollect, discontented issue. And I feel change has been sneaked in.  <br>
Also use of some with STATE_ and others without "STATE_" is annoying.<br></blockquote><div><br></div><div>The enum currently uses the prefixes:<br></div><div>   STATE_<ikev1></div><div>   STATE_<ikev2></div><div>   STATE_V2_<ikev2></div><div>this should be made consistent and:</div><div>  STATE_V1_...</div><div>  STATE_v2_...<br></div><div> used through out.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br><br>
2. log/debug started using short names and mixing them with long state <br>
names.  This should not happen! Please keep the state names long and <br>
consistent.  <br>
<br>
switching IKEv2 <a href="http://MD.ST" rel="noreferrer" target="_blank">MD.ST</a> from CHILD #3 V2_CREATE_R0 to CHILD #3 V2_CREATE_R0 <br>
<br>
In some other parts of the log it is full name.<br>
<br>
please keep the full name.<br>
<br></blockquote><div><br></div><div>For debug  I'm not fussed.</div><div><br></div><div>However for logging the state names shouldn't appear at all and instead <br></div><div>meaningful stories should be used.  If they really must then the short form (as in drop STATE_V2_) would be better  as the 'STATE_V2_' prefix just wastes real estate.<br></div><div><br></div><div>I also really really need to kill the redundant _I and _R established states.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
"east" #4 complete v2 state STATE_V2_REKEY_CHILD_R0 transition with <br>
STF_SUSPEND suspended from complete_v2_state_transition:3399 <br>
<br>
NOTE: because of changes to state names since 8abf1c415a  in master you will <br>
see. <br>
<br>
<a href="https://testing.libreswan.org/v3.30-507-ge06a82880f-master/ikev2-child-rekey/OUTPUT/east.pluto.log.gz" rel="noreferrer" target="_blank">https://testing.libreswan.org/v3.30-507-ge06a82880f-master/ikev2-child-rekey/OUTPUT/east.pluto.log.gz</a><br>
Apr 18 23:52:26.316312: | child state #3: V2_NEW_CHILD_R0(established IKE SA) => V2_IPSEC_R(established CHILD SA)<br>
<br>
It should be something like STATE_V2_REKEY_CHILD_R0 => STATE_V2_IPSEC_R(established)<br>
<br>
-antony<br>
<br>
PS: at first glance initiator code seems ok. Again hard to say becuase of <br>
log changes.<br>
_______________________________________________<br>
Swan-dev mailing list<br>
<a href="mailto:Swan-dev@lists.libreswan.org" target="_blank">Swan-dev@lists.libreswan.org</a><br>
<a href="https://lists.libreswan.org/mailman/listinfo/swan-dev" rel="noreferrer" target="_blank">https://lists.libreswan.org/mailman/listinfo/swan-dev</a><br>
</blockquote></div></div>