From owner-freebsd-hackers@FreeBSD.ORG Mon Jun 10 16:52:03 2013 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 21FEB632; Mon, 10 Jun 2013 16:52:03 +0000 (UTC) (envelope-from ermal.luci@gmail.com) Received: from mail-qe0-f48.google.com (mail-qe0-f48.google.com [209.85.128.48]) by mx1.freebsd.org (Postfix) with ESMTP id B35071368; Mon, 10 Jun 2013 16:52:02 +0000 (UTC) Received: by mail-qe0-f48.google.com with SMTP id 2so4185020qea.35 for ; Mon, 10 Jun 2013 09:52:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=26YHp/W4zVGAu7j5J7AtFMQcl2Lzstj0rFpC2qovtjI=; b=Kg29vaHviMWw3DncQHdTHjO4QVCg1Z0LOIYLc9bvsxp95464cSVkHCy7o49KioYmR7 qrPigr5iqAx8+5kd/KS7h8HSibUfGzdl4i46g4aev16zZt4Tm6YYc+1nv/YpkCe+scNh eZs42IhXWi0PTUf+Z4Q7XU6rjUXAUCa45gAj/eQFr9NqwV3FYk35cA5cmkjJAooJ4nFM lR3tl53TfEUMssY23+PCQts53ge8oEYSutRU5u2ajUb69FPB1zM12UMAu5ZvWDwEOy1I x0HdvSHYrOpMaug/JAtv34ZmxKGxwpj/+/yLmFgC9a+vo41YzAPXM92tBISKKuZMZNS1 RZ3Q== MIME-Version: 1.0 X-Received: by 10.229.144.14 with SMTP id x14mr4085767qcu.36.1370883121423; Mon, 10 Jun 2013 09:52:01 -0700 (PDT) Sender: ermal.luci@gmail.com Received: by 10.49.51.8 with HTTP; Mon, 10 Jun 2013 09:52:01 -0700 (PDT) In-Reply-To: References: <20120131110204.GA95472@onelab2.iet.unipi.it> <20120208133559.GK13554@FreeBSD.org> <20120208140921.GM13554@glebius.int.ru> <4F344CE4.301@freebsd.org> Date: Mon, 10 Jun 2013 18:52:01 +0200 X-Google-Sender-Auth: J4_vl2pAQ2S3gdwxfsAffdz3t-c Message-ID: Subject: Re: [PATCH] multiple instances of ipfw(4) From: =?ISO-8859-1?Q?Ermal_Lu=E7i?= To: Luigi Rizzo Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.14 Cc: freebsd-net , freebsd-hackers@freebsd.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Jun 2013 16:52:03 -0000 On Mon, Jun 10, 2013 at 5:01 PM, Luigi Rizzo wrote: > > > > On Mon, Jun 10, 2013 at 3:30 PM, Ermal Lu=E7i wrote: > >> Hello, >> >> reviving this old thread since i had time to bring the patch to FreeBSD = 10 >> and unified the whole controlling under ipfw(8) binary. >> >> For reminder, the patch located at [1] provides multiple instances for >> ipfw(4). >> Basically you can control which interfaces belong to which context/rules= et >> to make maintaining easier. >> >> > ... > > > >> Any objections on pushing this into FreeBSD? >> >> >> [1] >> >> https://github.com/pfsense/pfsense-tools/blob/master/patches/RELENG_10_0= /CP_multi_instance_ipfw.diff >> >> >> > > if i understand well, this has no runtime overhead as the ifp has > the index of the context it refers to ? > Or you need an additional IPFW_CTX_RLOCK() ? > Theoretically you would need for correctness the read lock. It has never been hit in pfSense hence no further investigation on it has been done. It can be made even a read mostly lock or to prevent the race the write lock of the pfil hooks so no packets are passed through?! > > Comments on the control/config path: > - in ipfw_ctl(), handling IP_FW_CTX_GET i am worried that you might > overflow the temporary buffer when building the list. You compute > the length under rlock, release the lock, malloc(), then fill the > list without checking if the total size is still correct. > This kind of code is terribly boring to write, but essentially > you need a bound check in the second loop and possibly > retry if you notice that you need more memory. > "ipfw show" addresses the problem by failing and requesting the > user application to pass a larger buffer. > Yeah that probably can be fixed. During implementation it was considered enough rare operation to not justify further thought. > > - similarly, how do you guarantee that deleting a context while > a packet is under processing does not cause dereferencing a > NULL pointer ? > Presently, none, but as i said during instance/context operation a write lock on the pfil hook can be taken? That should prevent the issue altogether and remove the requirement of having to read lock the fast path. If you agree with the above i can redo the patch again with the above changes for review? > cheers > luigi > > while >> -- >> Ermal >> >> _______________________________________________ >> freebsd-net@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-net >> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >> > > > > -- > -----------------------------------------+------------------------------- > Prof. Luigi RIZZO, rizzo@iet.unipi.it . Dip. di Ing. dell'Informazione > http://www.iet.unipi.it/~luigi/ . Universita` di Pisa > TEL +39-050-2211611 . via Diotisalvi 2 > Mobile +39-338-6809875 . 56122 PISA (Italy) > -----------------------------------------+------------------------------- > --=20 Ermal