Re: [PATCH iproute2-next] rdma: Add batch command support

[Leon Romanovsky]

On Wed, Feb 21, 2018 at 09:10:45AM -0700, David Ahern wrote:
> On 2/21/18 5:38 AM, Leon Romanovsky wrote:
> > @@ -36,17 +37,54 @@ static int rd_cmd(struct rd *rd)
> > { 0 }
> > };
> >
> > +   rd->argc = argc;
> > +   rd->argv = argv;
> > +
> > return rd_exec_cmd(rd, cmds, "object");
> >  }
> >
> > -static int rd_init(struct rd *rd, int argc, char **argv, char *filename)
> > +static int rd_batch(struct rd *rd, const char *name, bool force)
> > +{
> > +   char *line = NULL;
> > +   size_t len = 0;
> > +   int ret = 0;
> > +
> > +   if (name && strcmp(name, "-") != 0) {
> > +   if (!freopen(name, "r", stdin)) {
> > +   pr_err("Cannot open file \"%s\" for reading: %s\n",
> > +  name, strerror(errno));
> > +   return errno;
> > +   }
> > +   }
> > +
> > +   cmdlineno = 0;
> > +   while (getcmdline(, , stdin) != -1) {
> > +   char *largv[512];
> > +   int largc;
> > +
> > +   largc = makeargs(line, largv, 100);
>
> you have largv[512] declared but passing a max of 100. I realize other
> batch commands have it hardcoded, but ARRAY_SIZE is better.

Thanks for catching it, it was my copy/paste error.

I'll wait for a day for other comments and will resubmit if it is needed.

The fix is:

diff --git a/rdma/rdma.c b/rdma/rdma.c
index ab8f98d2..ab2c9608 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -62,7 +62,7 @@ static int rd_batch(struct rd *rd, const char *name, bool 
force)
char *largv[512];
int largc;

-   largc = makeargs(line, largv, 100);
+   largc = makeargs(line, largv, ARRAY_SIZE(largv));
if (!largc)
continue;   /* blank line */



signature.asc
Description: PGP signature

Re: [PATCH iproute2-next] rdma: Add batch command support

[David Ahern]

On 2/21/18 5:38 AM, Leon Romanovsky wrote:
> @@ -36,17 +37,54 @@ static int rd_cmd(struct rd *rd)
>   { 0 }
>   };
> 
> + rd->argc = argc;
> + rd->argv = argv;
> +
>   return rd_exec_cmd(rd, cmds, "object");
>  }
> 
> -static int rd_init(struct rd *rd, int argc, char **argv, char *filename)
> +static int rd_batch(struct rd *rd, const char *name, bool force)
> +{
> + char *line = NULL;
> + size_t len = 0;
> + int ret = 0;
> +
> + if (name && strcmp(name, "-") != 0) {
> + if (!freopen(name, "r", stdin)) {
> + pr_err("Cannot open file \"%s\" for reading: %s\n",
> +name, strerror(errno));
> + return errno;
> + }
> + }
> +
> + cmdlineno = 0;
> + while (getcmdline(, , stdin) != -1) {
> + char *largv[512];
> + int largc;
> +
> + largc = makeargs(line, largv, 100);

you have largv[512] declared but passing a max of 100. I realize other
batch commands have it hardcoded, but ARRAY_SIZE is better.

[PATCH iproute2-next] rdma: Add batch command support

[Leon Romanovsky]

From: Leon Romanovsky 

Implement an option (-b) to execute RDMAtool commands
from supplied file. This follows the same model as
in use for ip and devlink tools, by expecting
every new command to be on new line.

These commands are expected to be without any -*
(e.g. -d, -j, e.t.c) global flags, which should be
called externally.

Signed-off-by: Leon Romanovsky 
---
 man/man8/rdma.8 | 16 +
 rdma/rdma.c | 70 ++---
 2 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/man/man8/rdma.8 b/man/man8/rdma.8
index 798b33d3..fba77693 100644
--- a/man/man8/rdma.8
+++ b/man/man8/rdma.8
@@ -11,6 +11,12 @@ rdma \- RDMA tool
 .BR help " }"
 .sp

+.ti -8
+.B rdma
+.RB "[ " -force " ] "
+.BI "-batch " filename
+.sp
+
 .ti -8
 .IR OBJECT " := { "
 .BR dev " | " link " }"
@@ -31,6 +37,16 @@ Print the version of the
 .B rdma
 tool and exit.

+.TP
+.BR "\-b", " \-batch " 
+Read commands from provided file or standard input and invoke them.
+First failure will cause termination of rdma.
+
+.TP
+.BR "\-force"
+Don't terminate rdma 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 "\-d" , " --details"
 Otuput detailed information.
diff --git a/rdma/rdma.c b/rdma/rdma.c
index 19608f41..ab8f98d2 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -15,8 +15,9 @@
 static void help(char *name)
 {
pr_out("Usage: %s [ OPTIONS ] OBJECT { COMMAND | help }\n"
+  "   %s [ -f[orce] ] -b[atch] filename\n"
   "where  OBJECT := { dev | link | resource | help }\n"
-  "   OPTIONS := { -V[ersion] | -d[etails] | -j[son] | 
-p[retty]}\n", name);
+  "   OPTIONS := { -V[ersion] | -d[etails] | -j[son] | 
-p[retty]}\n", name, name);
 }

 static int cmd_help(struct rd *rd)
@@ -25,7 +26,7 @@ static int cmd_help(struct rd *rd)
return 0;
 }

-static int rd_cmd(struct rd *rd)
+static int rd_cmd(struct rd *rd, int argc, char **argv)
 {
const struct rd_cmd cmds[] = {
{ NULL, cmd_help },
@@ -36,17 +37,54 @@ static int rd_cmd(struct rd *rd)
{ 0 }
};

+   rd->argc = argc;
+   rd->argv = argv;
+
return rd_exec_cmd(rd, cmds, "object");
 }

-static int rd_init(struct rd *rd, int argc, char **argv, char *filename)
+static int rd_batch(struct rd *rd, const char *name, bool force)
+{
+   char *line = NULL;
+   size_t len = 0;
+   int ret = 0;
+
+   if (name && strcmp(name, "-") != 0) {
+   if (!freopen(name, "r", stdin)) {
+   pr_err("Cannot open file \"%s\" for reading: %s\n",
+  name, strerror(errno));
+   return errno;
+   }
+   }
+
+   cmdlineno = 0;
+   while (getcmdline(, , stdin) != -1) {
+   char *largv[512];
+   int largc;
+
+   largc = makeargs(line, largv, 100);
+   if (!largc)
+   continue;   /* blank line */
+
+   ret = rd_cmd(rd, largc, largv);
+   if (ret) {
+   pr_err("Command failed %s:%d\n", name, cmdlineno);
+   if (!force)
+   break;
+   }
+   }
+
+   free(line);
+
+   return ret;
+}
+
+static int rd_init(struct rd *rd, char *filename)
 {
uint32_t seq;
int ret;

rd->filename = filename;
-   rd->argc = argc;
-   rd->argv = argv;
INIT_LIST_HEAD(>dev_map_list);
INIT_LIST_HEAD(>filter_list);

@@ -87,11 +125,15 @@ int main(int argc, char **argv)
{ "json",   no_argument,NULL, 'j' },
{ "pretty", no_argument,NULL, 'p' },
{ "details",no_argument,NULL, 'd' },
+   { "force",  no_argument,NULL, 'f' },
+   { "batch",  required_argument,  NULL, 'b' },
{ NULL, 0, NULL, 0 }
};
+   const char *batch_file = NULL;
bool pretty_output = false;
bool show_details = false;
bool json_output = false;
+   bool force = false;
char *filename;
struct rd rd;
int opt;
@@ -99,7 +141,7 @@ int main(int argc, char **argv)

filename = basename(argv[0]);

-   while ((opt = getopt_long(argc, argv, "Vhdpj",
+   while ((opt = getopt_long(argc, argv, ":Vhdpjfb:",
  long_options, NULL)) >= 0) {
switch (opt) {
case 'V':
@@ -115,9 +157,18 @@ int main(int argc, char **argv)
case 'j':
json_output = true;
break;
+   case 'f':
+   force = true;
+