[PATCH iproute2 0/2] add batch command support to devlink

[PATCH iproute2 0/2] add batch command support to devlink

[Ivan Vecera]

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

[PATCH iproute2 1/2] lib: make resolve_hosts variable common

[Ivan Vecera]

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

[PATCH iproute2 2/2] devlink: add batch command support

[Ivan Vecera]

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

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Leon Romanovsky]

[-- 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 --]

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Ivan Vecera]

[-- 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 --]

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Leon Romanovsky]

[-- 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 --]

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Jiri Pirko]

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?

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Jiri Pirko]

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!

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Leon Romanovsky]

[-- 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 --]

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Jiri Pirko]

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
>
>>

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Stephen Hemminger]

[-- 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 --]

Re: [PATCH iproute2 0/2] add batch command support to devlink

[Stephen Hemminger]

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

Re: [PATCH iproute2 2/2] devlink: add batch command support

[Leon Romanovsky]

[-- 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 --]