On Fri, Jan 30, 2009 at 10:20:51AM -0800, Erik Nordmark wrote:
>> Here's the webrev:
>>
>> http://cr.opensolaris.org/~danmcd/6777776/
>
> ipsecesp.c would hit an assertion failure if you were to run this, because
> db_type can't be both a M_CTL and an M_DATA.
>
> 2307 ASSERT(ipsec_mp->b_datap->db_type == M_CTL);
> 2308 ASSERT(io->ipsec_out_type == IPSEC_OUT);
> 2309 ASSERT(ipsec_mp->b_cont != NULL);
> 2310 ASSERT(ipsec_mp->b_datap->db_type == M_DATA);
Doh! Forgot the ->b_cont...
ASSERT(ipsec_mp->b_datap->db_type == M_CTL);
ASSERT(io->ipsec_out_type == IPSEC_OUT);
ASSERT(ipsec_mp->b_cont != NULL);
ASSERT(ipsec_mp->b_cont->b_datap->db_type == M_DATA);
That's what I meant to say. My tester used non-DEBUG bits, and I was waiting
for more review prior to an IKE-STC run. (I've two other bugfix wads, and
this was not first.)
> For the inbound_task using ipsec_in_stackid I'm having a hard time
> convincing myself that ipsec_in_stackid is always set. Does it need to be
> set before we kick off inbound_task?
lpkt always has its stackid set. See sadb_set_lpkt() in sadb.c (part of this
bugfix) for where it is done.
> Or would it be better to only set ipsec_in_stackid in the 3 or so places
> where we set ipsec_in_ns? (i.e., remove the two places we currently set it
> in favor of those three.)
Now THAT might be a better idea. We'd need to add spd.c to the changeset,
and put it in the two places (ipsec_in_alloc() and ipsec_out_to_in())
ipsec_in_ns is set today.
I've posted a rewhacked webrev to take into account your suggestions:
http://cr.opensolaris.org/~danmcd/6777776-postreview/
Is that what you had in mind? If so, I'll go off to the IKE-STC races.
Thanks!
Dan
_______________________________________________
networking-discuss mailing list
[email protected]
