On Wed, Mar 27, 2002 at 04:07:19AM +0900, itojun@iijlab.net wrote:
> >I think there is a bug that prevents initial-contact working.
> >is attached patch ok?
> 
> 	1st part of the patch makes the code more confusing.  i don't think
> 	it is necessary.  2nd part looks correct.
Sorry, but i beg to differ:

Well, it is the FIRST part, where the bug was (if the first part
was taken from my PR). cmpsaddrwop() returns 0 if the addresses match.
The THEN statements process the no match condition. Thus the skip 
condition holds already (outer OR) on one reversed address test for
different addresses for src and dst. So SKIP is guaranteed in the
case where SA should be deleted (as one OR the other sub expression
will hold 8-(). The original code will skip as long as at least one
address mis-match is present (in original or reversed test order).
The code would have been correct if
the THEN part would process the deletion of SAs AND cmpsaddrwop() would
return !=0 for matches.
The patch actually is a De Morgan transformation of the original code taking
into account that the THEN section does process the SKIP statements and
the necessary negation on the inner operands is already there because
cmpsaddrwop() returns 0 on match.

If in doubt, step through the code for address pairs that should lead to
SA deletion. You'll find, that one side of the OR will be FALSE and the
other MUST be TRUE for different adresses for src and dst. This leads to
TRUE and thus to a SKIP - not what was intended by INITIAL CONTACT.

Sorry for being so elaborate, but the bug is subtle and somewhat hidden
inbetween assumtions and boolean algebra.

I haven't checked the second part.

> 
> itojun
> 

Frank

PS: first patch is pretty successful in production 8-) it actually deletes
    old SAs