* Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com> [2015-05-21 21:31]:
> On Thu, May 21, 2015 at 07:43:51PM +0200, Mike Belopuhov wrote:
> > On Thu, May 21, 2015 at 17:34 +0200, Alexandr Nedvedicky wrote:
> > > snippet below comes from pf_create_state():
> > >
> > > 3559 if (pf_state_insert(BOUND_IFACE(r, pd->kif), skw, sks,
> > > s)) {
> > > 3560 pf_state_key_detach(s, PF_SK_STACK);
> > > 3561 pf_state_key_detach(s, PF_SK_WIRE);
> > > 3562 *sks = *skw = NULL;
> > > 3563 REASON_SET(&reason, PFRES_STATEINS);
> > > 3564 goto csfailed;
> > > 3565 } else
> > > 3566 *sm = s;
> > > 3567
> > > 3568 /* attach src nodes late, otherwise cleanup on error
> > > nontrivial */
> > > 3569 for (i = 0; i < PF_SN_MAX; i++)
> > > 3570 if (sns[i] != NULL) {
> > > 3571 struct pf_sn_item *sni;
> > > 3572
> > > 3573 sni = pool_get(&pf_sn_item_pl, PR_NOWAIT);
> > > 3574 if (sni == NULL) {
> > > 3575 REASON_SET(&reason, PFRES_MEMORY);
> > > 3576 pf_src_tree_remove_state(s);
> > > 3577 STATE_DEC_COUNTERS(s);
> > > 3578 pool_put(&pf_state_pl, s);
> > > 3579 return (PF_DROP);
> > > 3580 }
> > > 3581 sni->sn = sns[i];
> > > 3582 SLIST_INSERT_HEAD(&s->src_nodes, sni,
> > > next);
> > > 3583 sni->sn->states++;
> > > 3584 }
> > >
> > > at line 3559 PF inserts state to table. If insert operation succeeds, then
> > > state can no longer be killed using simple pool_put() as it currently
> > > happens at line 3578. I think PF should go for pf_unlink_state() instead.
> > > patch below should kill the bug.
> > Indeed. But I don't like the comment stating that we're attaching
> > src nodes late because the "cleanup on error nontrivial". Perhaps
> > we should do a pf_state_insert afterwards? This might simplify
> > locking later on.
> perhaps swapping the for loop block with pf_state_insert() will work.
> We can then bail out using goto csfailed then (see patch below...)
makes sense, I like it.
> > > would you be interested in SMP patch for PF?
> > > it basically introduces fine locking and reference counting
> > > on PF data objects, so firewall can handle more packets at
> > > single instance of time.
I'd certainly like to see it.
As Mike points out, there's more to do before it can be useful for us
tho :/
> > Thanks for your quality diffs btw, help is always much appreciated.
absolutely!
--
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/
