From owner-freebsd-net@FreeBSD.ORG Sun Mar 30 17:47:26 2008 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 26AE41065672; Sun, 30 Mar 2008 17:47:26 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from ebb.errno.com (ebb.errno.com [69.12.149.25]) by mx1.freebsd.org (Postfix) with ESMTP id D2D448FC1F; Sun, 30 Mar 2008 17:47:25 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from trouble.errno.com (trouble.errno.com [10.0.0.248]) (authenticated bits=0) by ebb.errno.com (8.13.6/8.12.6) with ESMTP id m2UHlEPn093563 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 30 Mar 2008 10:47:15 -0700 (PDT) (envelope-from sam@freebsd.org) Message-ID: <47EFD222.2050008@freebsd.org> Date: Sun, 30 Mar 2008 10:47:14 -0700 From: Sam Leffler Organization: FreeBSD Project User-Agent: Thunderbird 2.0.0.9 (X11/20071125) MIME-Version: 1.0 To: Eugene Grosbein References: <47EE42C8.3070100@quip.cz> <20080329204344.GA66910@lor.one-eyed-alien.net> <20080330072137.GA35435@svzserv.kemerovo.su> <47EF69F0.1050304@FreeBSD.org> <20080330104525.GA57135@svzserv.kemerovo.su> <9C5282E0-B44F-4A07-A606-1783D7725B5A@elvandar.org> <20080330142951.GA80768@svzserv.kemerovo.su> In-Reply-To: <20080330142951.GA80768@svzserv.kemerovo.su> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-DCC-Misty-Metrics: ebb.errno.com; whitelist Cc: FreeBSD-Net mailing list , Remko Lodder , Brooks Davis , "Bruce M. Simpson" , Miroslav Lachman <000.fbsd@quip.cz> Subject: Re: 7.0 - ifconfig create is not working as expected? X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 30 Mar 2008 17:47:26 -0000 Eugene Grosbein wrote: > On Sun, Mar 30, 2008 at 01:14:36PM +0200, Remko Lodder wrote: > > >> Given that the idea is that we dont expect to get to this anytime >> soon, we welcome the person who does the analysis for us so that we >> might be able to fix this quicker (if possible with all the changes >> involved). >> > > Here is a patch for RELENG_7. I ask Miroslav Lachman to test it. > Apply: > > cd /usr/src/sbin/ifconfig > patch < /path/to/patchfile > make > > Test: > > ./ifconfig lo1 create inet 5.5.5.5 netmask 255.255.255.0 > > Or full-blown syntax: > > ./ifconfig gif0 create inet 6.6.6.6 7.7.7.7 tunnel 1.1.1.1 2.2.2.2 \ > netmask 255.255.255.255 mtu 1500 link2 > > Index: ifclone.c > =================================================================== > RCS file: /home/ncvs/src/sbin/ifconfig/ifclone.c,v > retrieving revision 1.3 > diff -u -r1.3 ifclone.c > --- ifclone.c 12 Aug 2006 18:07:17 -0000 1.3 > +++ ifclone.c 30 Mar 2008 14:19:08 -0000 > @@ -131,7 +131,9 @@ > static > DECL_CMD_FUNC(clone_create, arg, d) > { > - callback_register(ifclonecreate, NULL); > + if (strstr(name, "vlan") == name) > + callback_register(ifclonecreate, NULL); > + else ifclonecreate(s, NULL); > This breaks other cloning operations (e.g. wlan vaps that are about to show up in HEAD). In general it is wrong to embed knowledge about one type of cloning op in the common clone code. If you want to add the notion of cloning operations that should be done immediately vs. ones that should be deferred then do it generically; not by hacks like this. Understand however that now !vlan clone operations behave differently than vlans and many people will be utterly confused by the inconsistency. > } > > static > Index: ifconfig.c > =================================================================== > RCS file: /home/ncvs/src/sbin/ifconfig/ifconfig.c,v > retrieving revision 1.134 > diff -u -r1.134 ifconfig.c > --- ifconfig.c 4 Oct 2007 09:45:41 -0000 1.134 > +++ ifconfig.c 30 Mar 2008 14:22:00 -0000 > @@ -247,7 +247,12 @@ > if (iflen >= sizeof(name)) > errx(1, "%s: cloning name too long", > ifname); > - ifconfig(argc, argv, NULL); > + if (argc > 1) { > + afp = af_getbyname(argv[1]); > + if (afp != NULL) > + argv[1] = NULL; > + } > + ifconfig(argc, argv, afp); > exit(0); > } > errx(1, "interface %s does not exist", ifname); > @@ -451,6 +456,9 @@ > while (argc > 0) { > const struct cmd *p; > > + if(*argv == NULL) > + goto next; > + > p = cmd_lookup(*argv); > if (p == NULL) { > /* > @@ -479,6 +487,7 @@ > } else > p->c_u.c_func(*argv, p->c_parameter, s, afp); > } > + next: > argc--, argv++; > } > Aside from not maintaining prevailing style and breaking cloning of other devices you seem to understand the issue. How to handle it is however unclear. I considered making 2 passes over the arguments to collect those required for a clone operation but never got to it. That still seems like the correct approach. Sam