Date: Wed, 14 Aug 2013 17:28:31 +0200 From: Marko Zec <zec@fer.hr> To: Mikolaj Golub <trociny@freebsd.org> Cc: freebsd-virtualization@freebsd.org Subject: Re: RFC: ipfw nat VIMAGE improvements Message-ID: <201308141728.31361.zec@fer.hr> In-Reply-To: <20130811200111.GA49895@gmail.com> References: <20130811200111.GA49895@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308141728.31361.zec>