conf/104884: Add support EtherChannel configuration to rc.conf

[Florent Thoumie]

Norikatsu Shigemura wrote:
> On Sat, 28 Oct 2006 16:10:18 GMT
> FreeBSD-gnats-submit at FreeBSD.org wrote:
>> Thank you very much for your problem report.
>> It has the internal identification `conf/104884'.
>> The individual assigned to look at your
>> report is: freebsd-bugs. 
>> You can access the state of your problem report at any time
>> via this link:
>> http://www.freebsd.org/cgi/query-pr.cgi?pr=104884
>>> Category:       conf
>>> Responsible:    freebsd-bugs
>>> Synopsis:       Add support EtherChannel configuration to rc.conf
>>> Arrival-Date:   Sat Oct 28 16:10:18 GMT 2006
> 
> 	I chased HEAD.  Please see following patch.
> 	Anyone, please handle this PR?
> 	And I'll make a patch for 6-stable.

I'm sorry, I meant to answer but forgot about it. I don't know much
about technical details, so I'll only focus on style. Here are my
comments on the patch:

> Index: network.subr
> ===================================================================
> RCS file: /home/ncvs/src/etc/network.subr,v
> retrieving revision 1.176
> diff -u -r1.176 network.subr
> --- network.subr	29 Oct 2006 13:29:49 -0000	1.176
> +++ network.subr	28 Jan 2007 14:52:36 -0000
> @@ -907,3 +907,78 @@
>  		esac
>  	done
>  }
> +
> +ng_mkpeer() {
> +	ngctl -f - 2> /dev/null <<EOF
> +mkpeer $*
> +msg dummy nodeinfo
> +EOF
> +}
> +
> +ng_create_one() {
> +	ng_mkpeer $* | while read line; do
> +		t=`expr "${line}" : '.* name="\([a-z]*[0-9]*\)" .*'`
> +		if [ -n "${t}" ]; then
> +			echo ${t}
> +			return
> +		fi
> +	done
> +}
> +
> +ng_fec_create() {
> +	local req_iface iface bogus
> +	req_iface="$1"
> +
> +	if [ -z "${req_iface}" ]; then

Why are you testing this? It's only called in fec_up() and can't be
called with a empty argument. Or do you want to "export" the function to
other scripts?

> +		iface=`ng_create_one fec dummy fec`
> +		if [ -z "${iface}" ]; then
> +			exit 2
> +		fi
> +		echo ${iface}
> +		return
> +	fi
> +
> +	ngctl shutdown ${req_iface}: > /dev/null 2>&1
> +
> +	bogus=""
> +	while true; do
> +		iface=`ng_create_one fec dummy fec`
> +		if [ -z "${iface}" ]; then
> +			exit 2
> +		fi
> +		if [ "${iface}" = "${req_iface}" ]; then
> +			echo ${iface}
> +			break
> +		fi
> +		bogus="${bogus} ${iface}"
> +	done
> +
> +	for iface in ${bogus}; do
> +		ngctl shutdown ${iface}:
> +	done

These loops are a bit confusing. If I understand correctly, you're
creating interfaces until they reach the right number and then you
delete all the ones which have been created unnecessarily? Could it be
that iface is higher than req_iface (which would loop undefinitely)?

> +}
> +
> +# fec_up ifn
> +# Configure Fast EtherChannel for interface $ifn. Returns 0 if FEC
> +# arguments were found and configured; returns 1 otherwise.
> +fec_up() {
> +	case ${fec_interfaces} in
> +	[Nn][Oo] | '')
> +		;;

What's the point of this? The 'case' seems useless to me. Just got with
the 'for' loop. If it's an empty list, then it just won't do anything.
Default has to be '' and not 'NO' (but it seems more sensible anyway).

> +	*)
> +		for i in ${fec_interfaces}; do
> +			ng_fec_create $i
> +			for j in `get_if_var $i fecconfig_IF`; do
> +				case ${j} in
> +				'')
> +					continue
> +					;;
> +				*)
> +					ngctl msg ${i}: add_iface "\"${j}\""
> +					;;
> +				esac
> +			done
> +		done
> +		;;
> +	esac
> +}
> Index: defaults/rc.conf
> ===================================================================
> RCS file: /home/ncvs/src/etc/defaults/rc.conf,v
> retrieving revision 1.303
> diff -u -r1.303 rc.conf
> --- defaults/rc.conf	20 Jan 2007 04:24:19 -0000	1.303
> +++ defaults/rc.conf	28 Jan 2007 14:52:36 -0000
> @@ -183,6 +183,10 @@
>  				# Choose correct tunnel addrs.
>  #gifconfig_gif0="10.1.1.1 10.1.2.1"	# Examples typically for a router.
>  #gifconfig_gif1="10.1.1.2 10.1.2.2"	# Examples typically for a router.
> +fec_interfaces="NO"		# List of Fast EtherChannels (or "NO")

Set to '' instead of 'NO' as explained above.

-- 
Florent Thoumie
flz at FreeBSD.org
FreeBSD Committer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 250 bytes
Desc: OpenPGP digital signature
Url : http://lists.freebsd.org/pipermail/freebsd-bugs/attachments/20070128/153fc1f5/signature.pgp