[Swan-dev] flush_pending_ipsec() question

[Paul Wouters]

Hey,

I was looking through my stashing and found one that was addressing
the flush_pending_ipsec() issue we found. I verified the current
state and I see we have a different fix in place now.

I am wondering about this code from commit 7097a65bfd

+               if (IS_IPSEC_SA_ESTABLISHED(st->st_state))
+                       return;
+
+               delete_event(st);
+               if (st->st_serialno > c->newest_ipsec_sa &&
+                               (c->policy & POLICY_UP) &&
+                               (c->policy & POLICY_DONT_REKEY) == LEMPTY)
+               {
 			[ reschedule ]
 		} else {
 			[ expire ]
 		}

I don't understant the serialno check. newest_ipsec_sa is set via
set_newest_ipsec_sa() but as you can see where it is called, it only
refers to established ipsec SA's that have been successfully installed
in the kernel. So those are always IS_IPSEC_SA_ESTABLISHED()

So we have either no established ipsec sa (and newest points to SOS_NOBDY)
or we have an established and an unestablished ipsec sa on the same
connection. In the first case, the check is not needed as SOS_NOBDY is
the lowest possible serialno. In the second case I don't understand what
the serialno check indicates? What is it selecting and why?

Either this only happens when we were trying to initiate a new one while
having one, so the serialno will always be higher (and the check is not
needed), or there can be a situation where two unestablished ipsec states
were racing (while there was an established ipsec sa or none) and both
serial numbers are higher, or there was no established one and newest is
set to SOS_NOBODY and thus the serialno is always higher per definition.

So I think the serial check should be removed. If not, can someone tell
me which two cases are distinguished here that are based on serialno ?


Paul