From owner-freebsd-virtualization@FreeBSD.ORG  Wed Aug 14 15:29:32 2013
Return-Path: <owner-freebsd-virtualization@FreeBSD.ORG>
Delivered-To: freebsd-virtualization@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by hub.freebsd.org (Postfix) with ESMTP id 584EBC43;
 Wed, 14 Aug 2013 15:29:32 +0000 (UTC) (envelope-from zec@fer.hr)
Received: from mail.fer.hr (mail.fer.hr [161.53.72.233])
 (using TLSv1 with cipher AES128-SHA (128/128 bits))
 (No client certificate requested)
 by mx1.freebsd.org (Postfix) with ESMTPS id CF191222B;
 Wed, 14 Aug 2013 15:29:31 +0000 (UTC)
Received: from x23.lan (89.164.9.9) by MAIL.fer.hr (161.53.72.233) with
 Microsoft SMTP Server (TLS) id 14.2.342.3; Wed, 14 Aug 2013 17:28:18 +0200
From: Marko Zec <zec@fer.hr>
To: Mikolaj Golub <trociny@freebsd.org>
Subject: Re: RFC: ipfw nat VIMAGE improvements
Date: Wed, 14 Aug 2013 17:28:31 +0200
User-Agent: KMail/1.9.10
References: <20130811200111.GA49895@gmail.com>
In-Reply-To: <20130811200111.GA49895@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-ID: <201308141728.31361.zec@fer.hr>
X-Originating-IP: [89.164.9.9]
Cc: freebsd-virtualization@freebsd.org
X-BeenThere: freebsd-virtualization@freebsd.org
X-Mailman-Version: 2.1.14
Precedence: list
List-Id: "Discussion of various virtualization techniques FreeBSD supports."
 <freebsd-virtualization.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-virtualization>, 
 <mailto:freebsd-virtualization-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-virtualization>
List-Post: <mailto:freebsd-virtualization@freebsd.org>
List-Help: <mailto:freebsd-virtualization-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization>, 
 <mailto:freebsd-virtualization-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 14 Aug 2013 15:29:32 -0000

On Sunday 11 August 2013 22:01:12 Mikolaj Golub wrote:
> Hi,
>
> I would like to commit this patch that fixes some issues related to
> ipfw nat module load/unload on VIMAGE featured system.
>
> Any comments, objections?

Far from being an expert in ipfw, I'm worried that the proposed approach of 
simultaneously acquiring locks on _all_ ipfw instances might be calling for 
trouble:

+       VNET_LIST_RLOCK();
+       VNET_FOREACH(vnet_iter) {
+               CURVNET_SET(vnet_iter);
+               IPFW_WLOCK(&V_layer3_chain);
+               CURVNET_RESTORE();
+       }
        ipfw_nat_ptr = ipfw_nat;
        lookup_nat_ptr = lookup_nat;
        ipfw_nat_cfg_ptr = ipfw_nat_cfg;
        ipfw_nat_del_ptr = ipfw_nat_del;
        ipfw_nat_get_cfg_ptr = ipfw_nat_get_cfg;
        ipfw_nat_get_log_ptr = ipfw_nat_get_log;
-       IPFW_WUNLOCK(&V_layer3_chain);
-       V_ifaddr_event_tag = EVENTHANDLER_REGISTER(
+       VNET_FOREACH(vnet_iter) {
+               CURVNET_SET(vnet_iter);
+               IPFW_WUNLOCK(&V_layer3_chain);
+               CURVNET_RESTORE();
+       }
+       VNET_LIST_RUNLOCK();

Why couldn't we introduce a per-vnet flag, say V_ipfw_nat_ready, and use it 
as

#define IPFW_NAT_LOADED (V_ipfw_nat_ready)

instead of current version of that macro:

#define IPFW_NAT_LOADED (ipfw_nat_ptr != NULL)

I.e., perhaps in ipfw_nat_init() we could first set all the function 
pointers, and then iterate over all vnets and set V_ipfw_nat ready there.  
In ipfw_nat_destroy() we would first iterate over all vnets to clear the 
flag, before clearing function pointers?

Marko