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

Mon Jan 29 17:50:27 UTC 2007

The following reply was made to PR conf/104884; it has been noted by GNATS.

From: Florent Thoumie <flz at FreeBSD.org>
To: Norikatsu Shigemura <nork at FreeBSD.org>
Cc: freebsd-rc at FreeBSD.org,  bug-followup at FreeBSD.org
Subject: Re: conf/104884: Add support EtherChannel configuration to rc.conf
Date: Mon, 29 Jan 2007 17:15:51 +0000

 This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
 --------------enig3C1315AA345CB877ABCD4D34
 Content-Type: text/plain; charset=ISO-8859-15
 Content-Transfer-Encoding: quoted-printable

 Norikatsu Shigemura wrote:
 > The following reply was made to PR conf/104884; it has been noted by GN=
 ATS.
 >=20
 > From: Norikatsu Shigemura <nork at FreeBSD.org>
 > To: Florent Thoumie <flz at FreeBSD.org>
 > Cc: Norikatsu Shigemura <nork at FreeBSD.org>, freebsd-bugs at FreeBSD.org,
 >         FreeBSD-gnats-submit at FreeBSD.org, freebsd-rc at FreeBSD.org
 > Subject: Re: conf/104884: Add support EtherChannel configuration to rc.=
 conf
 > Date: Tue, 30 Jan 2007 01:53:17 +0900
 >=20
 >  On Sun, 28 Jan 2007 19:37:03 +0000
 >  Florent Thoumie <flz at freebsd.org> wrote:
 >  > >> http://www.freebsd.org/cgi/query-pr.cgi?pr=3D104884
 >  > >>> Category:       conf
 >  > >>> Responsible:    freebsd-bugs
 >  > >>> Synopsis:       Add support EtherChannel configuration to rc.con=
 f
 >  > >>> 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
 > =20
 >  	Thanks for your handling.
 > =20
 >  > about technical details, so I'll only focus on style. Here are my
 >  > comments on the patch:
 > =20
 >  	I think that the answer of all questions is 'BECAUSE netgraph's
 >  	specification.'. :-)

 =2E.. which I don't really know, so better not take everything I write as=

 face value :-)

 >  > > Index: network.subr
 >  > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 >  > > 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=3D`expr "${line}" : '.* name=3D"\([a-z]*[0-9]*\)" .*'`
 >  > > +		if [ -n "${t}" ]; then
 >  > > +			echo ${t}
 >  > > +			return
 >  > > +		fi
 >  > > +	done
 >  > > +}
 > =20
 >  	I implemented ng_mkpeer and ng_create_one as generic functions.
 >  	If anyone want to add other netgraph function, they can use
 >  	these functions.

 I wasn't commenting this one, seemed alright to me.

 >  > > +ng_fec_create() {
 >  > > +	local req_iface iface bogus
 >  > > +	req_iface=3D"$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 functio=
 n to
 >  > other scripts?
 > =20
 >  	Ah! This code's meaning was changed from original code.  Yes,
 >  	I think that this code should be removed.
 > =20
 >  > > +	ngctl shutdown ${req_iface}: > /dev/null 2>&1
 >  > > +	bogus=3D""
 >  > > +	while true; do
 >  > > +		iface=3D`ng_create_one fec dummy fec`
 >  > > +		if [ -z "${iface}" ]; then
 >  > > +			exit 2
 >  > > +		fi
 >  > > +		if [ "${iface}" =3D "${req_iface}" ]; then
 >  > > +			echo ${iface}
 >  > > +			break
 >  > > +		fi
 >  > > +		bogus=3D"${bogus} ${iface}"
 >  > > +	done
 >  > > +	for iface in ${bogus}; do
 >  > > +		ngctl shutdown ${iface}:
 >  > > +	done
 >  >=20
 >  > 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
 > =20
 >  	Your understanding is right:-).  But we cannot control unit number
 >  	in 'ngctl mkpeer', because 'Find the first free unit number for a
 >  	new interface' strategy in sys/netgraph/ng_fec.c#ng_fec_get_unit.
 > =20
 >  > that iface is higher than req_iface (which would loop undefinitely)?=

 > =20
 >  	Previously, I removed req_iface by ngctl shutdown.  So not reache
 >  	infinity:-).

 Fair enough.

 >  > > +# fec_up ifn
 >  > > +# Configure Fast EtherChannel for interface $ifn. Returns 0 if FE=
 C
 >  > > +# 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 w=
 ith
 >  > the 'for' loop. If it's an empty list, then it just won't do anythin=
 g.
 >  > Default has to be '' and not 'NO' (but it seems more sensible anyway=
 ).
 > =20
 >  	I obtained gif_up code.  I don't know why/where are problem in it.

 No problem, just seemed bad style to me :-)

 --=20
 Florent Thoumie
 flz at FreeBSD.org
 FreeBSD Committer

 --------------enig3C1315AA345CB877ABCD4D34
 Content-Type: application/pgp-signature; name="signature.asc"
 Content-Description: OpenPGP digital signature
 Content-Disposition: attachment; filename="signature.asc"

 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.5 (FreeBSD)
 Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

 iD8DBQFFvivMMxEkbVFH3PQRCny3AJ4xgCVFGTFMEKVlG9xIxSpVZHbCywCeOhSh
 sod+QxcYxMjAkupUSK7ph/o=
 =5W2+
 -----END PGP SIGNATURE-----

 --------------enig3C1315AA345CB877ABCD4D34--