Sam Leffler wrote:
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.
I think the attached patch against HEAD DTRT. Should work on RELENG_7 too.
Sam
Index: ifconfig.c
===================================================================
RCS file: /usr/ncvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.135
diff -u -r1.135 ifconfig.c
--- ifconfig.c 10 Dec 2007 02:31:00 -0000 1.135
+++ ifconfig.c 30 Mar 2008 19:29:39 -0000
@@ -93,7 +93,8 @@
int supmedia = 0;
int printkeys = 0; /* Print keying material for interfaces. */
-static int ifconfig(int argc, char *const *argv, const struct afswtch *afp);
+static int ifconfig(int argc, char *const *argv, int iscreate,
+ const struct afswtch *afp);
static void status(const struct afswtch *afp, const struct sockaddr_dl *sdl,
struct ifaddrs *ifa);
static void tunnel_status(int s);
@@ -247,7 +248,7 @@
if (iflen >= sizeof(name))
errx(1, "%s: cloning name too long",
ifname);
- ifconfig(argc, argv, NULL);
+ ifconfig(argc, argv, 1, NULL);
exit(0);
}
errx(1, "interface %s does not exist", ifname);
@@ -305,7 +306,7 @@
}
if (argc > 0)
- ifconfig(argc, argv, afp);
+ ifconfig(argc, argv, 0, afp);
else
status(afp, sdl, ifa);
}
@@ -433,17 +434,19 @@
DEF_CMD("ifdstaddr", 0, setifdstaddr);
static int
-ifconfig(int argc, char *const *argv, const struct afswtch *afp)
+ifconfig(int argc, char *const *argv, int iscreate, const struct afswtch *afp)
{
+ const struct afswtch *nafp;
struct callback *cb;
int s;
+ strncpy(ifr.ifr_name, name, sizeof ifr.ifr_name);
+top:
if (afp == NULL)
afp = af_getbyname("inet");
ifr.ifr_addr.sa_family =
afp->af_af == AF_LINK || afp->af_af == AF_UNSPEC ?
AF_INET : afp->af_af;
- strncpy(ifr.ifr_name, name, sizeof ifr.ifr_name);
if ((s = socket(ifr.ifr_addr.sa_family, SOCK_DGRAM, 0)) < 0)
err(1, "socket(family %u,SOCK_DGRAM", ifr.ifr_addr.sa_family);
@@ -460,6 +463,32 @@
p = (setaddr ? &setifdstaddr_cmd : &setifaddr_cmd);
}
if (p->c_u.c_func || p->c_u.c_func2) {
+ if (iscreate && !p->c_iscloneop) {
+ /*
+ * Push the clone create callback so the new
+ * device is created and can be used for any
+ * remaining arguments.
+ */
+ cb = callbacks;
+ if (cb == NULL)
+ errx(1, "internal error, no callback");
+ callbacks = cb->cb_next;
+ cb->cb_func(s, cb->cb_arg);
+ /*
+ * Handle any address family spec that
+ * immediately follows and potentially
+ * recreate the socket.
+ */
+ nafp = af_getbyname(*argv);
+ if (nafp != NULL) {
+ argc--, argv++;
+ if (nafp != afp) {
+ close(s);
+ afp = nafp;
+ goto top;
+ }
+ }
+ }
if (p->c_parameter == NEXTARG) {
if (argv[1] == NULL)
errx(1, "'%s' requires argument",
Index: ifconfig.h
===================================================================
RCS file: /usr/ncvs/src/sbin/ifconfig/ifconfig.h,v
retrieving revision 1.21
diff -u -r1.21 ifconfig.h
--- ifconfig.h 13 Jun 2007 18:07:59 -0000 1.21
+++ ifconfig.h 30 Mar 2008 19:38:31 -0000
@@ -52,6 +52,7 @@
c_func *c_func;
c_func2 *c_func2;
} c_u;
+ int c_iscloneop;
struct cmd *c_next;
};
void cmd_register(struct cmd *);
@@ -71,6 +72,8 @@
#define DEF_CMD_ARG(name, func) { name, NEXTARG, { .c_func =
func } }
#define DEF_CMD_OPTARG(name, func) { name, OPTARG, { .c_func =
func } }
#define DEF_CMD_ARG2(name, func) { name, NEXTARG2, { .c_func2 =
func } }
+#define DEF_CLONE_CMD(name, param, func) { name, param, { .c_func =
func }, 1 }
+#define DEF_CLONE_CMD_ARG(name, func) { name, NEXTARG, { .c_func =
func }, 1 }
struct ifaddrs;
struct addrinfo;
Index: ifvlan.c
===================================================================
RCS file: /usr/ncvs/src/sbin/ifconfig/ifvlan.c,v
retrieving revision 1.12
diff -u -r1.12 ifvlan.c
--- ifvlan.c 9 Jul 2006 06:10:23 -0000 1.12
+++ ifvlan.c 30 Mar 2008 19:25:26 -0000
@@ -172,8 +172,8 @@
}
static struct cmd vlan_cmds[] = {
- DEF_CMD_ARG("vlan", setvlantag),
- DEF_CMD_ARG("vlandev", setvlandev),
+ DEF_CLONE_CMD_ARG("vlan", setvlantag),
+ DEF_CLONE_CMD_ARG("vlandev", setvlandev),
/* XXX For compatibility. Should become DEF_CMD() some day. */
DEF_CMD_OPTARG("-vlandev", unsetvlandev),
DEF_CMD("vlanmtu", IFCAP_VLAN_MTU, setifcap),
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
