Bug 158171 - [patch] Modify rc scripts for ftp-proxy and pflog to start multiple instances
Summary: [patch] Modify rc scripts for ftp-proxy and pflog to start multiple instances
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 8.2-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Josh Paetzel
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-22 18:10 UTC by tom
Modified: 2015-04-05 02:54 UTC (History)
3 users (show)

See Also:


Attachments
patch.diff (6.24 KB, patch)
2011-06-22 18:10 UTC, tom
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description tom 2011-06-22 18:10:10 UTC
	The ftp-proxy and pflogd daemons both allow for multiple instances to be run simultaneously. However, the rc scripts that control the startup of these daemons have no concept of multiple instances.

	ftp-proxy requires a seperate instance for each "type" of proxying that you would like to perform. For example, two instances are required to allow operation through NAT for a group of internal servers connecting out, and an external server connecting inbound using active-mode FTP.

	pf allows logging to different target pflog interfaces, for more granular logging of traffic. This requires multiple pflogd instances, one to read each pflog interface.

Fix: The attached patch allows both startup scripts to handle multiple instances in a similar manner as the "static_routes" variable. If the optional <service>_instances list is defined in rc.conf, the scripts search for corresponding configuration for each "instance" listed in the _instances string. I have endeavored to maintain the previous functionality of the rc scripts, since it is likely that most users will not need the added functionality (for pflog in particular). My patch also includes updates to rc.conf(5) explaining the function of variables I have added. The patches below are run against HEAD, updated shortly before this PR.
Comment 1 Josh Paetzel freebsd_committer freebsd_triage 2011-07-01 15:27:52 UTC
Responsible Changed
From-To: freebsd-bugs->jpaetzel

I'll take it.
Comment 2 rainer 2012-07-13 15:01:52 UTC
Hi,

what's the status of this RFE?



Will this be included at some point?
Comment 3 tom 2012-07-13 15:50:50 UTC
It was kicked back to me with suggested changes, and it has sat in my
inbox, flagged "Priority" ever since. We're using it in production as-is.
But if there is interest, I can try to make it an actual priority.

-- 
Thomas Johnson
ClaimLynx, Inc.
952-593-5969 x2302
Comment 4 rainer 2012-07-16 09:17:09 UTC
Hi,


I think it would be good to have this in the base tree.
I might need it in production too and I really hate having "special"
systems around that can't be upgraded straightforward.
I *do* have enough of these already ;-)

It's probably too late to have in included for 9.1, and I need it on
8.3 anyway...
Comment 5 jeremy.phillippe 2013-01-24 22:58:35 UTC
This also appears to be an updated solution to the same sort of fix as suggested in PR 143085

I too would love to see this included, as I have need of this as well.
Comment 6 c.vasadi 2013-04-26 10:43:38 UTC
I second that.
Comment 7 c.vasadi 2013-04-26 14:54:50 UTC
PS: Tested and works under 9.1-STABLE r246451.
Comment 8 Eric F Crist 2013-11-12 20:22:40 UTC
Requesting this gets committed as-is to the tree.




-- 
This e-mail and any files transmitted with it are confidential and are 
intended solely for the use of the individual or entity to whom they are 
addressed. If you are not the intended recipient or the individual 
responsible for delivering the e-mail to the intended recipient, please be 
advised that you have received this e-mail in error and that any use, 
dissemination, forwarding, printing, or copying of this e-mail is strictly 
prohibited.

If you have received this communication in error, please return it to the 
sender immediately and delete the original message and any copy of it from 
your computer system. If you have any questions concerning this message, 
please contact the sender or call ClaimLynx at (952) 593-5969.
Comment 9 dfilter service freebsd_committer freebsd_triage 2013-11-13 03:50:39 UTC
Author: jpaetzel
Date: Wed Nov 13 03:50:31 2013
New Revision: 258080
URL: http://svnweb.freebsd.org/changeset/base/258080

Log:
  Add support for multiple instances of ftp-proxy
  and pflog devices.
  
  PR:	conf/158171
  Submitted by:	Thomas Johnson <tom@claimlynx.com>

Modified:
  head/etc/rc.d/ftp-proxy
  head/etc/rc.d/pflog
  head/share/man/man5/rc.conf.5

Modified: head/etc/rc.d/ftp-proxy
==============================================================================
--- head/etc/rc.d/ftp-proxy	Wed Nov 13 01:51:40 2013	(r258079)
+++ head/etc/rc.d/ftp-proxy	Wed Nov 13 03:50:31 2013	(r258080)
@@ -14,4 +14,62 @@ rcvar="ftpproxy_enable"
 command="/usr/sbin/ftp-proxy"
 
 load_rc_config $name
-run_rc_command "$1"
+
+#
+# manage_pid argument
+#	Create or remove a pidfile manually, for daemons that can't be bothered
+#	to do it themselves. Takes one argument, which is the argument provided
+#	to the rc script. The pidfile will be named /var/run/<$name>.pid,
+#	unless $pidfile is defined.
+#
+#	The method used to determine the pid is rather hacky; grep ps output to
+#	find '$procname|$command', then grep for ${name}_flags. If at all
+#	possible, use another method if at all possible, to avoid that dirty-
+#	code feeling.
+#
+manage_pid() {
+	local search_string ps_pid
+	case $1 in
+		*start)
+			cmd_string=`basename ${procname:-${command}}`
+			eval flag_string=\"\$${name}_flags\"
+			# Determine the pid.
+			ps_pid=`ps ax -o pid= -o command= | grep $cmd_string | grep -e "$flag_string" | grep -v grep | awk '{ print $1 }'`
+			# Write the pidfile depending on $pidfile status.
+			echo $ps_pid > ${pidfile:-"/var/run/$name.pid"}
+	       		;;
+		stop)
+	       		rm $pidfile
+	       		;;
+	esac
+}
+
+# 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="<arguments for foo>"
+#	ftpproxy_bar="<arguments for bar>"
+#
+# Starting more than one ftp-proxy?
+if [ "$ftpproxy_instances" ] && [ -n "${ftpproxy_instances}" ]; then
+	# Iterate through instance list.
+	for i in $ftpproxy_instances; do
+		#eval ftpproxy_${i}_flags=\$ftpproxy_${i}
+		#eval name=ftpproxy_${i}
+		# 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"
+		manage_pid $1
+	done
+else
+	# Traditional single-instance behavior
+	run_rc_command "$1"
+fi

Modified: head/etc/rc.d/pflog
==============================================================================
--- head/etc/rc.d/pflog	Wed Nov 13 01:51:40 2013	(r258079)
+++ head/etc/rc.d/pflog	Wed Nov 13 03:50:31 2013	(r258080)
@@ -24,25 +24,41 @@ pflog_prestart()
 {
 	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 [ "$pflog_instances" ] && [ -n "$pflog_instances" ]; then
+		rm $pidfile
+	fi
+
 	return 0
 }
 
@@ -53,4 +69,33 @@ pflog_resync()
 }
 
 load_rc_config $name
-run_rc_command "$1"
+
+# Check if spawning multiple pflogd
+echo "Starting pflogd: $pflog_instances"
+if [ "$pflog_instances" ] && [ -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

Modified: head/share/man/man5/rc.conf.5
==============================================================================
--- head/share/man/man5/rc.conf.5	Wed Nov 13 01:51:40 2013	(r258079)
+++ head/share/man/man5/rc.conf.5	Wed Nov 13 03:50:31 2013	(r258080)
@@ -917,6 +917,33 @@ Empty by default.
 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-seperated
+.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
@@ -935,6 +962,19 @@ Empty by default.
 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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 10 Josh Paetzel freebsd_committer freebsd_triage 2013-11-13 03:50:54 UTC
State Changed
From-To: open->closed

Committed, thanks!
Comment 11 js 2014-10-23 00:55:49 UTC
Can we get this merged into stable/10 please?
Comment 12 commit-hook freebsd_committer freebsd_triage 2014-11-09 20:09:23 UTC
A commit references this bug:

Author: jpaetzel
Date: Sun Nov  9 20:08:44 UTC 2014
New revision: 274327
URL: https://svnweb.freebsd.org/changeset/base/274327

Log:
  MFC: 258080

  Add support for multiple instances of ftp-proxy
  and pflog devices.

  PR:     conf/158171
  Submitted by:   Thomas Johnson <tom@claimlynx.com>

Changes:
_U  stable/10/
  stable/10/etc/rc.d/ftp-proxy
  stable/10/etc/rc.d/pflog
  stable/10/share/man/man5/rc.conf.5
Comment 13 Jason Unovitch freebsd_committer freebsd_triage 2015-04-05 02:54:39 UTC
Folks,

There are some semantic improvements to this in PR 199150.  This addresses /etc/rc.d/pflog always printing "Starting pflog".  This is both odd, for one, and breaks Puppet based off Puppet looking for specific output when 'service pflog rcvar' is checked.  I'm posting my test cases that show the expected output from the various `service pflog <command>` commands there but would appreciate any sanity checks and testing.  The details, patch, and full script are all available in https://bugs.freebsd.org/199150.

Josh,

It would be good to see the above mentioned PR closed before 10.2-RELEASE and 11.0-RELEASE.  I wouldn't want to see anyone else managing systems with a configuration management tool to see the issue I observed with Puppet.

-Jason