Date: Sun, 30 Mar 2008 10:53:33 -0700 From: Sam Leffler <sam@freebsd.org> To: Eugene Grosbein <eugen@kuzbass.ru> Cc: FreeBSD-Net mailing list <freebsd-net@freebsd.org>, Remko Lodder <remko@elvandar.org>, Brooks Davis <brooks@freebsd.org>, "Bruce M. Simpson" <bms@freebsd.org>, Miroslav Lachman <000.fbsd@quip.cz> Subject: Re: 7.0 - ifconfig create is not working as expected? Message-ID: <47EFD39D.8030107@freebsd.org> In-Reply-To: <47EFD222.2050008@freebsd.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?47EFD39D.8030107>