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>