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

_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to