From owner-freebsd-hackers@FreeBSD.ORG Tue Jan 31 11:01:33 2012 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BEA2A106564A; Tue, 31 Jan 2012 11:01:33 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 635B98FC20; Tue, 31 Jan 2012 11:01:33 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id DA28F7300A; Tue, 31 Jan 2012 12:02:04 +0100 (CET) Date: Tue, 31 Jan 2012 12:02:04 +0100 From: Luigi Rizzo To: Ermal Lu?i Message-ID: <20120131110204.GA95472@onelab2.iet.unipi.it> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i Cc: freebsd-net , freebsd-hackers@freebsd.org Subject: Re: [PATCH] multiple instances of ipfw(4) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Jan 2012 11:01:33 -0000 On Mon, Jan 30, 2012 at 01:01:13PM +0100, Ermal Lu?i wrote: > Hello, > > from needs on pfSense a patch for allowing multiple intances of > ipfw(4) in kernel to co-exist was developed. > It can be found here > https://raw.github.com/bsdperimeter/pfsense-tools/master/patches/RELENG_9_0/CP_multi_instance_ipfw.diff > > It is used in conjuction with this tool > https://raw.github.com/bsdperimeter/pfsense-tools/master/pfPorts/ipfw_context/files/ipfw_context.c > It allows creation of contextes/instances and assignment of specific > interfaces to specific contexts/instances. > > Surely i know that this is not the best way to implement generically > but it gets the job done for us as it is, read below. > > What i would like to know is if there is interest to see such > functionality in FreeBSD? > > I am asking first to see if there is some consensus about this as a > feature, needed or not! > If interest is shown i will transform the patch to allow: > - ipfw(8) to manage the contextes create/destroy > - ipfw(8) to manage interface membership. Closing the race of two > parallell clients modifying different contextes. > > There is another design choice to be made about storing the membership > of interfaces into contexts/instances, but i do not see that as > blocking. > > It is quite handy feature, which can be exploited even to scale on SMP > machines by extending it to bind a specific instance(with its > interaces) to a specific CPU/core?! > > Comments/Feedback expected, if i understand what the patch does, i think it makes sense to be able to hook ipfw instances to specific interfaces/sets of interfaces, as it permits the writing of more readable rulesets. Right now the workaround is start the ruleset with skipto rules matching on interface names, and then use some discipline in "reserving" a range of rule numbers to each interface. Before making more changes to the code, it would help if you could give a high level description of 1. what the change does and how specific cases are handled. E.g. With this change you can create multiple rulesets (contexts ?) and bind one or more interfaces to a context. - what happens with outgoing packets where the context to be picked up depend on the route in effect at the time of the transmission ? - what happens with encapsulated interfaces (vlan) ? - can you skipto across contexts (i guess not) ? 2. how intrusive are code changes ? The kernel patch you show seems small, which makes sense as i believe all is needed is to start from a specific chain instead of the default one when an interface is bound to a context. A few comments: - if you use one of the if_ispare directly, instead of renaming it to if_context, this would make backporting and testing easier. - I think the explosion of sockopt names is a bad thing. The IP_FW3 command was introduced exactly to have a single entry point to the firewall and avoid a ton of new names in raw_ip.c and in.h - can you reduce the number of global ipfw-related variables ? There used to be one (layer3_chain), now you have 3 of them. You should delete layer3_chain and replace it with another single global (could be ip_fw_contexts) which contains the whole firewall state. - how do you handle reinjects (e.g. from dummynet or divert) ? I don't remember if the metadata that stores where you reinject the packet also has a pointer to the root of the chain. - i don't completely follow the connection between ip_fw_chain, ip_fw_ctx_iflist, ip_fw_ctxmember, ip_fw_ctx, ip_fw_ctx_list. The way i see it: - the ip_fw_chain could be embedded in the ip_fw_ctx, as they map 1:1 - why do you need ip_fw_ctx_iflist and ip_fw_ctxmember ? You should never need to determine context membership during packet processing, and for sockopt calls you could as well iterate over the list of interfaces; cheers luigi