[Swan-dev] I think 229e2d24a4 needs to be reverted

Andrew Cagney andrew.cagney at gmail.com
Wed May 8 14:30:55 UTC 2019


On Tue, 7 May 2019 at 21:22, Paul Wouters <paul at nohats.ca> wrote:
>
>
> -#define IS_IKE_SA(st) ( ((st)->st_clonedfrom == SOS_NOBODY) && \
> -       (IS_PHASE1((st)->st_state) || IS_PHASE15((st)->st_state) ||
>          IS_PARENT_SA(st)) )
> +#define IS_IKE_SA(st)   ((st)->st_clonedfrom == SOS_NOBODY)
>
>
>
> The idea here is that the normal case for checking IS_IKE_SA() depends
> only on the state it is in.

Turns out it's no longer the case, and I suspect hasn't been since the
IS_{PARENT,CHILD}_SA() macros were reduced to using
(st)->st_clonedfrom ?= SOS_NOBODY.

I'll offer this as proof.  Given:

     #define IS_CHILD_SA(st)  ((st)->st_clonedfrom != SOS_NOBODY)
     #define IS_PARENT_SA(st) (!IS_CHILD_SA(st))
     #define IS_IKE_SA(st) ( ((st)->st_clonedfrom == SOS_NOBODY) && \
            (IS_PHASE1((st)->st_state) || IS_PHASE15((st)->st_state)
|| IS_PARENT_SA(st)) )

-> expanding IS_IKE_SA(st_clonedfrom!=SOS_NOBODY):

    (false) && (IS_PHASE1((st)->st_state) ||
IS_PHASE15((st)->st_state) || IS_PARENT_SA(st)) )
    (false) /// false&&anything == false

since false && anything == false -
IS_IKE_SA(st_clonedfrom!=SOS_NOBODY) == false.

-> expanding IS_IKE_SA(st_clonedfrom==SOS_NOBODY):
   IS_CHILD_SA() == false
   IS_PARENT_SA() == true
   (true) && (IS_PHASE1((st)->st_state) || IS_PHASE15((st)->st_state) || true) )
   (true) && (true) /// anything||true == true
   (true)

So here - to Antony's point about the dark ages - both IS_PHASE1() and
IS_PHASE15() are actually don't cares.

However, if this doesn't seem right for 3.28 revert it; it can go back
in afterwards.

BTW, the patch I'm not sure about is the one that follows - both
flush_pending_child() and eliminate an O(#STATES) loop.


More information about the Swan-dev mailing list