From owner-freebsd-rc@FreeBSD.ORG Sat Feb 11 19:51:25 2012 Return-Path: Delivered-To: freebsd-rc@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3EE14106564A for ; Sat, 11 Feb 2012 19:51:25 +0000 (UTC) (envelope-from jpaetzel@freebsd.org) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by mx1.freebsd.org (Postfix) with ESMTP id 0155A8FC08 for ; Sat, 11 Feb 2012 19:51:24 +0000 (UTC) Received: from compute5.internal (compute5.nyi.mail.srv.osa [10.202.2.45]) by gateway1.nyi.mail.srv.osa (Postfix) with ESMTP id 805D120AA4 for ; Sat, 11 Feb 2012 14:32:59 -0500 (EST) Received: from frontend1.nyi.mail.srv.osa ([10.202.2.160]) by compute5.internal (MEProxy); Sat, 11 Feb 2012 14:32:59 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type; s=smtpout; bh=AIFe DRlCfCERJL03R4ut0EGhVGE=; b=YbQHgba1aSC0+nDAs8QIg0QREH+3UKanb0F+ gPo71Rcyz8ROx+LKN0yvoiTE/5HnRCQgSOOZPiUDiitLUrSpq0+KlkgTRRj/zenS BRNLb2s3wsjJibcgtS5L30RJmcBW2v6R8b7+5wiQRZHebD+s/YsNYyh4U2OSdLq7 1Iskm94= X-Sasl-enc: /Bu8vGZY7yZYOJQ+KYsDsypZjc9bjRPD0mVIkExoxxdI 1328988778 Received: from roadrash.tcbug.org (unknown [216.139.7.151]) by mail.messagingengine.com (Postfix) with ESMTPSA id A1E7B8E00D9; Sat, 11 Feb 2012 14:32:58 -0500 (EST) Message-ID: <4F36C269.9060201@freebsd.org> Date: Sat, 11 Feb 2012 11:32:57 -0800 From: Josh Paetzel Organization: FreeBSS User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20111227 Thunderbird/9.0 MIME-Version: 1.0 To: Jilles Tjoelker References: <4E0E1CFA.1080904@freebsd.org> <20110701205529.GA93981@stack.nl> In-Reply-To: <20110701205529.GA93981@stack.nl> X-Enigmail-Version: undefined Content-Type: multipart/mixed; boundary="------------030106020900080405000307" Cc: freebsd-rc@freebsd.org Subject: Re: Fwd: Commit approval requested X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 11 Feb 2012 19:51:25 -0000 This is a multi-part message in MIME format. --------------030106020900080405000307 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 07/01/2011 13:55, Jilles Tjoelker wrote: > On Fri, Jul 01, 2011 at 02:16:10PM -0500, Josh Paezel wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 > >> This patch is in production at an organization which uses both single >> and multiple pflog devices on a range of devices. (eg: it doesn't break >> current configurations) > >> It allows multiple pflog devices as well as multiple ftp-proxy instances. > >> The patch was submitted as a PR conf/158181 > >> I've applied the patch to a HEAD svn co and regenerated the patch with >> svn diff from that. > > Comments are inline. Note that I have not tested the patch nor any > proposed changes. > I have applied the changes suggested and am pinging the submitter to give it a try, just mailing it here for a review as well to catch any glaring issues I've caused or missed. -- Thanks, Josh Paetzel FreeBSD -- The power to serve --------------030106020900080405000307 Content-Type: text/plain; name="pflog.txt" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pflog.txt" Index: share/man/man5/rc.conf.5 =================================================================== --- share/man/man5/rc.conf.5 (revision 231526) +++ share/man/man5/rc.conf.5 (working copy) @@ -880,6 +880,33 @@ This variable contains additional flags passed to the .Xr pflogd 8 program. +.It Va pflog_instances +.Pq Vt str +If logging to more than one +.Xr pflog 4 +interface is desired, +.Va pflog_instances +is set to the list of +.Xr pflogd 8 +instances that should be started at system boot time. If +.Va pflog_instances +is set, for each whitespace-separated +.Ar element +in the list, +.Ao Ar element Ac Ns Va _dev +and +.Ao Ar element Ac Ns Va _logfile +elements are assumed to exist. +.Ao Ar element Ac Ns Va _dev +must contain the +.Xr pflog 4 +interface to be watched by the named +.Xr pflogd 8 +instance. +.Ao Ar element Ac Ns Va _logfile +must contain the name of the logfile that will be used by the +.Xr pflogd 8 +instance. .It Va ftpproxy_enable .Pq Vt bool Set to @@ -898,6 +925,19 @@ This variable contains additional flags passed to the .Xr ftp-proxy 8 program. +.It Va ftpproxy_instances +.Pq Vt str +Empty by default. If multiple instances of +.Xr ftp-proxy 8 +are desired at boot time, +.Va ftpproxy_instances +should contain a whitespace-seperated list of instance names. For each +.Ar element +in the list, a variable named +.Ao Ar element Ac Ns Va _flags +should be defined, containing the command-line flags to be passed to the +.Xr ftp-proxy 8 +instance. .It Va pfsync_enable .Pq Vt bool Set to Index: etc/rc.d/ftp-proxy =================================================================== --- etc/rc.d/ftp-proxy (revision 231526) +++ etc/rc.d/ftp-proxy (working copy) @@ -12,6 +12,66 @@ name="ftpproxy" rcvar="ftpproxy_enable" command="/usr/sbin/ftp-proxy" +start_postcmd="ftp_proxy_poststart" +stop_postcmd="ftp_proxy_poststop" load_rc_config $name -run_rc_command "$1" + +ftp_proxy_poststart() { + local ps_pid + cmd_string=${procname:-${command}} + cmd_string=${cmd_string##*/} + eval flag_string=\"\$${name}_flags\" + # Determine the pid. + ps_pid=$(pgrep -f "$cmd_string.*$flag_string") + # Write the pidfile depending on $pidfile status. + echo $ps_pid > ${pidfile:-"/var/run/$name.pid"} +} + +ftp_proxy_poststop() { + rm ${pidfile:-"/var/run/$name.pid"} +} + +# Allow ftp-proxy to start up in two different ways. The typical behavior +# is to start up one instance of ftp-proxy by setting ftpproxy_enable and +# ftpproxy_flags. The alternate behavior allows multiple instances of ftp- +# proxy to be started, allowing different types of proxy behavior. To use the +# new behavior, a list of instances must be defined, and a list of flags for +# each instance. For example, if we want to start two instances of ftp-proxy, +# foo and bar, we would set the following vars. +# ftpproxy_enable="YES" +# ftpproxy_instances="foo bar" +# ftpproxy_foo="" +# ftpproxy_bar="" +# +# Starting more than one ftp-proxy? +if [ -n "${ftpproxy_instances}" ]; then + # Iterate through instance list. + for i in $ftpproxy_instances; do + # Set flags for this instance. + eval ftpproxy_flags=\$ftpproxy_${i} + # Define a unique pid file name. + pidfile="/var/run/ftp-proxy.$i.pid" + run_rc_command "$1" + ftp_proxy_poststart + done +else + # Traditional single-instance behavior + run_rc_command "$1" +fi + +# Stopping more than one ftp-proxy? +if [ -n "${ftpproxy_instances}" ]; then + # Iterate through instance list. + for i in $ftpproxy_instances; do + # Set flags for this instance. + eval ftpproxy_flags=\$ftpproxy_${i} + # Define a unique pid file name. + pidfile="/var/run/ftp-proxy.$i.pid" + run_rc_command "$1" + ftp_proxy_poststop + done +else + # Traditional single-instance behavior + run_rc_command "$1" +fi Index: etc/rc.d/pflog =================================================================== --- etc/rc.d/pflog (revision 231526) +++ etc/rc.d/pflog (working copy) @@ -24,25 +24,41 @@ { load_kld pflog || return 1 - # set pflog0 interface to up state - if ! ifconfig pflog0 up; then - warn 'could not bring up pflog0.' + # set pflog_dev interface to up state + if ! ifconfig $pflog_dev up; then + warn "could not bring up $pflog_dev." return 1 fi # prepare the command line for pflogd - rc_flags="-f $pflog_logfile $rc_flags" + rc_flags="-f $pflog_logfile -i $pflog_dev $rc_flags" # report we're ready to run pflogd return 0 } +pflog_poststart() { + # Allow child pflogd to settle + sleep 0.10 + # More elegant(?) method for getting a unique pid + if [ -f /var/run/pflogd.pid ]; then + mv /var/run/pflogd.pid $pidfile + else + warn "/var/run/pflogd.pid does not exist. Too fast." + fi +} + pflog_poststop() { - if ! ifconfig pflog0 down; then - warn 'could not bring down pflog0.' + if ! ifconfig $pflog_dev down; then + warn "could not bring down $pflog_dev." return 1 fi + + if [ -n "$pflog_instances" ]; then + rm $pidfile + fi + return 0 } @@ -53,4 +69,33 @@ } load_rc_config $name -run_rc_command "$1" + +# Check if spawning multiple pflogd +echo "Starting pflogd: $pflog_instances" +if [ -n "$pflog_instances" ]; then + start_postcmd="pflog_poststart" + # Interate through requested instances. + for i in $pflog_instances; do + # Set required variables + eval pflog_dev=\$pflog_${i}_dev + eval pflog_logfile=\$pflog_${i}_logfile + eval pflog_flags=\$pflog_${i}_flags + # Check that required vars have non-zero length, warn if not. + if [ -z "$pflog_dev" ]; then + warn "pflog_dev not set" + continue + fi + if [ -z "$pflog_logfile" ]; then + warn "pflog_logfile not set" + continue + fi + # pflogd sets a pidfile, but the name is hardcoded. Concoct a + # unique pidfile name. + pidfile="/var/run/pflogd.$i.pid" + run_rc_command "$1" + done +else + # Typical case, spawn single instance only. + pflog_dev=${pflog_dev:-"pflog0"} + run_rc_command "$1" +fi --------------030106020900080405000307--