* [PATCH iproute2 0/2] add batch command support to devlink
@ 2017-11-10 6:20 Ivan Vecera
2017-11-10 6:20 ` [PATCH iproute2 1/2] lib: make resolve_hosts variable common Ivan Vecera
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Ivan Vecera @ 2017-11-10 6:20 UTC (permalink / raw)
To: netdev; +Cc: jiri, arkadis
This patch series adds support for devlink commands batching. The first
just removes a requirement to have declared 'resolve_hosts' variable in
any command that use any function implemented in utils.c (it is really
confusing to see this declaration in utils like bridge or devlink).
Ivan Vecera (2):
lib: make resolve_hosts variable common
devlink: add batch command support
bridge/bridge.c | 1 -
devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++-------
genl/genl.c | 1 -
ip/ip.c | 1 -
ip/rtmon.c | 1 -
lib/utils.c | 1 +
man/man8/devlink.8 | 16 +++++++++++++
misc/arpd.c | 2 --
misc/ss.c | 1 -
tc/tc.c | 1 -
10 files changed, 79 insertions(+), 16 deletions(-)
--
2.13.6
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH iproute2 1/2] lib: make resolve_hosts variable common 2017-11-10 6:20 [PATCH iproute2 0/2] add batch command support to devlink Ivan Vecera @ 2017-11-10 6:20 ` Ivan Vecera 2017-11-10 6:20 ` [PATCH iproute2 2/2] devlink: add batch command support Ivan Vecera 2017-11-13 0:17 ` [PATCH iproute2 0/2] add batch command support to devlink Stephen Hemminger 2 siblings, 0 replies; 13+ messages in thread From: Ivan Vecera @ 2017-11-10 6:20 UTC (permalink / raw) To: netdev; +Cc: jiri, arkadis Any iproute utility that uses any function from lib/utils.c needs to declare its own resolve_hosts variable instance although it does not need/use hostname resolving functionality (currently only 'ip' and 'ss' commands uses this). The patch declares single common instance of resolve_hosts directly in utils.c so the existing ones can be removed (the same approach that is used for timestamp_short). Cc: Jiri Pirko <jiri@mellanox.com> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- bridge/bridge.c | 1 - genl/genl.c | 1 - ip/ip.c | 1 - ip/rtmon.c | 1 - lib/utils.c | 1 + misc/arpd.c | 2 -- misc/ss.c | 1 - tc/tc.c | 1 - 8 files changed, 1 insertion(+), 8 deletions(-) diff --git a/bridge/bridge.c b/bridge/bridge.c index 5ff038d6..6658cb8f 100644 --- a/bridge/bridge.c +++ b/bridge/bridge.c @@ -18,7 +18,6 @@ struct rtnl_handle rth = { .fd = -1 }; int preferred_family = AF_UNSPEC; -int resolve_hosts; int oneline; int show_stats; int show_details; diff --git a/genl/genl.c b/genl/genl.c index 747074b0..7e4a208d 100644 --- a/genl/genl.c +++ b/genl/genl.c @@ -30,7 +30,6 @@ int show_stats = 0; int show_details = 0; int show_raw = 0; -int resolve_hosts = 0; static void *BODY; static struct genl_util * genl_list; diff --git a/ip/ip.c b/ip/ip.c index e66f6970..e2da46dd 100644 --- a/ip/ip.c +++ b/ip/ip.c @@ -30,7 +30,6 @@ int human_readable; int use_iec; int show_stats; int show_details; -int resolve_hosts; int oneline; int brief; int json; diff --git a/ip/rtmon.c b/ip/rtmon.c index 1c2981f7..94baa38e 100644 --- a/ip/rtmon.c +++ b/ip/rtmon.c @@ -25,7 +25,6 @@ #include "utils.h" #include "libnetlink.h" -int resolve_hosts; static int init_phase = 1; static void write_stamp(FILE *fp) diff --git a/lib/utils.c b/lib/utils.c index ac155bf5..f77be1fd 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -37,6 +37,7 @@ #include "utils.h" #include "namespace.h" +int resolve_hosts; int timestamp_short; int get_hex(char c) diff --git a/misc/arpd.c b/misc/arpd.c index c2666f76..67d86b67 100644 --- a/misc/arpd.c +++ b/misc/arpd.c @@ -38,8 +38,6 @@ #include "utils.h" #include "rt_names.h" -int resolve_hosts; - DB *dbase; char *dbname = "/var/lib/arpd/arpd.db"; diff --git a/misc/ss.c b/misc/ss.c index 56a9ad41..45a0c330 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -88,7 +88,6 @@ static int security_get_initial_context(char *name, char **context) } #endif -int resolve_hosts; int resolve_services = 1; int preferred_family = AF_UNSPEC; int show_options; diff --git a/tc/tc.c b/tc/tc.c index 8e64a82b..32924164 100644 --- a/tc/tc.c +++ b/tc/tc.c @@ -39,7 +39,6 @@ int show_graph; int timestamp; int batch_mode; -int resolve_hosts; int use_iec; int force; bool use_names; -- 2.13.6 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 2/2] devlink: add batch command support 2017-11-10 6:20 [PATCH iproute2 0/2] add batch command support to devlink Ivan Vecera 2017-11-10 6:20 ` [PATCH iproute2 1/2] lib: make resolve_hosts variable common Ivan Vecera @ 2017-11-10 6:20 ` Ivan Vecera 2017-11-10 6:57 ` Leon Romanovsky 2017-11-12 5:08 ` Jiri Pirko 2017-11-13 0:17 ` [PATCH iproute2 0/2] add batch command support to devlink Stephen Hemminger 2 siblings, 2 replies; 13+ messages in thread From: Ivan Vecera @ 2017-11-10 6:20 UTC (permalink / raw) To: netdev; +Cc: jiri, arkadis The patch adds support to batch devlink commands. Cc: Jiri Pirko <jiri@mellanox.com> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- man/man8/devlink.8 | 16 +++++++++++++ 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index 39cda067..1b15eef8 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -3803,12 +3803,16 @@ static int cmd_dpipe(struct dl *dl) static void help(void) { pr_err("Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }\n" + " devlink [ -f[orce] ] -b[atch] filename\n" "where OBJECT := { dev | port | sb | monitor | dpipe }\n" " OPTIONS := { -V[ersion] | -n[no-nice-names] | -j[json] | -p[pretty] | -v[verbose] }\n"); } -static int dl_cmd(struct dl *dl) +static int dl_cmd(struct dl *dl, int argc, char **argv) { + dl->argc = argc; + dl->argv = argv; + if (dl_argv_match(dl, "help") || dl_no_arg(dl)) { help(); return 0; @@ -3832,13 +3836,10 @@ static int dl_cmd(struct dl *dl) return -ENOENT; } -static int dl_init(struct dl *dl, int argc, char **argv) +static int dl_init(struct dl *dl) { int err; - dl->argc = argc; - dl->argv = argv; - dl->nlg = mnlg_socket_open(DEVLINK_GENL_NAME, DEVLINK_GENL_VERSION); if (!dl->nlg) { pr_err("Failed to connect to devlink Netlink\n"); @@ -3890,16 +3891,59 @@ static void dl_free(struct dl *dl) free(dl); } +static int dl_batch(struct dl *dl, const char *name, bool force) +{ + char *line = NULL; + size_t len = 0; + int ret = EXIT_SUCCESS; + + if (name && strcmp(name, "-") != 0) { + if (freopen(name, "r", stdin) == NULL) { + fprintf(stderr, + "Cannot open file \"%s\" for reading: %s\n", + name, strerror(errno)); + return EXIT_FAILURE; + } + } + + cmdlineno = 0; + while (getcmdline(&line, &len, stdin) != -1) { + char *largv[100]; + int largc; + + largc = makeargs(line, largv, 100); + if (!largc) + continue; /* blank line */ + + if (dl_cmd(dl, largc, largv)) { + fprintf(stderr, "Command failed %s:%d\n", + name, cmdlineno); + ret = EXIT_FAILURE; + if (!force) + break; + } + } + + if (line) + free(line); + + return ret; +} + int main(int argc, char **argv) { static const struct option long_options[] = { { "Version", no_argument, NULL, 'V' }, + { "force", no_argument, NULL, 'f' }, + { "batch", required_argument, NULL, 'b' }, { "no-nice-names", no_argument, NULL, 'n' }, { "json", no_argument, NULL, 'j' }, { "pretty", no_argument, NULL, 'p' }, { "verbose", no_argument, NULL, 'v' }, { NULL, 0, NULL, 0 } }; + const char *batch_file = NULL; + bool force = false; struct dl *dl; int opt; int err; @@ -3911,7 +3955,7 @@ int main(int argc, char **argv) return EXIT_FAILURE; } - while ((opt = getopt_long(argc, argv, "Vnjpv", + while ((opt = getopt_long(argc, argv, "Vfb:njpv", long_options, NULL)) >= 0) { switch (opt) { @@ -3919,6 +3963,12 @@ int main(int argc, char **argv) printf("devlink utility, iproute2-ss%s\n", SNAPSHOT); ret = EXIT_SUCCESS; goto dl_free; + case 'f': + force = true; + break; + case 'b': + batch_file = optarg; + break; case 'n': dl->no_nice_names = true; break; @@ -3942,13 +3992,17 @@ int main(int argc, char **argv) argc -= optind; argv += optind; - err = dl_init(dl, argc, argv); + err = dl_init(dl); if (err) { ret = EXIT_FAILURE; goto dl_free; } - err = dl_cmd(dl); + if (batch_file) + err = dl_batch(dl, batch_file, force); + else + err = dl_cmd(dl, argc, argv); + if (err) { ret = EXIT_FAILURE; goto dl_fini; diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 index a480766c..a975ef34 100644 --- a/man/man8/devlink.8 +++ b/man/man8/devlink.8 @@ -12,6 +12,12 @@ devlink \- Devlink tool .sp .ti -8 +.B devlink +.RB "[ " -force " ] " +.BI "-batch " filename +.sp + +.ti -8 .IR OBJECT " := { " .BR dev " | " port " | " monitor " }" .sp @@ -32,6 +38,16 @@ Print the version of the utility and exit. .TP +.BR "\-b", " \-batch " <FILENAME> +Read commands from provided file or standard input and invoke them. +First failure will cause termination of devlink. + +.TP +.BR "\-force" +Don't terminate devlink on errors in batch mode. +If there were any errors during execution of the commands, the application return code will be non zero. + +.TP .BR "\-n" , " --no-nice-names" Turn off printing out nice names, for example netdevice ifnames instead of devlink port identification. -- 2.13.6 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-10 6:20 ` [PATCH iproute2 2/2] devlink: add batch command support Ivan Vecera @ 2017-11-10 6:57 ` Leon Romanovsky 2017-11-10 7:10 ` Ivan Vecera 2017-11-12 5:08 ` Jiri Pirko 1 sibling, 1 reply; 13+ messages in thread From: Leon Romanovsky @ 2017-11-10 6:57 UTC (permalink / raw) To: Ivan Vecera; +Cc: netdev, jiri, arkadis [-- Attachment #1: Type: text/plain, Size: 1580 bytes --] On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote: > The patch adds support to batch devlink commands. > > Cc: Jiri Pirko <jiri@mellanox.com> > Cc: Arkadi Sharshevsky <arkadis@mellanox.com> > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- > man/man8/devlink.8 | 16 +++++++++++++ > 2 files changed, 78 insertions(+), 8 deletions(-) > <..> > diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 > index a480766c..a975ef34 100644 > --- a/man/man8/devlink.8 > +++ b/man/man8/devlink.8 > @@ -12,6 +12,12 @@ devlink \- Devlink tool > .sp > > .ti -8 > +.B devlink > +.RB "[ " -force " ] " > +.BI "-batch " filename > +.sp > + > +.ti -8 > .IR OBJECT " := { " > .BR dev " | " port " | " monitor " }" > .sp > @@ -32,6 +38,16 @@ Print the version of the > utility and exit. > > .TP > +.BR "\-b", " \-batch " <FILENAME> > +Read commands from provided file or standard input and invoke them. > +First failure will cause termination of devlink. It is worth to document the expected format of that file. And IMHO, it is better to have ability to load JSON fie which was generated by -j, instead of declaring new format/knob. > + > +.TP > +.BR "\-force" > +Don't terminate devlink on errors in batch mode. > +If there were any errors during execution of the commands, the application return code will be non zero. > + > +.TP > .BR "\-n" , " --no-nice-names" > Turn off printing out nice names, for example netdevice ifnames instead of devlink port identification. > > -- > 2.13.6 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-10 6:57 ` Leon Romanovsky @ 2017-11-10 7:10 ` Ivan Vecera 2017-11-10 19:47 ` Leon Romanovsky 0 siblings, 1 reply; 13+ messages in thread From: Ivan Vecera @ 2017-11-10 7:10 UTC (permalink / raw) To: Leon Romanovsky; +Cc: netdev, jiri, arkadis [-- Attachment #1.1: Type: text/plain, Size: 1436 bytes --] On 10.11.2017 07:57, Leon Romanovsky wrote: > On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote: >> The patch adds support to batch devlink commands. >> >> Cc: Jiri Pirko <jiri@mellanox.com> >> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >> --- >> devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- >> man/man8/devlink.8 | 16 +++++++++++++ >> 2 files changed, 78 insertions(+), 8 deletions(-) >> > > <..> > >> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 >> index a480766c..a975ef34 100644 >> --- a/man/man8/devlink.8 >> +++ b/man/man8/devlink.8 >> @@ -12,6 +12,12 @@ devlink \- Devlink tool >> .sp >> >> .ti -8 >> +.B devlink >> +.RB "[ " -force " ] " >> +.BI "-batch " filename >> +.sp >> + >> +.ti -8 >> .IR OBJECT " := { " >> .BR dev " | " port " | " monitor " }" >> .sp >> @@ -32,6 +38,16 @@ Print the version of the >> utility and exit. >> >> .TP >> +.BR "\-b", " \-batch " <FILENAME> >> +Read commands from provided file or standard input and invoke them. >> +First failure will cause termination of devlink. > > It is worth to document the expected format of that file. > And IMHO, it is better to have ability to load JSON fie which was > generated by -j, instead of declaring new format/knob. It's just a list of command-lines... like other utils (bridge,ip...) I. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 862 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-10 7:10 ` Ivan Vecera @ 2017-11-10 19:47 ` Leon Romanovsky 2017-11-12 5:06 ` Jiri Pirko 2017-11-13 0:16 ` Stephen Hemminger 0 siblings, 2 replies; 13+ messages in thread From: Leon Romanovsky @ 2017-11-10 19:47 UTC (permalink / raw) To: Ivan Vecera; +Cc: netdev, jiri, arkadis [-- Attachment #1: Type: text/plain, Size: 1677 bytes --] On Fri, Nov 10, 2017 at 08:10:43AM +0100, Ivan Vecera wrote: > On 10.11.2017 07:57, Leon Romanovsky wrote: > > On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote: > >> The patch adds support to batch devlink commands. > >> > >> Cc: Jiri Pirko <jiri@mellanox.com> > >> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> > >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> > >> --- > >> devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- > >> man/man8/devlink.8 | 16 +++++++++++++ > >> 2 files changed, 78 insertions(+), 8 deletions(-) > >> > > > > <..> > > > >> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 > >> index a480766c..a975ef34 100644 > >> --- a/man/man8/devlink.8 > >> +++ b/man/man8/devlink.8 > >> @@ -12,6 +12,12 @@ devlink \- Devlink tool > >> .sp > >> > >> .ti -8 > >> +.B devlink > >> +.RB "[ " -force " ] " > >> +.BI "-batch " filename > >> +.sp > >> + > >> +.ti -8 > >> .IR OBJECT " := { " > >> .BR dev " | " port " | " monitor " }" > >> .sp > >> @@ -32,6 +38,16 @@ Print the version of the > >> utility and exit. > >> > >> .TP > >> +.BR "\-b", " \-batch " <FILENAME> > >> +Read commands from provided file or standard input and invoke them. > >> +First failure will cause termination of devlink. > > > > It is worth to document the expected format of that file. > > And IMHO, it is better to have ability to load JSON fie which was > > generated by -j, instead of declaring new format/knob. > It's just a list of command-lines... like other utils (bridge,ip...) I'm implementing similar thing in RDMAtool (part of iproute2) and choose JSON approach, it is more user and script friendly. Thanks > > I. > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-10 19:47 ` Leon Romanovsky @ 2017-11-12 5:06 ` Jiri Pirko 2017-11-12 8:19 ` Leon Romanovsky 2017-11-13 0:16 ` Stephen Hemminger 1 sibling, 1 reply; 13+ messages in thread From: Jiri Pirko @ 2017-11-12 5:06 UTC (permalink / raw) To: Leon Romanovsky; +Cc: Ivan Vecera, netdev, jiri, arkadis Fri, Nov 10, 2017 at 08:47:35PM CET, leon@kernel.org wrote: >On Fri, Nov 10, 2017 at 08:10:43AM +0100, Ivan Vecera wrote: >> On 10.11.2017 07:57, Leon Romanovsky wrote: >> > On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote: >> >> The patch adds support to batch devlink commands. >> >> >> >> Cc: Jiri Pirko <jiri@mellanox.com> >> >> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> >> >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >> >> --- >> >> devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- >> >> man/man8/devlink.8 | 16 +++++++++++++ >> >> 2 files changed, 78 insertions(+), 8 deletions(-) >> >> >> > >> > <..> >> > >> >> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 >> >> index a480766c..a975ef34 100644 >> >> --- a/man/man8/devlink.8 >> >> +++ b/man/man8/devlink.8 >> >> @@ -12,6 +12,12 @@ devlink \- Devlink tool >> >> .sp >> >> >> >> .ti -8 >> >> +.B devlink >> >> +.RB "[ " -force " ] " >> >> +.BI "-batch " filename >> >> +.sp >> >> + >> >> +.ti -8 >> >> .IR OBJECT " := { " >> >> .BR dev " | " port " | " monitor " }" >> >> .sp >> >> @@ -32,6 +38,16 @@ Print the version of the >> >> utility and exit. >> >> >> >> .TP >> >> +.BR "\-b", " \-batch " <FILENAME> >> >> +Read commands from provided file or standard input and invoke them. >> >> +First failure will cause termination of devlink. >> > >> > It is worth to document the expected format of that file. >> > And IMHO, it is better to have ability to load JSON fie which was >> > generated by -j, instead of declaring new format/knob. >> It's just a list of command-lines... like other utils (bridge,ip...) > >I'm implementing similar thing in RDMAtool (part of iproute2) and choose JSON >approach, it is more user and script friendly. Leon, we should really do things in a way they are currently done and used. Batching is implemented in "ip" for a long time. It makes perfect sense to have one command line per line of the batch file. In contrary, json output sounds really odd in this case. With json, there is no relation to ordinary ip command line params. Or do you want to extend it to accept json as well? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-12 5:06 ` Jiri Pirko @ 2017-11-12 8:19 ` Leon Romanovsky 2017-11-12 8:21 ` Jiri Pirko 0 siblings, 1 reply; 13+ messages in thread From: Leon Romanovsky @ 2017-11-12 8:19 UTC (permalink / raw) To: Jiri Pirko; +Cc: Ivan Vecera, netdev, jiri, arkadis [-- Attachment #1: Type: text/plain, Size: 2605 bytes --] On Sun, Nov 12, 2017 at 06:06:42AM +0100, Jiri Pirko wrote: > Fri, Nov 10, 2017 at 08:47:35PM CET, leon@kernel.org wrote: > >On Fri, Nov 10, 2017 at 08:10:43AM +0100, Ivan Vecera wrote: > >> On 10.11.2017 07:57, Leon Romanovsky wrote: > >> > On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote: > >> >> The patch adds support to batch devlink commands. > >> >> > >> >> Cc: Jiri Pirko <jiri@mellanox.com> > >> >> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> > >> >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> > >> >> --- > >> >> devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- > >> >> man/man8/devlink.8 | 16 +++++++++++++ > >> >> 2 files changed, 78 insertions(+), 8 deletions(-) > >> >> > >> > > >> > <..> > >> > > >> >> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 > >> >> index a480766c..a975ef34 100644 > >> >> --- a/man/man8/devlink.8 > >> >> +++ b/man/man8/devlink.8 > >> >> @@ -12,6 +12,12 @@ devlink \- Devlink tool > >> >> .sp > >> >> > >> >> .ti -8 > >> >> +.B devlink > >> >> +.RB "[ " -force " ] " > >> >> +.BI "-batch " filename > >> >> +.sp > >> >> + > >> >> +.ti -8 > >> >> .IR OBJECT " := { " > >> >> .BR dev " | " port " | " monitor " }" > >> >> .sp > >> >> @@ -32,6 +38,16 @@ Print the version of the > >> >> utility and exit. > >> >> > >> >> .TP > >> >> +.BR "\-b", " \-batch " <FILENAME> > >> >> +Read commands from provided file or standard input and invoke them. > >> >> +First failure will cause termination of devlink. > >> > > >> > It is worth to document the expected format of that file. > >> > And IMHO, it is better to have ability to load JSON fie which was > >> > generated by -j, instead of declaring new format/knob. > >> It's just a list of command-lines... like other utils (bridge,ip...) > > > >I'm implementing similar thing in RDMAtool (part of iproute2) and choose JSON > >approach, it is more user and script friendly. > > Leon, we should really do things in a way they are currently done and > used. Batching is implemented in "ip" for a long time. It makes perfect > sense to have one command line per line of the batch file. > > In contrary, json output sounds really odd in this case. With json, > there is no relation to ordinary ip command line params. Or do you want > to extend it to accept json as well? Yes, I wanted to add new field - "cmd", and whole logic to parse JSON to properly rebuild command from that with right parameters and device name. However, most probably, I'll never finish it near future, so in meanwhile I will provide the same batch format as Ivan proposed. Thanks > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-12 8:19 ` Leon Romanovsky @ 2017-11-12 8:21 ` Jiri Pirko 0 siblings, 0 replies; 13+ messages in thread From: Jiri Pirko @ 2017-11-12 8:21 UTC (permalink / raw) To: Leon Romanovsky; +Cc: Ivan Vecera, netdev, jiri, arkadis Sun, Nov 12, 2017 at 09:19:23AM CET, leon@kernel.org wrote: >On Sun, Nov 12, 2017 at 06:06:42AM +0100, Jiri Pirko wrote: >> Fri, Nov 10, 2017 at 08:47:35PM CET, leon@kernel.org wrote: >> >On Fri, Nov 10, 2017 at 08:10:43AM +0100, Ivan Vecera wrote: >> >> On 10.11.2017 07:57, Leon Romanovsky wrote: >> >> > On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote: >> >> >> The patch adds support to batch devlink commands. >> >> >> >> >> >> Cc: Jiri Pirko <jiri@mellanox.com> >> >> >> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> >> >> >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >> >> >> --- >> >> >> devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- >> >> >> man/man8/devlink.8 | 16 +++++++++++++ >> >> >> 2 files changed, 78 insertions(+), 8 deletions(-) >> >> >> >> >> > >> >> > <..> >> >> > >> >> >> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 >> >> >> index a480766c..a975ef34 100644 >> >> >> --- a/man/man8/devlink.8 >> >> >> +++ b/man/man8/devlink.8 >> >> >> @@ -12,6 +12,12 @@ devlink \- Devlink tool >> >> >> .sp >> >> >> >> >> >> .ti -8 >> >> >> +.B devlink >> >> >> +.RB "[ " -force " ] " >> >> >> +.BI "-batch " filename >> >> >> +.sp >> >> >> + >> >> >> +.ti -8 >> >> >> .IR OBJECT " := { " >> >> >> .BR dev " | " port " | " monitor " }" >> >> >> .sp >> >> >> @@ -32,6 +38,16 @@ Print the version of the >> >> >> utility and exit. >> >> >> >> >> >> .TP >> >> >> +.BR "\-b", " \-batch " <FILENAME> >> >> >> +Read commands from provided file or standard input and invoke them. >> >> >> +First failure will cause termination of devlink. >> >> > >> >> > It is worth to document the expected format of that file. >> >> > And IMHO, it is better to have ability to load JSON fie which was >> >> > generated by -j, instead of declaring new format/knob. >> >> It's just a list of command-lines... like other utils (bridge,ip...) >> > >> >I'm implementing similar thing in RDMAtool (part of iproute2) and choose JSON >> >approach, it is more user and script friendly. >> >> Leon, we should really do things in a way they are currently done and >> used. Batching is implemented in "ip" for a long time. It makes perfect >> sense to have one command line per line of the batch file. >> >> In contrary, json output sounds really odd in this case. With json, >> there is no relation to ordinary ip command line params. Or do you want >> to extend it to accept json as well? > >Yes, I wanted to add new field - "cmd", and whole logic to parse JSON to >properly rebuild command from that with right parameters and device name. > >However, most probably, I'll never finish it near future, so in meanwhile I will >provide the same batch format as Ivan proposed. Good. I think it is more than enough for now :) Thanks! > >Thanks > >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-10 19:47 ` Leon Romanovsky 2017-11-12 5:06 ` Jiri Pirko @ 2017-11-13 0:16 ` Stephen Hemminger 2017-11-13 8:59 ` Leon Romanovsky 1 sibling, 1 reply; 13+ messages in thread From: Stephen Hemminger @ 2017-11-13 0:16 UTC (permalink / raw) To: Leon Romanovsky; +Cc: Ivan Vecera, netdev, jiri, arkadis [-- Attachment #1: Type: text/plain, Size: 2079 bytes --] On Fri, 10 Nov 2017 21:47:35 +0200 Leon Romanovsky <leon@kernel.org> wrote: > On Fri, Nov 10, 2017 at 08:10:43AM +0100, Ivan Vecera wrote: > > On 10.11.2017 07:57, Leon Romanovsky wrote: > > > On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote: > > >> The patch adds support to batch devlink commands. > > >> > > >> Cc: Jiri Pirko <jiri@mellanox.com> > > >> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> > > >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> > > >> --- > > >> devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- > > >> man/man8/devlink.8 | 16 +++++++++++++ > > >> 2 files changed, 78 insertions(+), 8 deletions(-) > > >> > > > > > > <..> > > > > > >> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 > > >> index a480766c..a975ef34 100644 > > >> --- a/man/man8/devlink.8 > > >> +++ b/man/man8/devlink.8 > > >> @@ -12,6 +12,12 @@ devlink \- Devlink tool > > >> .sp > > >> > > >> .ti -8 > > >> +.B devlink > > >> +.RB "[ " -force " ] " > > >> +.BI "-batch " filename > > >> +.sp > > >> + > > >> +.ti -8 > > >> .IR OBJECT " := { " > > >> .BR dev " | " port " | " monitor " }" > > >> .sp > > >> @@ -32,6 +38,16 @@ Print the version of the > > >> utility and exit. > > >> > > >> .TP > > >> +.BR "\-b", " \-batch " <FILENAME> > > >> +Read commands from provided file or standard input and invoke them. > > >> +First failure will cause termination of devlink. > > > > > > It is worth to document the expected format of that file. > > > And IMHO, it is better to have ability to load JSON fie which was > > > generated by -j, instead of declaring new format/knob. > > It's just a list of command-lines... like other utils (bridge,ip...) > > I'm implementing similar thing in RDMAtool (part of iproute2) and choose JSON > approach, it is more user and script friendly. > If you want to do batch form rdmatool then it must take list of commands by default. An additional option to take json input "rdmatool -j --batch..." would be good as well. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-13 0:16 ` Stephen Hemminger @ 2017-11-13 8:59 ` Leon Romanovsky 0 siblings, 0 replies; 13+ messages in thread From: Leon Romanovsky @ 2017-11-13 8:59 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Ivan Vecera, netdev, jiri, arkadis [-- Attachment #1: Type: text/plain, Size: 2204 bytes --] On Sun, Nov 12, 2017 at 04:16:50PM -0800, Stephen Hemminger wrote: > On Fri, 10 Nov 2017 21:47:35 +0200 > Leon Romanovsky <leon@kernel.org> wrote: > > > On Fri, Nov 10, 2017 at 08:10:43AM +0100, Ivan Vecera wrote: > > > On 10.11.2017 07:57, Leon Romanovsky wrote: > > > > On Fri, Nov 10, 2017 at 07:20:14AM +0100, Ivan Vecera wrote: > > > >> The patch adds support to batch devlink commands. > > > >> > > > >> Cc: Jiri Pirko <jiri@mellanox.com> > > > >> Cc: Arkadi Sharshevsky <arkadis@mellanox.com> > > > >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> > > > >> --- > > > >> devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- > > > >> man/man8/devlink.8 | 16 +++++++++++++ > > > >> 2 files changed, 78 insertions(+), 8 deletions(-) > > > >> > > > > > > > > <..> > > > > > > > >> diff --git a/man/man8/devlink.8 b/man/man8/devlink.8 > > > >> index a480766c..a975ef34 100644 > > > >> --- a/man/man8/devlink.8 > > > >> +++ b/man/man8/devlink.8 > > > >> @@ -12,6 +12,12 @@ devlink \- Devlink tool > > > >> .sp > > > >> > > > >> .ti -8 > > > >> +.B devlink > > > >> +.RB "[ " -force " ] " > > > >> +.BI "-batch " filename > > > >> +.sp > > > >> + > > > >> +.ti -8 > > > >> .IR OBJECT " := { " > > > >> .BR dev " | " port " | " monitor " }" > > > >> .sp > > > >> @@ -32,6 +38,16 @@ Print the version of the > > > >> utility and exit. > > > >> > > > >> .TP > > > >> +.BR "\-b", " \-batch " <FILENAME> > > > >> +Read commands from provided file or standard input and invoke them. > > > >> +First failure will cause termination of devlink. > > > > > > > > It is worth to document the expected format of that file. > > > > And IMHO, it is better to have ability to load JSON fie which was > > > > generated by -j, instead of declaring new format/knob. > > > It's just a list of command-lines... like other utils (bridge,ip...) > > > > I'm implementing similar thing in RDMAtool (part of iproute2) and choose JSON > > approach, it is more user and script friendly. > > > > If you want to do batch form rdmatool then it must take list of commands by default. > An additional option to take json input "rdmatool -j --batch..." would be good as well. > I'll do. Thanks > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 2/2] devlink: add batch command support 2017-11-10 6:20 ` [PATCH iproute2 2/2] devlink: add batch command support Ivan Vecera 2017-11-10 6:57 ` Leon Romanovsky @ 2017-11-12 5:08 ` Jiri Pirko 1 sibling, 0 replies; 13+ messages in thread From: Jiri Pirko @ 2017-11-12 5:08 UTC (permalink / raw) To: Ivan Vecera; +Cc: netdev, jiri, arkadis Fri, Nov 10, 2017 at 07:20:14AM CET, ivecera@redhat.com wrote: >The patch adds support to batch devlink commands. > >Cc: Jiri Pirko <jiri@mellanox.com> >Cc: Arkadi Sharshevsky <arkadis@mellanox.com> >Signed-off-by: Ivan Vecera <ivecera@redhat.com> Acked-by: Jiri Pirko <jiri@mellanox.com> Thanks! ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 0/2] add batch command support to devlink 2017-11-10 6:20 [PATCH iproute2 0/2] add batch command support to devlink Ivan Vecera 2017-11-10 6:20 ` [PATCH iproute2 1/2] lib: make resolve_hosts variable common Ivan Vecera 2017-11-10 6:20 ` [PATCH iproute2 2/2] devlink: add batch command support Ivan Vecera @ 2017-11-13 0:17 ` Stephen Hemminger 2 siblings, 0 replies; 13+ messages in thread From: Stephen Hemminger @ 2017-11-13 0:17 UTC (permalink / raw) To: Ivan Vecera; +Cc: netdev, jiri, arkadis On Fri, 10 Nov 2017 07:20:12 +0100 Ivan Vecera <ivecera@redhat.com> wrote: > This patch series adds support for devlink commands batching. The first > just removes a requirement to have declared 'resolve_hosts' variable in > any command that use any function implemented in utils.c (it is really > confusing to see this declaration in utils like bridge or devlink). > > Ivan Vecera (2): > lib: make resolve_hosts variable common > devlink: add batch command support > > bridge/bridge.c | 1 - > devlink/devlink.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++------- > genl/genl.c | 1 - > ip/ip.c | 1 - > ip/rtmon.c | 1 - > lib/utils.c | 1 + > man/man8/devlink.8 | 16 +++++++++++++ > misc/arpd.c | 2 -- > misc/ss.c | 1 - > tc/tc.c | 1 - > 10 files changed, 79 insertions(+), 16 deletions(-) > Applied ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-11-13 8:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-10 6:20 [PATCH iproute2 0/2] add batch command support to devlink Ivan Vecera 2017-11-10 6:20 ` [PATCH iproute2 1/2] lib: make resolve_hosts variable common Ivan Vecera 2017-11-10 6:20 ` [PATCH iproute2 2/2] devlink: add batch command support Ivan Vecera 2017-11-10 6:57 ` Leon Romanovsky 2017-11-10 7:10 ` Ivan Vecera 2017-11-10 19:47 ` Leon Romanovsky 2017-11-12 5:06 ` Jiri Pirko 2017-11-12 8:19 ` Leon Romanovsky 2017-11-12 8:21 ` Jiri Pirko 2017-11-13 0:16 ` Stephen Hemminger 2017-11-13 8:59 ` Leon Romanovsky 2017-11-12 5:08 ` Jiri Pirko 2017-11-13 0:17 ` [PATCH iproute2 0/2] add batch command support to devlink Stephen Hemminger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).