Thanks Donald, please see inline. On Sun, Mar 13, 2016 at 1:04 PM, Donald Sharp <[email protected]> wrote:
> Avneesh - > > Can you expand a bit on why adding the ability to call random functions > from the cli is better than just adding a 'test XXX' function as needed? > The functionality is equivalent. The benefits of the 'invoke' approach are: - It is slightly easier to add a test -- just write a function enclosed inside an ifdef DEV_BUILD. The cli plumbing (help text, installation of the command node etc.) is not necessary. - In a similar vein, when writing tests you can restart a protocol daemon and run new tests without having to update vtysh. - Test functions can be located in any file, not just in the files that are parsed for cli commands. - Non-development builds are smaller because test code is compiled out. - If (when?) quagga switches to a better cli, we won't have to plumb each test function for the new cli -- just the one command (invoke). - The 'invoke' handler also prints out some useful meta information, such as CPU time taken. - AFAICT, 'test' commands are present in shipped code by default. This does not seem like a good idea for unit tests. It is certainly possible to omit the commands from a build, but this requires additional touches to the code, and an understanding of how the cli works. In summary, I think that 'invoke' makes it slightly easier to write a test. Possibly enough to encourage people to write more tests :-). Additionally how do you plan to allow the end user to pass in useful data > to functions if the function you want to call takes a complex data > structure? > The expectation is that most functions will be unit tests that require no inputs or few inputs. Since a function runs in the regular context of a daemon, most setup can be performed via config commands (for example, a static route). If a test works on a fixed set of inputs, these can be compiled in (conditionally, just like the test) as static C structures. If user-supplied inputs are really necessary for some reason, they would be passed in through cli arguments. Complex inputs can be saved to a file, with the file name passed to the test via an argument. Presumably one would do something similar with a 'test xxx' command. Thanks, Avneesh I'm pretty dubious about adding a cause a crash button even for developer > builds, especially since adding a test XXX function is an easy task. > > donald > > On Fri, Mar 11, 2016 at 3:21 PM, Avneesh Sachdev <[email protected]> > wrote: > >> This functionality is only compiled into a developer build, It allows >> test functions to be invoked by name from the vtysh as follows. >> >> Changes: >> >> * vtysh/vtysh.c >> >> Handle the 'invoke <component> function <function-name> [args]' >> command. >> Forward it the client with the given name. >> >> * lib/vty_invoke.c >> >> Code that handles an 'invoke function' command inside a vtysh >> client process (e;g., a routing protocol daemon). It invokes the >> named function, and prints out some stats about the execution >> time. >> >> * lib/thread.h >> >> Extern timeval_elapsed(). >> >> * lib/vty.[ch] >> >> - vty_init(): >> >> Install element for 'invoke' command handler. This sets up the >> handler that runs inside of a vtysh client. >> >> * lib/Makefile.am >> >> Also compile vty_invoke.c. >> >> * configure.ac >> >> Include '-rdynamic -ldl' in LDFLAGS for a development build. These >> options allow dlsym() to work for functions inside a quagga >> program. >> >> Signed-off-by: Avneesh Sachdev <[email protected]> >> --- >> configure.ac | 4 ++ >> lib/Makefile.am | 2 +- >> lib/thread.h | 3 ++ >> lib/vty.c | 4 ++ >> lib/vty_invoke.c | 120 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> lib/vty_invoke.h | 30 ++++++++++++++ >> vtysh/vtysh.c | 78 ++++++++++++++++++++++++++++++++++++ >> 7 files changed, 240 insertions(+), 1 deletion(-) >> create mode 100644 lib/vty_invoke.c >> create mode 100644 lib/vty_invoke.h >> >> diff --git a/configure.ac b/configure.ac >> index ae2e527..60a9118 100755 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -331,6 +331,10 @@ fi >> >> if test "x${enable_dev_build}" = "xyes"; then >> AC_DEFINE(DEV_BUILD,,Build for development) >> + >> + # Link with -ldl so that the developer build can use dlsym and >> + # company. >> + LDFLAGS="${LDFLAGS} -rdynamic -ldl" >> fi >> AM_CONDITIONAL([DEV_BUILD], [test "x$enable_dev_build" = "xyes"]) >> >> diff --git a/lib/Makefile.am b/lib/Makefile.am >> index ac51fc6..17dc575 100644 >> --- a/lib/Makefile.am >> +++ b/lib/Makefile.am >> @@ -13,7 +13,7 @@ libzebra_la_SOURCES = \ >> sockunion.c prefix.c thread.c if.c memory.c buffer.c table.c >> hash.c \ >> filter.c routemap.c distribute.c stream.c str.c log.c plist.c \ >> zclient.c sockopt.c smux.c agentx.c snmp.c md5.c if_rmap.c >> keychain.c privs.c \ >> - sigevent.c pqueue.c jhash.c memtypes.c workqueue.c vrf.c >> + sigevent.c pqueue.c jhash.c memtypes.c workqueue.c vrf.c >> vty_invoke.c >> >> BUILT_SOURCES = memtypes.h route_types.h gitversion.h >> >> diff --git a/lib/thread.h b/lib/thread.h >> index 5bc756c..215ac5d 100644 >> --- a/lib/thread.h >> +++ b/lib/thread.h >> @@ -229,6 +229,9 @@ extern time_t quagga_time (time_t *); >> extern unsigned long thread_consumed_time(RUSAGE_T *after, RUSAGE_T >> *before, >> unsigned long >> *cpu_time_elapsed); >> >> +/* Returns elapsed time in microseconds */ >> +extern unsigned long timeval_elapsed (struct timeval a, struct timeval >> b); >> + >> /* Global variable containing a recent result from gettimeofday. This >> can >> be used instead of calling gettimeofday if a recent value is >> sufficient. >> This is guaranteed to be refreshed before a thread is called. */ >> diff --git a/lib/vty.c b/lib/vty.c >> index 8befcb0..657ed97 100644 >> --- a/lib/vty.c >> +++ b/lib/vty.c >> @@ -40,6 +40,8 @@ >> #include <arpa/telnet.h> >> #include <termios.h> >> >> +#include "vty_invoke.h" >> + >> /* Vty events */ >> enum event >> { >> @@ -3073,6 +3075,8 @@ vty_init (struct thread_master *master_thread) >> install_element (VTY_NODE, &vty_ipv6_access_class_cmd); >> install_element (VTY_NODE, &no_vty_ipv6_access_class_cmd); >> #endif /* HAVE_IPV6 */ >> + >> + vty_invoke_init (); >> } >> >> void >> diff --git a/lib/vty_invoke.c b/lib/vty_invoke.c >> new file mode 100644 >> index 0000000..3c43568 >> --- /dev/null >> +++ b/lib/vty_invoke.c >> @@ -0,0 +1,120 @@ >> +/* >> + * vty_invoke.c >> + * >> + * @copyright Copyright (C) 2016 Sproute Networks, Inc. >> + * >> + * @author Avneesh Sachdev <[email protected]> >> + * >> + * This file is part of GNU Zebra. >> + * >> + * GNU Zebra is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License as published by the >> + * Free Software Foundation; either version 2, or (at your option) any >> + * later version. >> + * >> + * GNU Zebra is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with GNU Zebra; see the file COPYING. If not, write to the Free >> + * Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA >> + * 02111-1307, USA. >> + */ >> + >> +/* >> + * Support for invoking functions by name from a vty shell. >> + * >> + * This code is only compiled into a developer build, and allows >> + * functions that follow certain signatures to be invoked by name from >> + * the vtysh. >> + */ >> + >> +#include <zebra.h> >> + >> +#include <dlfcn.h> >> + >> +#include "vector.h" >> +#include "vty.h" >> +#include "command.h" >> + >> +#include "vty_invoke.h" >> + >> +typedef int (*vty_invoke_func_t) (int argc, const char **argv); >> + >> +#ifdef DEV_BUILD >> + >> +DEFUN (invoke_function, >> + invoke_function_cmd, >> + "invoke function NAME [ARG1] [ARG2] [ARG3] [ARG4] [ARG5] [ARG6]", >> + "Invoke\n" >> + "Invoke a function\n" >> + "Name of function to invoke\n" >> + "First argument\n" >> + "Second argument\n" >> + "Third argument\n" >> + "Fourth argument\n" >> + "Fifth argument\n" >> + "Sixth argument\n") >> +{ >> + int i; >> + const char *func_name; >> + vty_invoke_func_t func; >> + RUSAGE_T before, after; >> + ulong elapsed; >> + >> + assert (argc >= 1); >> + >> + func_name = argv[0]; >> + >> + func = dlsym (NULL, func_name); >> + if (!func) >> + { >> + vty_out (vty, "Can't find function %s", func_name); >> + return CMD_WARNING; >> + } >> + >> + vty_out (vty, "Invoking %s(", func_name); >> + >> + for (i = 1; i < argc; i++) >> + { >> + vty_out (vty, "%s%s", i == 1 ? "" : ", ", argv[i]); >> + } >> + vty_out (vty, ")\n"); >> + >> + GETRUSAGE (&before); >> + i = func (argc - 1, argv + 1); >> + GETRUSAGE (&after); >> + >> + vty_out (vty, "Return value: %d\n\n", i); >> + >> + elapsed = timeval_elapsed (after.real, before.real); >> + vty_out (vty, "%-20s %9lu ms\n", "Real time:", elapsed / 1000); >> + >> +#ifdef HAVE_RUSAGE >> + { >> + elapsed = timeval_elapsed (after.cpu.ru_utime, before.cpu.ru_utime); >> + >> + vty_out (vty, "%-20s %9lu ms\n", "User time:", elapsed / 1000); >> + >> + elapsed = timeval_elapsed (after.cpu.ru_stime, before.cpu.ru_stime); >> + vty_out (vty, "%-20s %9lu ms\n", "System time:", elapsed / 1000); >> + } >> +#endif >> + >> + return CMD_SUCCESS; >> +} >> + >> +#endif /* DEV_BUILD */ >> + >> +/* >> + * vty_invoke_init >> + */ >> +void >> +vty_invoke_init (void) >> +{ >> +#ifdef DEV_BUILD >> + install_element (ENABLE_NODE, &invoke_function_cmd); >> +#endif >> +} >> diff --git a/lib/vty_invoke.h b/lib/vty_invoke.h >> new file mode 100644 >> index 0000000..a2364a2 >> --- /dev/null >> +++ b/lib/vty_invoke.h >> @@ -0,0 +1,30 @@ >> +/* >> + * vty_invoke.h >> + * >> + * @copyright Copyright (C) 2016 Sproute Networks, Inc. >> + * >> + * @author Avneesh Sachdev <[email protected]> >> + * >> + * This file is part of GNU Zebra. >> + * >> + * GNU Zebra is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License as published by the >> + * Free Software Foundation; either version 2, or (at your option) any >> + * later version. >> + * >> + * GNU Zebra is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with GNU Zebra; see the file COPYING. If not, write to the Free >> + * Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA >> + * 02111-1307, USA. >> + */ >> +#ifndef _VTY_INVOKE_H >> +#define _VTY_INVOKE_H >> + >> +extern void vty_invoke_init (void); >> + >> +#endif /* _VTY_INVOKE_H */ >> diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c >> index b55c671..b0f96d9 100644 >> --- a/vtysh/vtysh.c >> +++ b/vtysh/vtysh.c >> @@ -2149,6 +2149,80 @@ DEFUN (vtysh_start_zsh, >> return CMD_SUCCESS; >> } >> >> +#ifdef DEV_BUILD >> + >> +/* >> + * 'invoke' command >> + * >> + * Command that allows user to invoke functions from the cli shell in >> + * development build. >> + */ >> +DEFUN (invoke_function, >> + invoke_function_cmd, >> + "invoke COMPONENT function NAME [ARG1] [ARG2] [ARG3] [ARG4] >> [ARG5] [ARG6]", >> + "Invoke\n" >> + "Component name in which to invoke function\n" >> + "Invoke a function\n" >> + "Name of function to invoke\n" >> + "First argument\n" >> + "Second argument\n" >> + "Third argument\n" >> + "Fourth argument\n" >> + "Fifth argument\n" >> + "Sixth argument\n") >> +{ >> + unsigned int u; >> + char line[1000]; >> + char *cur, *end; >> + int ret, i; >> + const char *component_name; >> + struct vtysh_client *client; >> + >> + cur = line; >> + end = cur + sizeof (line); >> + >> + cur += snprintf (cur, end - cur, "invoke function "); >> + >> + /* >> + * Build command string, skipping over the component name. >> + */ >> + for (i = 1; i < argc; i++) >> + { >> + ret = snprintf (cur, end - cur, "%s ", argv[i]); >> + if (ret < 0 || ret == (end - cur)) >> + { >> + vty_out (vty, "Command string is too long"); >> + return CMD_WARNING; >> + } >> + cur += ret; >> + } >> + >> + component_name = argv[0]; >> + >> + for (u = 0; u < array_size (vtysh_client); u++) >> + { >> + client = &vtysh_client[u]; >> + if (strcmp (client->name, component_name)) >> + { >> + continue; >> + } >> + if (client->fd < 0) >> + { >> + fprintf (stdout, "Not connected to component %s\n", >> component_name); >> + return CMD_WARNING; >> + } >> + >> + ret = vtysh_client_execute (client, line, stdout); >> + fprintf (stdout, "\n"); >> + return ret; >> + } >> + >> + fprintf (stdout, "Could not find component %s\n", component_name); >> + return CMD_WARNING; >> +} >> + >> +#endif /* DEV_BUILD */ >> + >> static void >> vtysh_install_default (enum node_type node) >> { >> @@ -2530,4 +2604,8 @@ vtysh_init_vty (void) >> install_element (CONFIG_NODE, &vtysh_enable_password_text_cmd); >> install_element (CONFIG_NODE, &no_vtysh_enable_password_cmd); >> >> +#ifdef DEV_BUILD >> + install_element (ENABLE_NODE, &invoke_function_cmd); >> +#endif >> + >> } >> -- >> 1.9.1 >> >> >> _______________________________________________ >> Quagga-dev mailing list >> [email protected] >> https://lists.quagga.net/mailman/listinfo/quagga-dev >> > >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
