Date: Mon, 31 Mar 2014 22:41:13 +0300 From: Mikolaj Golub <trociny@FreeBSD.org> To: Martin Matuska <mm@FreeBSD.org> Cc: freebsd-pf@FreeBSD.org Subject: Re: CFR projects/pf: vnet awareness for pf_overloadqueue Message-ID: <20140331194109.GA17582@gmail.com> In-Reply-To: <5337D55A.6030607@FreeBSD.org> References: <5337D55A.6030607@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@. 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140331194109.GA17582>