CFR projects/pf: vnet awareness for pf_overloadqueue

[Mikolaj Golub]

On Sun, Mar 30, 2014 at 10:27:06AM +0200, Martin Matuska wrote:
> Hi,
> 
> with the pf_mtag_z patch applied, the second patch that fixes panics I
> experience is the overload queue patch.
> 
> I have looked into solving this via context (adding a "struct vnet"
> member to pf_overload_entry). This leaves unsolved problems - first, we
> have now vnet information on per-entry instead of per-queue.
> 
> There are two places in pf_overload_task() where we are not processing
> an entry but need vnet information:
> 
> 1. V_pf_idhash[i] in pf_overload_task():
> 
>         for (int i = 0; i <= pf_hashmask; i++) {
>                 struct pf_idhash *ih = &V_pf_idhash[i];
>                 struct pf_state_key *sk;
>                 struct pf_state *s;
> 
>                 PF_HASHROW_LOCK(ih);
>                 LIST_FOREACH(s, &ih->states, entry) {
> 
> 2. end of pf_overload_task() but that is only the debug tunable
> V_pf_status_debug:
> 
>         if (V_pf_status.debug >= PF_DEBUG_MISC)
>                 printf("%s: %u states killed", __func__, killed);
> 
> On the other hand, if we want to keep per-vnet overloadqueues than it
> makes sense to store vnet information on queue level.
> If we pack vnet information into each entry and the overloadqueue has
> global locks anyway, why not keeping a single global queue with entries
> from different vnets?
> 
> At the current state the code causes panics if pf_overload_task() is
> fired because vnet context is missing. It needs to be fixed in any of
> the ways. A patch for adding per-queue vnet information is attached.

Martin, I think you missed my point in the message to src at . You don't
need to embed vnet into pf_overloadqueue. What you need is a way to
access it in a right vnet context, i.e. by passing vnet as an argument
to pf_overload_task:

 pf_vnet_initialize():

-       TASK_INIT(&V_pf_overloadtask, 0, pf_overload_task, &V_pf_overloadqueue);
+       TASK_INIT(&V_pf_overloadtask, 0, pf_overload_task, curvnet);

 pf_overload_task(void *c, int pending):
 {
+       struct vnet *vnet;
        struct pf_overload_head queue;
        struct pfr_addr p;
        struct pf_overload_entry *pfoe, *pfoe1;
        uint32_t killed = 0;
 
+       vnet = (vnet *)c;
+       CURVNET_SET(vnet);
        PF_OVERLOADQ_LOCK();
-       queue = *(struct pf_overload_head *)c;
-       SLIST_INIT((struct pf_overload_head *)c);
+       queue = V_pf_overloadqueue;
+       SLIST_INIT(V_pf_overloadqueue);
        PF_OVERLOADQ_UNLOCK();

-- 
Mikolaj Golub