From owner-freebsd-net@FreeBSD.ORG Sun Mar 30 17:53:40 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 4F3DB106566C; Sun, 30 Mar 2008 17:53:40 +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 0405A8FC17; Sun, 30 Mar 2008 17:53:39 +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 m2UHrXHu093599 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 30 Mar 2008 10:53:33 -0700 (PDT) (envelope-from sam@freebsd.org) Message-ID: <47EFD39D.8030107@freebsd.org> Date: Sun, 30 Mar 2008 10:53:33 -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> <47EFD222.2050008@freebsd.org> In-Reply-To: <47EFD222.2050008@freebsd.org> 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:53:40 -0000 Sam Leffler wrote: > 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. It might be simpler to just do 1 pass over the args and push the clone callback on the first non-clone parameter. Right now however there's no way to tell what is clone-related and what is not. Sam