[Swan-dev] IKEv2 responder rekey code is fooed

[Andrew Cagney]

On Sun, 19 Apr 2020 at 06:57, Antony Antony <antony at phenome.org> wrote:

> Dear fellow developers.
>
> I just noticed the IKEv2 IPsec rekey responder code has regressed beyond
> recognition! too many changes after the main regression:) While trying to
> figure out I notice logging and debugging lines changed too (possibly old)
> some with STATE_ and other without the prefix STATE_.  This make it hard
> to
> follow the regression.
>
> I suggest we take pause and retrace the steps. Also changing IKEv2 STATE_
> is
> , as I recollect, discontented issue. And I feel change has been sneaked
> in.
> Also use of some with STATE_ and others without "STATE_" is annoying.
>

The enum currently uses the prefixes:
   STATE_<ikev1>
   STATE_<ikev2>
   STATE_V2_<ikev2>
this should be made consistent and:
  STATE_V1_...
  STATE_v2_...
used through out.


>
> 2. log/debug started using short names and mixing them with long state
> names.  This should not happen! Please keep the state names long and
> consistent.
>
> switching IKEv2 MD.ST from CHILD #3 V2_CREATE_R0 to CHILD #3 V2_CREATE_R0
>
> In some other parts of the log it is full name.
>
> please keep the full name.
>
>
For debug  I'm not fussed.

However for logging the state names shouldn't appear at all and instead
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.

I also really really need to kill the redundant _I and _R established
states.

"east" #4 complete v2 state STATE_V2_REKEY_CHILD_R0 transition with
> STF_SUSPEND suspended from complete_v2_state_transition:3399
>
> NOTE: because of changes to state names since 8abf1c415a  in master you
> will
> see.
>
>
> https://testing.libreswan.org/v3.30-507-ge06a82880f-master/ikev2-child-rekey/OUTPUT/east.pluto.log.gz
> Apr 18 23:52:26.316312: | child state #3: V2_NEW_CHILD_R0(established IKE
> SA) => V2_IPSEC_R(established CHILD SA)
>
> It should be something like STATE_V2_REKEY_CHILD_R0 =>
> STATE_V2_IPSEC_R(established)
>
> -antony
>
> PS: at first glance initiator code seems ok. Again hard to say becuase of
> log changes.
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libreswan.org/pipermail/swan-dev/attachments/20200504/35d28a80/attachment-0001.html>