Message ID | 1418252195-2612-1-git-send-email-vadim4j@gmail.com |
---|---|
State | Superseded, archived |
Delegated to: | stephen hemminger |
Headers | show |
On Thu, Dec 11, 2014 at 11:58:21AM +0100, Nicolas Dichtel wrote: > Le 10/12/2014 23:56, Vadim Kochan a écrit : > >From: Vadim Kochan <vadim4j@gmail.com> > > > >Added new '-ns' option to simplify executing following cmd: > > > > ip netns exec NETNS ip OPTIONS COMMAND OBJECT > > > > to > > > > ip -ns NETNS OPTIONS COMMAND OBJECT > > > >e.g.: > > > > ip -ns vnet0 link add br0 type bridge > > > >Signed-off-by: Vadim Kochan <vadim4j@gmail.com> > >--- > >May be new option should have better name than '-ns' ? > What about 'ip -netns' to be explicit like other options? > user may still use 'ip -n' at the end. > > > Regards, > Nicolas May be left '-n' for some other future option, but use the following options: -net[ns] and -ns ? What do you think ? Thanks, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 10/12/2014 23:56, Vadim Kochan a écrit : > From: Vadim Kochan <vadim4j@gmail.com> > > Added new '-ns' option to simplify executing following cmd: > > ip netns exec NETNS ip OPTIONS COMMAND OBJECT > > to > > ip -ns NETNS OPTIONS COMMAND OBJECT > > e.g.: > > ip -ns vnet0 link add br0 type bridge > > Signed-off-by: Vadim Kochan <vadim4j@gmail.com> > --- > May be new option should have better name than '-ns' ? What about 'ip -netns' to be explicit like other options? user may still use 'ip -n' at the end. Regards, Nicolas -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 11/12/2014 11:57, vadim4j@gmail.com a écrit : > On Thu, Dec 11, 2014 at 11:58:21AM +0100, Nicolas Dichtel wrote: >> Le 10/12/2014 23:56, Vadim Kochan a écrit : >>> From: Vadim Kochan <vadim4j@gmail.com> >>> >>> Added new '-ns' option to simplify executing following cmd: >>> >>> ip netns exec NETNS ip OPTIONS COMMAND OBJECT >>> >>> to >>> >>> ip -ns NETNS OPTIONS COMMAND OBJECT >>> >>> e.g.: >>> >>> ip -ns vnet0 link add br0 type bridge >>> >>> Signed-off-by: Vadim Kochan <vadim4j@gmail.com> >>> --- >>> May be new option should have better name than '-ns' ? >> What about 'ip -netns' to be explicit like other options? >> user may still use 'ip -n' at the end. >> >> >> Regards, >> Nicolas > > May be left '-n' for some other future option, but use the following Options parsing in iproute2 will match -netns when typing -n because there is no other options that begin with a 'n' (I've done a quick look, maybe I've missed one). Like -d which matches -details, etc. > options: -net[ns] and -ns ? What do you think ? One option is enough. '-netns' is an explicit reference to 'ip netns'. Regards, Nicolas -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 11, 2014 at 01:50:17PM +0100, Nicolas Dichtel wrote: > Le 11/12/2014 11:57, vadim4j@gmail.com a écrit : > >On Thu, Dec 11, 2014 at 11:58:21AM +0100, Nicolas Dichtel wrote: > >>Le 10/12/2014 23:56, Vadim Kochan a écrit : > >>>From: Vadim Kochan <vadim4j@gmail.com> > >>> > >>>Added new '-ns' option to simplify executing following cmd: > >>> > >>> ip netns exec NETNS ip OPTIONS COMMAND OBJECT > >>> > >>> to > >>> > >>> ip -ns NETNS OPTIONS COMMAND OBJECT > >>> > >>>e.g.: > >>> > >>> ip -ns vnet0 link add br0 type bridge > >>> > >>>Signed-off-by: Vadim Kochan <vadim4j@gmail.com> > >>>--- > >>>May be new option should have better name than '-ns' ? > >>What about 'ip -netns' to be explicit like other options? > >>user may still use 'ip -n' at the end. > >> > >> > >>Regards, > >>Nicolas > > > >May be left '-n' for some other future option, but use the following > Options parsing in iproute2 will match -netns when typing -n because there > is no other options that begin with a 'n' (I've done a quick look, maybe I've > missed one). > Like -d which matches -details, etc. > > >options: -net[ns] and -ns ? What do you think ? > One option is enough. '-netns' is an explicit reference to 'ip netns'. > > > Regards, > Nicolas OK, I agree. I will re-work and resend v2. Thanks, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 11 Dec 2014 00:56:35 +0200, Vadim Kochan wrote: > From: Vadim Kochan <vadim4j@gmail.com> > > Added new '-ns' option to simplify executing following cmd: > > ip netns exec NETNS ip OPTIONS COMMAND OBJECT > > to > > ip -ns NETNS OPTIONS COMMAND OBJECT > > e.g.: > > ip -ns vnet0 link add br0 type bridge This is great! It's a thing that has been bothering me for long time but never got high enough on my todo list. Thanks for working on this. However, > --- a/ip/ip.c > +++ b/ip/ip.c > @@ -262,6 +262,12 @@ int main(int argc, char **argv) > rcvbuf = size; > } else if (matches(opt, "-help") == 0) { > usage(); > + } else if (matches(opt, "-ns") == 0) { > + argc--; > + argv++; > + argv[0] = argv[1]; > + argv[1] = basename; > + return netns_exec(argc, argv); I very much dislike this. There's no reason to exec another ip binary. The main reason I wanted the -n (or whatever) option was to speed up execution of test scripts in environments with hundreds of interfaces in different net namespaces. Please just change to the specified netns and continue with interpreting of the rest of the command line, there's absolutely no reason for doing the exec. Thanks, Jiri
On Thu, Dec 11, 2014 at 05:09:28PM +0100, Jiri Benc wrote: > On Thu, 11 Dec 2014 00:56:35 +0200, Vadim Kochan wrote: > > From: Vadim Kochan <vadim4j@gmail.com> > > > > Added new '-ns' option to simplify executing following cmd: > > > > ip netns exec NETNS ip OPTIONS COMMAND OBJECT > > > > to > > > > ip -ns NETNS OPTIONS COMMAND OBJECT > > > > e.g.: > > > > ip -ns vnet0 link add br0 type bridge > > This is great! It's a thing that has been bothering me for long time > but never got high enough on my todo list. Thanks for working on this. > > However, > > > --- a/ip/ip.c > > +++ b/ip/ip.c > > @@ -262,6 +262,12 @@ int main(int argc, char **argv) > > rcvbuf = size; > > } else if (matches(opt, "-help") == 0) { > > usage(); > > + } else if (matches(opt, "-ns") == 0) { > > + argc--; > > + argv++; > > + argv[0] = argv[1]; > > + argv[1] = basename; > > + return netns_exec(argc, argv); > > I very much dislike this. There's no reason to exec another ip binary. > The main reason I wanted the -n (or whatever) option was to speed up > execution of test scripts in environments with hundreds of interfaces > in different net namespaces. > > Please just change to the specified netns and continue with interpreting > of the rest of the command line, there's absolutely no reason for doing > the exec. Yes, I will follow that way. Thanks, > > Thanks, > > Jiri > > -- > Jiri Benc -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11-12-2014 14:33, vadim4j@gmail.com wrote: > On Thu, Dec 11, 2014 at 05:09:28PM +0100, Jiri Benc wrote: >> On Thu, 11 Dec 2014 00:56:35 +0200, Vadim Kochan wrote: >>> From: Vadim Kochan <vadim4j@gmail.com> >>> >>> Added new '-ns' option to simplify executing following cmd: >>> >>> ip netns exec NETNS ip OPTIONS COMMAND OBJECT >>> >>> to >>> >>> ip -ns NETNS OPTIONS COMMAND OBJECT >>> >>> e.g.: >>> >>> ip -ns vnet0 link add br0 type bridge >> >> This is great! It's a thing that has been bothering me for long time >> but never got high enough on my todo list. Thanks for working on this. >> >> However, >> >>> --- a/ip/ip.c >>> +++ b/ip/ip.c >>> @@ -262,6 +262,12 @@ int main(int argc, char **argv) >>> rcvbuf = size; >>> } else if (matches(opt, "-help") == 0) { >>> usage(); >>> + } else if (matches(opt, "-ns") == 0) { >>> + argc--; >>> + argv++; >>> + argv[0] = argv[1]; >>> + argv[1] = basename; >>> + return netns_exec(argc, argv); >> >> I very much dislike this. There's no reason to exec another ip binary. >> The main reason I wanted the -n (or whatever) option was to speed up >> execution of test scripts in environments with hundreds of interfaces >> in different net namespaces. >> >> Please just change to the specified netns and continue with interpreting >> of the rest of the command line, there's absolutely no reason for doing >> the exec. > Yes, I will follow that way. In that case, it would be interesting to also accelerate the original use case, no? So all usages we currently have will benefit from this speed up without a change. if (command to be executed == myself) switch namespace, continue without fork/exec.. I'm not sure if this is feasible, though. Just sharing the idea, didn't even open the code.. Marcelo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 11 Dec 2014 15:33:34 -0200, Marcelo Ricardo Leitner wrote: > In that case, it would be interesting to also accelerate the original use > case, no? So all usages we currently have will benefit from this speed up > without a change. > > if (command to be executed == myself) > switch namespace, continue without fork/exec.. It's never good idea to do such tricks behind the user's back. This particular case could easily break for users wanting to execute a different ip binary (for whatever reason). All programs should do what they are told to do, not try to outsmart the user. Jiri
On 11-12-2014 16:08, Jiri Benc wrote: > On Thu, 11 Dec 2014 15:33:34 -0200, Marcelo Ricardo Leitner wrote: >> In that case, it would be interesting to also accelerate the original use >> case, no? So all usages we currently have will benefit from this speed up >> without a change. >> >> if (command to be executed == myself) >> switch namespace, continue without fork/exec.. > > It's never good idea to do such tricks behind the user's back. This > particular case could easily break for users wanting to execute a > different ip binary (for whatever reason). Then the if() above wouldn't match. That if means to check /proc/self/exe against the result of the path expansion. If that fails, continue with the normal path. If it matches, it is the same binary, and no need to re-exec itself. > All programs should do what they are told to do, not try to outsmart > the user. It's not outsmarting, it's just not being dumb and doing it the proper way. Bash itself does this twist a lot. If you just type 'echo hi', it won't execute /bin/echo but use a built-in version. But if you write "/bin/echo hi", it will use /bin/echo.. We could use the same idea. "ip netns exec ip" -> ellipse it and avoid the fork/exec. But if it's cmd != "ip", execute it.. Now consider other applications that are user of this command. They will have to implement something like: if (this ip command has --netns argument) { cmd="ip --netns ..." } else { cmd="ip netns exec ..." } which is ugly and inconvenient. Marcelo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/ip/ip.c b/ip/ip.c index 5f759d5..35cf463 100644 --- a/ip/ip.c +++ b/ip/ip.c @@ -262,6 +262,12 @@ int main(int argc, char **argv) rcvbuf = size; } else if (matches(opt, "-help") == 0) { usage(); + } else if (matches(opt, "-ns") == 0) { + argc--; + argv++; + argv[0] = argv[1]; + argv[1] = basename; + return netns_exec(argc, argv); } else { fprintf(stderr, "Option \"%s\" is unknown, try \"ip -help\".\n", opt); exit(-1); diff --git a/ip/ip_common.h b/ip/ip_common.h index 75bfb82..d4f7e1f 100644 --- a/ip/ip_common.h +++ b/ip/ip_common.h @@ -88,6 +88,7 @@ struct link_util struct link_util *get_link_kind(const char *kind); struct link_util *get_link_slave_kind(const char *slave_kind); int get_netns_fd(const char *name); +int netns_exec(int argc, char **argv); #ifndef INFINITY_LIFE_TIME #define INFINITY_LIFE_TIME 0xFFFFFFFFU diff --git a/ip/ipnetns.c b/ip/ipnetns.c index 1c8aa02..367841c 100644 --- a/ip/ipnetns.c +++ b/ip/ipnetns.c @@ -129,7 +129,7 @@ static void bind_etc(const char *name) closedir(dir); } -static int netns_exec(int argc, char **argv) +int netns_exec(int argc, char **argv) { /* Setup the proper environment for apps that are not netns * aware, and execute a program in that environment. diff --git a/man/man8/ip.8 b/man/man8/ip.8 index 2d42e98..bfb0c53 100644 --- a/man/man8/ip.8 +++ b/man/man8/ip.8 @@ -134,6 +134,27 @@ the output. use the system's name resolver to print DNS names instead of host addresses. +.TP +.BR "\-ns " <NETNS> +executes the following +.RI "[ " OPTIONS " ] " OBJECT " { " COMMAND " | " +.BR help " }" +in the specified network namespace +.IR NETNS . +Actually it just simplifies executing of: + +.B ip netns exec +.IR NETNS +.B ip +.RI "[ " OPTIONS " ] " OBJECT " { " COMMAND " | " +.BR help " }" + +to + +.B ip +.RI "-ns " NETNS " [ " OPTIONS " ] " OBJECT " { " COMMAND " | " +.BR help " }" + .SH IP - COMMAND SYNTAX .SS