Merge pull request #8982 from dlqs/lua-func-stack

bgpd/bgp_routemap.c

@@ -366,38 +366,37 @@ static enum route_map_cmd_result_t
 route_match_script(void *rule, const struct prefix *prefix, void *object)
 {
 	const char *scriptname = rule;
+	const char *routematch_function = "route_match";
 	struct bgp_path_info *path = (struct bgp_path_info *)object;
 
-	struct frrscript *fs = frrscript_load(scriptname, NULL);
+	struct frrscript *fs = frrscript_new(scriptname);
 
-	if (!fs) {
+	if (frrscript_load(fs, routematch_function, NULL)) {
-		zlog_err("Issue loading script rule; defaulting to no match");
+		zlog_err(
+			"Issue loading script or function; defaulting to no match");
 		return RMAP_NOMATCH;
 	}
 
-	enum frrlua_rm_status lrm_status = LUA_RM_FAILURE,
-			      status_nomatch = LUA_RM_NOMATCH,
-			      status_match = LUA_RM_MATCH,
-			      status_match_and_change = LUA_RM_MATCH_AND_CHANGE;
-
 	struct attr newattr = *path->attr;
 
 	int result = frrscript_call(
-		fs, ("RM_FAILURE", (long long *)&lrm_status),
+		fs, routematch_function, ("prefix", prefix),
-		("RM_NOMATCH", (long long *)&status_nomatch),
+		("attributes", &newattr), ("peer", path->peer),
-		("RM_MATCH", (long long *)&status_match),
+		("RM_FAILURE", LUA_RM_FAILURE), ("RM_NOMATCH", LUA_RM_NOMATCH),
-		("RM_MATCH_AND_CHANGE", (long long *)&status_match_and_change),
+		("RM_MATCH", LUA_RM_MATCH),
-		("action", (long long *)&lrm_status), ("prefix", prefix),
+		("RM_MATCH_AND_CHANGE", LUA_RM_MATCH_AND_CHANGE));
-		("attributes", &newattr), ("peer", path->peer));
 
 	if (result) {
 		zlog_err("Issue running script rule; defaulting to no match");
 		return RMAP_NOMATCH;
 	}
 
+	long long *action = frrscript_get_result(fs, routematch_function,
+						 "action", lua_tointegerp);
+
 	int status = RMAP_NOMATCH;
 
-	switch (lrm_status) {
+	switch (*action) {
 	case LUA_RM_FAILURE:
 		zlog_err(
 			"Executing route-map match script '%s' failed; defaulting to no match",
@@ -428,7 +427,9 @@ route_match_script(void *rule, const struct prefix *prefix, void *object)
 		break;
 	}
 
-	frrscript_unload(fs);
+	XFREE(MTYPE_SCRIPT_RES, action);
+
+	frrscript_delete(fs);
 
 	return status;
 }

doc/developer/scripting.rst

@@ -14,8 +14,8 @@ is implemented using the standard Lua C bindings. The supported version of Lua
 is 5.3.
 
 C objects may be passed into Lua and Lua objects may be retrieved by C code via
-a marshalling system. In this way, arbitrary data from FRR may be passed to
+a encoding/decoding system. In this way, arbitrary data from FRR may be passed to
-scripts. It is possible to pass C functions as well.
+scripts.
 
 The Lua environment is isolated from the C environment; user scripts cannot
 access FRR's address space unless explicitly allowed by FRR.
@@ -53,150 +53,290 @@ Reasons against supporting multiple scripting languages:
   with which a given script can be shared
 
 General
-^^^^^^^
+-------
 
-FRR's concept of a script is somewhat abstracted away from the fact that it is
+FRR's scripting functionality is provided in the form of Lua functions in Lua
-Lua underneath. A script in has two things:
+scripts (``.lua`` files). One Lua script may contain many Lua functions. These
-
+are respectively encapsulated in the following structures:
-- name
-- state
-
-In code:
 
 .. code-block:: c
 
    struct frrscript {
-           /* Script name */
+       /* Lua file name */
-           char *name;
+       char *name;
 
-           /* Lua state */
+       /* hash of lua_function_states */
-           struct lua_State *L;
+       struct hash *lua_function_hash;
+   };
+
+   struct lua_function_state {
+       /* Lua function name */
+       char *name;
+
+       lua_State *L;
    };
 
 
-``name`` is simply a string. Everything else is in ``state``, which is itself a
+`struct frrscript`: Since all Lua functions are contained within scripts, the
-Lua library object (``lua_State``). This is an opaque struct that is
+following APIs manipulates this structure. ``name`` contains the
-manipulated using ``lua_*`` functions. The basic ones are imported from
+Lua script name and a hash of Lua functions to their function names.
-``lua.h`` and the rest are implemented within FRR to fill our use cases. The
-thing to remember is that all operations beyond the initial loading the script
-take place on this opaque state object.
 
-There are four basic actions that can be done on a script:
+`struct lua_function_state` is an internal structure, but it essentially contains
+the name of the Lua function and its state (a stack), which is run using Lua
+library functions.
 
-- load
+In general, to run a Lua function, these steps must take place:
-- execute
-- query state
-- unload
 
-They are typically done in this order.
+- Initialization
+- Load
+- Call
+- Delete
 
-
+Initialization
-Loading
-^^^^^^^
-
-A snippet of Lua code is referred to as a "chunk". These are simply text. FRR
-presently assumes chunks are located in individual files specific to one task.
-These files are stored in the scripts directory and must end in ``.lua``.
-
-A script object is created by loading a script. This is done with
-``frrscript_load()``. This function takes the name of the script and an
-optional callback function. The string ".lua" is appended to the script name,
-and the resultant filename is looked for in the scripts directory.
-
-For example, to load ``/etc/frr/scripts/bingus.lua``:
-
-.. code-block:: c
-
-   struct frrscript *fs = frrscript_load("bingus", NULL);
-
-During loading the script is validated for syntax and its initial environment
-is setup. By default this does not include the Lua standard library; there are
-security issues to consider, though for practical purposes untrusted users
-should not be able to write the scripts directory anyway. If desired the Lua
-standard library may be added to the script environment using
-``luaL_openlibs(fs->L)`` after loading the script. Further information on
-setting up the script environment is in the Lua manual.
-
-
-Executing
-^^^^^^^^^
-
-After loading, scripts may be executed. A script may take input in the form of
-variable bindings set in its environment prior to being run, and may provide
-results by setting the value of variables. Arbitrary C values may be
-transferred into the script environment, including functions.
-
-A typical execution call looks something like this:
-
-.. code-block:: c
-
-   struct frrscript *fs = frrscript_load(...);
-
-   int status_ok = 0, status_fail = 1;
-   struct prefix p = ...;
-
-   int result = frrscript_call(fs,
-                ("STATUS_FAIL", &status_fail),
-                ("STATUS_OK", &status_ok),
-                ("prefix", &p));
-
-
-To execute a loaded script, we need to define the inputs. These inputs are
-passed in by binding values to variable names that will be accessible within the
-Lua environment. Basically, all communication with the script takes place via
-global variables within the script, and to provide inputs we predefine globals
-before the script runs. This is done by passing ``frrscript_call()`` a list of
-parenthesized pairs, where the first and second fields identify, respectively,
-the name of the global variable within the script environment and the value it
-is bound to.
-
-The script is then executed and returns a general status code. In the success
-case this will be 0, otherwise it will be nonzero. The script itself does not
-determine this code, it is provided by the Lua interpreter.
-
-
-Querying State
 ^^^^^^^^^^^^^^
 
-.. todo::
+The ``frrscript`` object encapsulates the Lua function state(s) from
+one Lua script file. To create, use ``frrscript_new()`` which takes the
+name of the Lua script.
+The string ".lua" is appended to the script name, and the resultant filename
+will be used to look for the script when we want to load a Lua function from it.
 
-   This section will be updated once ``frrscript_get_result`` has been
+For example, to create ``frrscript`` for ``/etc/frr/scripts/bingus.lua``:
-   updated to work with the new ``frrscript_call`` and the rest of the new API.
-
-
-Unloading
-^^^^^^^^^
-
-To destroy a script and its associated state:
 
 .. code-block:: c
 
-   frrscript_unload(fs);
+   struct frrscript *fs = frrscript_new("bingus");
 
 
-.. _marshalling:
+The script is *not* read at this stage.
+This function cannot be used to test for a script's presence.
 
-Marshalling
+Load
-^^^^^^^^^^^
+^^^^
+
+The function to be called must first be loaded. Use ``frrscript_load()``
+which takes a ``frrscript`` object, the name of the Lua function
+and a callback function.
+
+For example, to load the Lua function ``on_foo``
+in ``/etc/frr/scripts/bingus.lua``:
+
+.. code-block:: c
+
+   int ret = frrscript_load(fs, "on_foo", NULL);
+
+
+This function returns 0 if and only if the Lua function was successfully loaded.
+A non-zero return could indicate either a missing Lua script, a missing
+Lua function, or an error when loading the function.
+
+During loading the script is validated for syntax and its environment
+is set up. By default this does not include the Lua standard library; there are
+security issues to consider, though for practical purposes untrusted users
+should not be able to write the scripts directory anyway.
+
+Call
+^^^^
+
+After loading, Lua functions may be called.
+
+Input
+"""""
+
+Inputs to the Lua script should be given by providing a list of parenthesized
+pairs,
+where the first and second field identify the name of the variable and the
+value it is bound to, respectively.
+The types of the values must have registered encoders (more below); the compiler
+will warn you otherwise.
+
+These variables are first encoded in-order, then provided as arguments
+to the Lua function. In the example, note that ``c`` is passed in as a value
+while ``a`` and ``b`` are passed in as pointers.
+
+.. code-block:: c
+
+   int a = 100, b = 200, c = 300;
+   frrscript_call(fs, "on_foo", ("a", &a), ("b", &b), ("c", c));
+
+
+.. code-block:: lua
+
+   function on_foo(a, b, c)
+     -- a is 100, b is 200, c is 300
+     ...
+
+
+Output
+""""""
+
+.. code-block:: c
+
+   int a = 100, b = 200, c = 300;
+   frrscript_call(fs, "on_foo", ("a", &a), ("b", &b), ("c", c));
+   // a is 500, b is 200, c is 300
+
+   int* d = frrscript_get_result(fs, "on_foo", "d", lua_tointegerp);
+   // d is 800
+
+
+.. code-block:: lua
+
+   function on_foo(a, b, c)
+     b = 600
+     return { ["a"] = 500, ["c"] = 700, ["d"] = 800 }
+   end
+
+
+**Lua functions being called must return a single table of string names to
+values.**
+(Lua functions should return an empty table if there is no output.)
+The keys of the table are mapped back to names of variables in C. Note that
+the values in the table can also be tables. Since tables are Lua's primary
+data structure, this design lets us return any Lua value.
+
+After the Lua function returns, the names of variables  to ``frrscript_call()``
+are matched against keys of the returned table, and then decoded. The types
+being decoded must have registered decoders (more below); the compiler will
+warn you otherwise.
+
+In the example, since ``a`` was in the returned table and ``b`` was not,
+``a`` was decoded and its value modified, while ``b`` was not decoded.
+``c`` was decoded as well, but its decoder is a noop.
+What modifications happen given a variable depends whether its name was
+in the returned table and the decoder's implementation.
+
+.. warning::
+   Always keep in mind that non const-qualified pointers in
+   ``frrscript_call()`` may be modified - this may be a source of bugs.
+   On the other hand, const-qualified pointers and other values cannot
+   be modified.
+
+
+.. tip::
+   You can make a copy of a data structure and pass that in instead,
+   so that modifications only happen to that copy.
+
+``frrscript_call()`` returns 0 if and only if the Lua function was successfully
+called. A non-zero return could indicate either a missing Lua script, a missing
+Lua function, or an error from the Lua interpreter.
+
+In the above example, ``d`` was not an input to ``frrscript_call()``, so its
+value must be explicitly retrieved with ``frrscript_get_result``.
+
+``frrscript_get_result()`` takes a
+decoder and string name which is used as a key to search the returned table.
+Returns the pointer to the decoded value, or NULL if it was not found.
+In the example, ``d`` is a "new" value in C space,
+so memory allocation might take place. Hence the caller is
+responsible for memory deallocation.
+
+
+Delete
+^^^^^^
+
+To delete a script and the all Lua states associated with it:
+
+.. code-block:: c
+
+   frrscript_delete(fs);
+
+
+A complete example
+""""""""""""""""""
+
+So, a typical execution call, with error checking, looks something like this:
+
+.. code-block:: c
+
+   struct frrscript *fs = frrscript_new("my_script"); // name *without* .lua
+
+   int ret = frrscript_load(fs, "on_foo", NULL);
+   if (ret != 0)
+       goto DONE; // Lua script or function might have not been found
+
+   int a = 100, b = 200, c = 300;
+   ret = frrscript_call(fs, "on_foo", ("a", &a), ("b", &b), ("c", c));
+   if (ret != 0)
+       goto DONE; // Lua function might have not successfully run
+
+   // a and b might be modified
+   assert(a == 500);
+   assert(b == 200);
+
+   // c could not have been modified
+   assert(c == 300);
+
+   // d is new
+   int* d = frrscript_get_result(fs, "on_foo", "d", lua_tointegerp);
+
+   if (!d)
+       goto DONE; // "d" might not have been in returned table
+
+   assert(*d == 800);
+   XFREE(MTYPE_SCRIPT_RES, d); // caller responsible for free
+
+   DONE:
+   frrscript_delete(fs);
+
+
+.. code-block:: lua
+
+   function on_foo(a, b, c)
+     b = 600
+     return { a = 500, c = 700, d = 800 }
+   end
+
+
+Note that ``{ a = ...`` is same as ``{ ["a"] = ...``; it is Lua shorthand to
+use the variable name as the key in a table.
+
+Encoding and Decoding
+^^^^^^^^^^^^^^^^^^^^^
 
 Earlier sections glossed over the types of values that can be passed into
-``frrscript_call`` and how data is passed between C and Lua. Lua, as a dynamically
+``frrscript_call()`` and how data is passed between C and Lua. Lua, as a
-typed, garbage collected language, cannot directly use C values without some
+dynamically typed, garbage collected language, cannot directly use C values
-kind of marshalling / unmarshalling system to translate types between the two
+without some kind of encoding / decoding system to
-runtimes.
+translate types between the two runtimes.
 
 Lua communicates with C code using a stack. C code wishing to provide data to
-Lua scripts must provide a function that marshalls the C data into a Lua
+Lua scripts must provide a function that encodes the C data into a Lua
 representation and pushes it on the stack. C code wishing to retrieve data from
-Lua must provide a corresponding unmarshalling function that retrieves a Lua
+Lua must provide a corresponding decoder function that retrieves a Lua
-value from the stack and converts it to the corresponding C type. These
+value from the stack and converts it to the corresponding C type.
-functions are known as encoders and decoders in FRR.
 
-An encoder is a function that takes a ``lua_State *`` and a C type and pushes
+Encoders and decoders are provided for common data types.
-onto the Lua stack a value representing the C type. For C structs, the usual
+Developers wishing to pass their own data structures between C and Lua need to
-case, this will typically be a Lua table (tables are the only datastructure Lua
+create encoders and decoders for that data type.
-has). For example, here is the encoder function for ``struct prefix``:
 
+We try to keep them named consistently.
+There are three kinds of encoders and decoders:
+
+1. lua_push*: encodes a value onto the Lua stack.
+   Required for ``frrscript_call``.
+
+2. lua_decode*: decodes a value from the Lua stack.
+   Required for ``frrscript_call``.
+   Only non const-qualified pointers may be actually decoded (more below).
+
+3. lua_to*: allocates memory and decodes a value from the Lua stack.
+   Required for ``frrscript_get_result``.
+
+This design allows us to combine typesafe *modification* of C values as well as
+*allocation* of new C values.
+
+In the following sections, we will use the encoders/decoders for ``struct prefix`` as an example.
+
+Encoding
+""""""""
+
+An encoder function takes a ``lua_State *``, a C type and pushes that value onto
+the Lua state (a stack).
+For C structs, the usual case,
+this will typically be encoded to a Lua table, then pushed onto the Lua stack.
+
+Here is the encoder function for ``struct prefix``:
 
 .. code-block:: c
 
@@ -204,8 +344,6 @@ has). For example, here is the encoder function for ``struct prefix``:
    {
            char buffer[PREFIX_STRLEN];
 
-           zlog_debug("frrlua: pushing prefix table");
-
            lua_newtable(L);
            lua_pushstring(L, prefix2str(prefix, buffer, PREFIX_STRLEN));
            lua_setfield(L, -2, "network");
@@ -215,7 +353,7 @@ has). For example, here is the encoder function for ``struct prefix``:
            lua_setfield(L, -2, "family");
    }
 
-This function pushes a single value onto the Lua stack. It is a table whose
+This function pushes a single value, a table, onto the Lua stack, whose
 equivalent in Lua is:
 
 .. code-block:: c
@@ -223,16 +361,23 @@ equivalent in Lua is:
    { ["network"] = "1.2.3.4/24", ["prefixlen"] = 24, ["family"] = 2 }
 
 
+Decoding
+""""""""
+
 Decoders are a bit more involved. They do the reverse; a decoder function takes
 a ``lua_State *``, pops a value off the Lua stack and converts it back into its
 C type.
-However, since Lua programs have the ability to directly modify their inputs
-(i.e. values passed in via ``frrscript_call``), we need two separate decoder
-functions, called ``lua_decode_*`` and ``lua_to*``.
 
-A ``lua_decode_*`` function takes a ``lua_State*``, an index, and a C type, and
+There are two: ``lua_decode*`` and ``lua_to*``. The former does no mememory
-unmarshalls a Lua value into that C type.
+allocation and is needed for ``frrscript_call``.
-Again, for ``struct prefix``:
+The latter performs allocation and is optional.
+
+A ``lua_decode_*`` function takes a ``lua_State*``, an index, and a pointer
+to a C data structure, and directly modifies the structure with values from the
+Lua stack. Note that only non const-qualified pointers may be modified;
+``lua_decode_*`` for other types will be noops.
+
+Again, for ``struct prefix *``:
 
 .. code-block:: c
 
@@ -240,29 +385,52 @@ Again, for ``struct prefix``:
    {
         lua_getfield(L, idx, "network");
         (void)str2prefix(lua_tostring(L, -1), prefix);
+        /* pop the netork string */
         lua_pop(L, 1);
-        /* pop the table */
+        /* pop the prefix table */
         lua_pop(L, 1);
    }
 
+
+Note:
+ - Before ``lua_decode*`` is run, the "prefix" table is already on the top of
+   the stack. ``frrscript_call`` does this for us.
+ - However, at the end of ``lua_decode*``, the "prefix" table should be popped.
+ - The other two fields in the "network" table are disregarded, meaning that any
+   modification to them is discarded in C space. In this case, this is desired
+   behavior.
+
 .. warning::
 
-   ``lua_decode_prefix`` functions should leave the Lua stack completely empty
+   ``lua_decode*`` functions should pop all values that ``lua_to*`` pushed onto
-   when they return.
+   the Lua stack.
-   For decoders that unmarshall fields from tables, remember to pop the table
+   For encoders that pushed a table, its decoder should pop the table at the end.
-   at the end.
+   The above is an example.
 
 
-A ``lua_to*`` function perform a similar role except that it first allocates
+
-memory for the new C type before decoding the value from the Lua stack, then
+``int`` is not a non const-qualified pointer, so for ``int``:
-returns a pointer to the newly allocated C type.
+
+.. code-block:: c
+
+   void lua_decode_int_noop(lua_State *L, int idx, int i)
+   { //noop
+   }
+
+
+A ``lua_to*`` function provides identical functionality except that it first
+allocates memory for the new C type before decoding the value from the Lua stack,
+then returns a pointer to the newly allocated C type. You only need to implement
+this function to use with ``frrscript_get_result`` to retrieve a result of
+this type.
+
 This function can and should be implemented using ``lua_decode_*``:
 
 .. code-block:: c
 
    void *lua_toprefix(lua_State *L, int idx)
    {
-           struct prefix *p = XCALLOC(MTYPE_TMP, sizeof(struct prefix));
+           struct prefix *p = XCALLOC(MTYPE_SCRIPT_RES, sizeof(struct prefix));
 
            lua_decode_prefix(L, idx, p);
            return p;
@@ -270,22 +438,15 @@ This function can and should be implemented using ``lua_decode_*``:
 
 
 The returned data must always be copied off the stack and the copy must be
-allocated with ``MTYPE_TMP``. This way it is possible to unload the script
+allocated with ``MTYPE_SCRIPT_RES``. This way it is possible to unload the script
 (destroy the state) without invalidating any references to values stored in it.
 Note that it is the caller's responsibility to free the data.
 
-For consistency, we should always name functions of the first type
-``lua_decode_*``.
-Functions of the second type should be named ``lua_to*``, as this is the
-naming convention used by the Lua C library for the basic types e.g.
-``lua_tointeger`` and ``lua_tostring``.
 
-This two-function design allows the compiler to warn if a value passed into
+Registering encoders and decoders for frrscript_call
-``frrscript_call`` does not have a encoder and decoder for that type.
+""""""""""""""""""""""""""""""""""""""""""""""""""""
-The ``lua_to*`` functions enable us to easily create decoders for nested
-structures.
 
-To register a new type with its corresponding encoding and decoding functions,
+To register a new type with its ``lua_push*`` and ``lua_decode*`` functions,
 add the mapping in the following macros in ``frrscript.h``:
 
 .. code-block:: diff
@@ -296,7 +457,7 @@ add the mapping in the following macros in ``frrscript.h``:
    - struct peer * : lua_pushpeer \
    + struct peer * : lua_pushpeer, \
    + struct prefix * : lua_pushprefix \
-     )(L, value)
+     )((L), (value))
 
      #define DECODE_ARGS_WITH_STATE(L, value) \
           _Generic((value), \
@@ -304,7 +465,7 @@ add the mapping in the following macros in ``frrscript.h``:
    - struct peer * : lua_decode_peer \
    + struct peer * : lua_decode_peer, \
    + struct prefix * : lua_decode_prefix \
-     )(L, -1, value)
+     )((L), -1, (value))
 
 
 At compile time, the compiler will search for encoders/decoders for the type of
@@ -331,11 +492,12 @@ For that, use ``lua_decode_noop``:
 
 .. note::
 
-   Marshalled types are not restricted to simple values like integers, strings
+   Encodable/decodable types are not restricted to simple values like integers,
-   and tables. It is possible to marshall a type such that the resultant object
+   strings and tables.
-   in Lua is an actual object-oriented object, complete with methods that call
+   It is possible to encode a type such that the resultant object in Lua
-   back into defined C functions. See the Lua manual for how to do this; for a
+   is an actual object-oriented object, complete with methods that call
-   code example, look at how zlog is exported into the script environment.
+   back into defined C functions. See the Lua manual for how to do this;
+   for a code example, look at how zlog is exported into the script environment.
 
 
 Script Environment
@@ -364,10 +526,11 @@ Examples
 For a complete code example involving passing custom types, retrieving results,
 and doing complex calculations in Lua, look at the implementation of the
 ``match script SCRIPT`` command for BGP routemaps. This example calls into a
-script with a route prefix and attributes received from a peer and expects the
+script with a function named ``route_match``,
-script to return a match / no match / match and update result.
+provides route prefix and attributes received from a peer and expects the
+function to return a match / no match / match and update result.
 
-An example script to use with this follows. This script matches, does not match
+An example script to use with this follows. This function matches, does not match
 or updates a route depending on how many BGP UPDATE messages the peer has
 received when the script is called, simply as a demonstration of what can be
 accomplished with scripting.
@@ -378,64 +541,75 @@ accomplished with scripting.
    -- Example route map matching
    -- author: qlyoung
    --
-   -- The following variables are available to us:
+   -- The following variables are available in the global environment:
    --   log
    --     logging library, with the usual functions
-   --   prefix
+   --
+   -- route_match arguments:
+   --   table prefix
    --     the route under consideration
-   --   attributes
+   --   table attributes
    --     the route's attributes
-   --   peer
+   --   table peer
    --     the peer which received this route
-   --   RM_FAILURE
+   --   integer RM_FAILURE
    --     status code in case of failure
-   --   RM_NOMATCH
+   --   integer RM_NOMATCH
    --     status code for no match
-   --   RM_MATCH
+   --   integer RM_MATCH
    --     status code for match
-   --   RM_MATCH_AND_CHANGE
+   --   integer RM_MATCH_AND_CHANGE
    --     status code for match-and-set
    --
-   -- We need to set the following out values:
+   -- route_match returns table with following keys:
-   --   action
+   --   integer action, required
-   --      Set to the appropriate status code to indicate what we did
+   --     resultant status code. Should be one of RM_*
-   --   attributes
+   --   table attributes, optional
-   --      Setting fields on here will propagate them back up to the caller if
+   --     updated route attributes
-   --      'action' is set to RM_MATCH_AND_CHANGE.
+   --
-   
-   
-   log.info("Evaluating route " .. prefix.network .. " from peer " .. peer.remote_id.string)
-   
-   function on_match (prefix, attrs)
-           log.info("Match")
-           action = RM_MATCH
-   end
-   
-   function on_nomatch (prefix, attrs)
-           log.info("No match")
-           action = RM_NOMATCH
-   end
-   
-   function on_match_and_change (prefix, attrs)
-           action = RM_MATCH_AND_CHANGE
-           log.info("Match and change")
-           attrs["metric"] = attrs["metric"] + 7
-   end
-   
-   special_routes = {
-           ["172.16.10.4/24"] = on_match,
-           ["172.16.13.1/8"] = on_nomatch,
-           ["192.168.0.24/8"] = on_match_and_change,
-   }
-   
-   
-   if special_routes[prefix.network] then
-           special_routes[prefix.network](prefix, attributes)
-   elseif peer.stats.update_in % 3 == 0 then
-           on_match(prefix, attributes)
-   elseif peer.stats.update_in % 2 == 0 then
-           on_nomatch(prefix, attributes)
-   else
-           on_match_and_change(prefix, attributes)
-   end
 
+   function route_match(prefix, attributes, peer,
+           RM_FAILURE, RM_NOMATCH, RM_MATCH, RM_MATCH_AND_CHANGE)
+
+           log.info("Evaluating route " .. prefix.network .. " from peer " .. peer.remote_id.string)
+   
+           function on_match (prefix, attributes)
+                   log.info("Match")
+                   return {
+                           attributes = RM_MATCH
+                   }
+           end
+   
+           function on_nomatch (prefix, attributes)
+                   log.info("No match")
+                   return {
+                           action = RM_NOMATCH
+                   }
+           end
+
+           function on_match_and_change (prefix, attributes)
+                   log.info("Match and change")
+                   attributes["metric"] = attributes["metric"] + 7
+                   return {
+                           action = RM_MATCH_AND_CHANGE,
+                           attributes = attributes
+                   }
+           end
+
+           special_routes = {
+                   ["172.16.10.4/24"] = on_match,
+                   ["172.16.13.1/8"] = on_nomatch,
+                   ["192.168.0.24/8"] = on_match_and_change,
+           }
+
+
+           if special_routes[prefix.network] then
+                   return special_routes[prefix.network](prefix, attributes)
+           elseif peer.stats.update_in % 3 == 0 then
+                   return on_match(prefix, attributes)
+           elseif peer.stats.update_in % 2 == 0 then
+                   return on_nomatch(prefix, attributes)
+           else
+                   return on_match_and_change(prefix, attributes)
+           end
+    end

lib/command.c

@@ -2422,28 +2422,30 @@ DEFUN(find,
 }
 
 #if defined(DEV_BUILD) && defined(HAVE_SCRIPTING)
-DEFUN(script,
+DEFUN(script, script_cmd, "script SCRIPT FUNCTION",
-      script_cmd,
+      "Test command - execute a function in a script\n"
-      "script SCRIPT",
+      "Script name (same as filename in /etc/frr/scripts/)\n"
-      "Test command - execute a script\n"
+      "Function name (in the script)\n")
-      "Script name (same as filename in /etc/frr/scripts/\n")
 {
 	struct prefix p;
 
 	(void)str2prefix("1.2.3.4/24", &p);
-	struct frrscript *fs = frrscript_load(argv[1]->arg, NULL);
+	struct frrscript *fs = frrscript_new(argv[1]->arg);
 
-	if (fs == NULL) {
+	if (frrscript_load(fs, argv[2]->arg, NULL)) {
-		vty_out(vty, "Script '/etc/frr/scripts/%s.lua' not found\n",
+		vty_out(vty,
-			argv[1]->arg);
+			"/etc/frr/scripts/%s.lua or function '%s' not found\n",
-	} else {
+			argv[1]->arg, argv[2]->arg);
-		int ret = frrscript_call(fs, ("p", &p));
-		char buf[40];
-		prefix2str(&p, buf, sizeof(buf));
-		vty_out(vty, "p: %s\n", buf);
-		vty_out(vty, "Script result: %d\n", ret);
 	}
 
+	int ret = frrscript_call(fs, argv[2]->arg, ("p", &p));
+	char buf[40];
+	prefix2str(&p, buf, sizeof(buf));
+	vty_out(vty, "p: %s\n", buf);
+	vty_out(vty, "Script result: %d\n", ret);
+
+	frrscript_delete(fs);
+
 	return CMD_SUCCESS;
 }
 #endif

lib/frrlua.c

@@ -29,6 +29,8 @@
 #include "log.h"
 #include "buffer.h"
 
+DEFINE_MTYPE(LIB, SCRIPT_RES, "Scripting results");
+
 /* Lua stuff */
 
 /*
@@ -81,7 +83,7 @@ void lua_decode_prefix(lua_State *L, int idx, struct prefix *prefix)
 
 void *lua_toprefix(lua_State *L, int idx)
 {
-	struct prefix *p = XCALLOC(MTYPE_TMP, sizeof(struct prefix));
+	struct prefix *p = XCALLOC(MTYPE_SCRIPT_RES, sizeof(struct prefix));
 	lua_decode_prefix(L, idx, p);
 	return p;
 }
@@ -153,7 +155,8 @@ void lua_decode_interface(lua_State *L, int idx, struct interface *ifp)
 }
 void *lua_tointerface(lua_State *L, int idx)
 {
-	struct interface *ifp = XCALLOC(MTYPE_TMP, sizeof(struct interface));
+	struct interface *ifp =
+		XCALLOC(MTYPE_SCRIPT_RES, sizeof(struct interface));
 
 	lua_decode_interface(L, idx, ifp);
 	return ifp;
@@ -183,7 +186,8 @@ void lua_decode_inaddr(lua_State *L, int idx, struct in_addr *inaddr)
 
 void *lua_toinaddr(lua_State *L, int idx)
 {
-	struct in_addr *inaddr = XCALLOC(MTYPE_TMP, sizeof(struct in_addr));
+	struct in_addr *inaddr =
+		XCALLOC(MTYPE_SCRIPT_RES, sizeof(struct in_addr));
 	lua_decode_inaddr(L, idx, inaddr);
 	return inaddr;
 }
@@ -213,7 +217,8 @@ void lua_decode_in6addr(lua_State *L, int idx, struct in6_addr *in6addr)
 
 void *lua_toin6addr(lua_State *L, int idx)
 {
-	struct in6_addr *in6addr = XCALLOC(MTYPE_TMP, sizeof(struct in6_addr));
+	struct in6_addr *in6addr =
+		XCALLOC(MTYPE_SCRIPT_RES, sizeof(struct in6_addr));
 	lua_decode_in6addr(L, idx, in6addr);
 	return in6addr;
 }
@@ -243,7 +248,8 @@ void lua_decode_sockunion(lua_State *L, int idx, union sockunion *su)
 
 void *lua_tosockunion(lua_State *L, int idx)
 {
-	union sockunion *su = XCALLOC(MTYPE_TMP, sizeof(union sockunion));
+	union sockunion *su =
+		XCALLOC(MTYPE_SCRIPT_RES, sizeof(union sockunion));
 
 	lua_decode_sockunion(L, idx, su);
 	return su;
@@ -262,7 +268,7 @@ void lua_decode_timet(lua_State *L, int idx, time_t *t)
 
 void *lua_totimet(lua_State *L, int idx)
 {
-	time_t *t = XCALLOC(MTYPE_TMP, sizeof(time_t));
+	time_t *t = XCALLOC(MTYPE_SCRIPT_RES, sizeof(time_t));
 
 	lua_decode_timet(L, idx, t);
 	return t;
@@ -283,7 +289,7 @@ void lua_decode_integerp(lua_State *L, int idx, long long *num)
 
 void *lua_tointegerp(lua_State *L, int idx)
 {
-	long long *num = XCALLOC(MTYPE_TMP, sizeof(long long));
+	long long *num = XCALLOC(MTYPE_SCRIPT_RES, sizeof(long long));
 
 	lua_decode_integerp(L, idx, num);
 	return num;
@@ -297,7 +303,7 @@ void lua_decode_stringp(lua_State *L, int idx, char *str)
 
 void *lua_tostringp(lua_State *L, int idx)
 {
-	char *string = XSTRDUP(MTYPE_TMP, lua_tostring(L, idx));
+	char *string = XSTRDUP(MTYPE_SCRIPT_RES, lua_tostring(L, idx));
 
 	return string;
 }
@@ -309,6 +315,14 @@ void lua_decode_noop(lua_State *L, int idx, const void *ptr)
 {
 }
 
+
+/*
+ * Noop decoder for int.
+ */
+void lua_decode_integer_noop(lua_State *L, int idx, int i)
+{
+}
+
 /*
  * Logging.
  *

lib/frrlua.h

@@ -34,6 +34,8 @@
 extern "C" {
 #endif
 
+DECLARE_MTYPE(SCRIPT_RES);
+
 /*
  * gcc-10 is complaining about the wrapper function
  * not being compatible with lua_pushstring returning
@@ -162,10 +164,12 @@ void lua_decode_stringp(lua_State *L, int idx, char *str);
 void *lua_tostringp(lua_State *L, int idx);
 
 /*
- * No-op decocder
+ * No-op decoders
  */
 void lua_decode_noop(lua_State *L, int idx, const void *ptr);
 
+void lua_decode_integer_noop(lua_State *L, int idx, int i);
+
 /*
  * Retrieve an integer from table on the top of the stack.
  *

lib/frrscript.c

@@ -102,67 +102,136 @@ static void codec_free(struct codec *c)
 }
 #endif
 
-/* Generic script APIs */
+/* Lua function hash utils */
 
-int _frrscript_call(struct frrscript *fs)
+unsigned int lua_function_hash_key(const void *data)
+{
+	const struct lua_function_state *lfs = data;
+
+	return string_hash_make(lfs->name);
+}
+
+bool lua_function_hash_cmp(const void *d1, const void *d2)
+{
+	const struct lua_function_state *lfs1 = d1;
+	const struct lua_function_state *lfs2 = d2;
+
+	return strmatch(lfs1->name, lfs2->name);
+}
+
+void *lua_function_alloc(void *arg)
+{
+	struct lua_function_state *tmp = arg;
+
+	struct lua_function_state *lfs =
+		XCALLOC(MTYPE_SCRIPT, sizeof(struct lua_function_state));
+	lfs->name = tmp->name;
+	lfs->L = tmp->L;
+	return lfs;
+}
+
+static void lua_function_free(struct hash_bucket *b, void *data)
+{
+	struct lua_function_state *lfs = (struct lua_function_state *)b->data;
+	lua_close(lfs->L);
+	XFREE(MTYPE_SCRIPT, lfs);
+}
+
+/* internal frrscript APIs */
+
+int _frrscript_call_lua(struct lua_function_state *lfs, int nargs)
 {
 
-	int ret = lua_pcall(fs->L, 0, 0, 0);
+	int ret;
+	ret = lua_pcall(lfs->L, nargs, 1, 0);
 
 	switch (ret) {
 	case LUA_OK:
 		break;
 	case LUA_ERRRUN:
-		zlog_err("Script '%s' runtime error: %s", fs->name,
+		zlog_err("Lua hook call '%s' : runtime error: %s", lfs->name,
-			 lua_tostring(fs->L, -1));
+			 lua_tostring(lfs->L, -1));
 		break;
 	case LUA_ERRMEM:
-		zlog_err("Script '%s' memory error: %s", fs->name,
+		zlog_err("Lua hook call '%s' : memory error: %s", lfs->name,
-			 lua_tostring(fs->L, -1));
+			 lua_tostring(lfs->L, -1));
 		break;
 	case LUA_ERRERR:
-		zlog_err("Script '%s' error handler error: %s", fs->name,
+		zlog_err("Lua hook call '%s' : error handler error: %s",
-			 lua_tostring(fs->L, -1));
+			 lfs->name, lua_tostring(lfs->L, -1));
 		break;
 	case LUA_ERRGCMM:
-		zlog_err("Script '%s' garbage collector error: %s", fs->name,
+		zlog_err("Lua hook call '%s' : garbage collector error: %s",
-			 lua_tostring(fs->L, -1));
+			 lfs->name, lua_tostring(lfs->L, -1));
 		break;
 	default:
-		zlog_err("Script '%s' unknown error: %s", fs->name,
+		zlog_err("Lua hook call '%s' : unknown error: %s", lfs->name,
-			 lua_tostring(fs->L, -1));
+			 lua_tostring(lfs->L, -1));
 		break;
 	}
 
 	if (ret != LUA_OK) {
-		lua_pop(fs->L, 1);
+		lua_pop(lfs->L, 1);
 		goto done;
 	}
 
+	if (lua_gettop(lfs->L) != 1) {
+		zlog_err(
+			"Lua hook call '%s': Lua function should return only 1 result",
+			lfs->name);
+		ret = 1;
+		goto done;
+	}
+
+	if (lua_istable(lfs->L, 1) != 1) {
+		zlog_err(
+			"Lua hook call '%s': Lua function should return a Lua table",
+			lfs->name);
+		ret = 1;
+	}
+
 done:
 	/* LUA_OK is 0, so we can just return lua_pcall's result directly */
 	return ret;
 }
 
-void *frrscript_get_result(struct frrscript *fs,
+void *frrscript_get_result(struct frrscript *fs, const char *function_name,
-			   const struct frrscript_env *result)
+			   const char *name,
+			   void *(*lua_to)(lua_State *L, int idx))
 {
-	void *r;
+	void *p;
-	struct frrscript_codec c = {.typename = result->typename};
+	struct lua_function_state *lfs;
+	struct lua_function_state lookup = {.name = function_name};
 
-	struct frrscript_codec *codec = hash_lookup(codec_hash, &c);
+	lfs = hash_lookup(fs->lua_function_hash, &lookup);
-	assert(codec && "No encoder for type");
 
-	if (!codec->decoder) {
+	if (lfs == NULL)
-		zlog_err("No script decoder for type '%s'", result->typename);
+		return NULL;
+
+	/* At this point, the Lua state should have only the returned table.
+	 * We will then search the table for the key/value we're interested in.
+	 * Then if the value is present (i.e. non-nil), call the lua_to*
+	 * decoder.
+	 */
+	assert(lua_gettop(lfs->L) == 1);
+	assert(lua_istable(lfs->L, -1) == 1);
+	lua_getfield(lfs->L, -1, name);
+	if (lua_isnil(lfs->L, -1)) {
+		lua_pop(lfs->L, 1);
+		zlog_warn(
+			"frrscript: '%s.lua': '%s': tried to decode '%s' as result but failed",
+			fs->name, function_name, name);
 		return NULL;
 	}
+	p = lua_to(lfs->L, 2);
 
-	lua_getglobal(fs->L, result->name);
+	/* At the end, the Lua state should be same as it was at the start
-	r = codec->decoder(fs->L, -1);
+	 * i.e. containing soley the returned table.
-	lua_pop(fs->L, 1);
+	 */
+	assert(lua_gettop(lfs->L) == 1);
+	assert(lua_istable(lfs->L, -1) == 1);
 
-	return r;
+	return p;
 }
 
 void frrscript_register_type_codec(struct frrscript_codec *codec)
@@ -183,61 +252,99 @@ void frrscript_register_type_codecs(struct frrscript_codec *codecs)
 		frrscript_register_type_codec(&codecs[i]);
 }
 
-struct frrscript *frrscript_load(const char *name,
+struct frrscript *frrscript_new(const char *name)
-				 int (*load_cb)(struct frrscript *))
 {
 	struct frrscript *fs = XCALLOC(MTYPE_SCRIPT, sizeof(struct frrscript));
 
 	fs->name = XSTRDUP(MTYPE_SCRIPT, name);
-	fs->L = luaL_newstate();
+	fs->lua_function_hash =
-	frrlua_export_logging(fs->L);
+		hash_create(lua_function_hash_key, lua_function_hash_cmp,
+			    "Lua function state hash");
+	return fs;
+}
 
-	char fname[MAXPATHLEN * 2];
+int frrscript_load(struct frrscript *fs, const char *function_name,
-	snprintf(fname, sizeof(fname), "%s/%s.lua", scriptdir, fs->name);
+		   int (*load_cb)(struct frrscript *))
+{
 
-	int ret = luaL_loadfile(fs->L, fname);
+	/* Set up the Lua script */
+	lua_State *L = luaL_newstate();
+
+	frrlua_export_logging(L);
+
+	char script_name[MAXPATHLEN];
+
+	if (snprintf(script_name, sizeof(script_name), "%s/%s.lua", scriptdir,
+		     fs->name)
+	    >= (int)sizeof(script_name)) {
+		zlog_err("frrscript: path to script %s/%s.lua is too long",
+			 scriptdir, fs->name);
+		goto fail;
+	}
+	int ret = luaL_dofile(L, script_name);
 
 	switch (ret) {
 	case LUA_OK:
 		break;
 	case LUA_ERRSYNTAX:
-		zlog_err("Failed loading script '%s': syntax error: %s", fname,
+		zlog_err(
-			 lua_tostring(fs->L, -1));
+			"frrscript: failed loading script '%s.lua': syntax error: %s",
+			script_name, lua_tostring(L, -1));
 		break;
 	case LUA_ERRMEM:
-		zlog_err("Failed loading script '%s': out-of-memory error: %s",
+		zlog_err(
-			 fname, lua_tostring(fs->L, -1));
+			"frrscript: failed loading script '%s.lua': out-of-memory error: %s",
+			script_name, lua_tostring(L, -1));
 		break;
 	case LUA_ERRGCMM:
 		zlog_err(
-			"Failed loading script '%s': garbage collector error: %s",
+			"frrscript: failed loading script '%s.lua': garbage collector error: %s",
-			fname, lua_tostring(fs->L, -1));
+			script_name, lua_tostring(L, -1));
 		break;
 	case LUA_ERRFILE:
-		zlog_err("Failed loading script '%s': file read error: %s",
+		zlog_err(
-			 fname, lua_tostring(fs->L, -1));
+			"frrscript: failed loading script '%s.lua': file read error: %s",
+			script_name, lua_tostring(L, -1));
 		break;
 	default:
-		zlog_err("Failed loading script '%s': unknown error: %s", fname,
+		zlog_err(
-			 lua_tostring(fs->L, -1));
+			"frrscript: failed loading script '%s.lua': unknown error: %s",
+			script_name, lua_tostring(L, -1));
 		break;
 	}
 
 	if (ret != LUA_OK)
 		goto fail;
 
-	if (load_cb && (*load_cb)(fs) != 0)
+	/* Push the Lua function we want */
+	lua_getglobal(L, function_name);
+	if (lua_isfunction(L, lua_gettop(L)) == 0) {
+		zlog_err("frrscript: loaded script '%s.lua' but %s not found",
+			 script_name, function_name);
 		goto fail;
+	}
 
-	return fs;
+	if (load_cb && (*load_cb)(fs) != 0) {
+		zlog_err(
+			"frrscript: '%s.lua': %s: loaded but callback returned non-zero exit code",
+			script_name, function_name);
+		goto fail;
+	}
+
+	/* Add the Lua function state to frrscript */
+	struct lua_function_state key = {.name = function_name, .L = L};
+
+	hash_get(fs->lua_function_hash, &key, lua_function_alloc);
+
+	return 0;
 fail:
-	frrscript_unload(fs);
+	lua_close(L);
-	return NULL;
+	return 1;
 }
 
-void frrscript_unload(struct frrscript *fs)
+void frrscript_delete(struct frrscript *fs)
 {
-	lua_close(fs->L);
+	hash_iterate(fs->lua_function_hash, lua_function_free, NULL);
 	XFREE(MTYPE_SCRIPT, fs->name);
 	XFREE(MTYPE_SCRIPT, fs);
 }

lib/frrscript.h

@@ -25,7 +25,7 @@
 
 #include <lua.h>
 #include "frrlua.h"
-#include "../bgpd/bgp_script.h"
+#include "bgpd/bgp_script.h" // for peer and attr encoders/decoders
 
 #ifdef __cplusplus
 extern "C" {
@@ -40,14 +40,30 @@ struct frrscript_codec {
 	decoder_func decoder;
 };
 
+struct lua_function_state {
+	const char *name;
+	lua_State *L;
+};
+
 struct frrscript {
 	/* Script name */
 	char *name;
 
-	/* Lua state */
+	/* Hash of Lua function name to Lua function state */
-	struct lua_State *L;
+	struct hash *lua_function_hash;
 };
 
+
+/*
+ * Hash related functions for lua_function_hash
+ */
+
+void *lua_function_alloc(void *arg);
+
+unsigned int lua_function_hash_key(const void *data);
+
+bool lua_function_hash_cmp(const void *d1, const void *d2);
+
 struct frrscript_env {
 	/* Value type */
 	const char *typename;
@@ -60,15 +76,24 @@ struct frrscript_env {
 };
 
 /*
- * Create new FRR script.
+ * Create new struct frrscript for a Lua script.
+ * This will hold the states for the Lua functions in this script.
+ *
+ * scriptname
+ *     Name of the Lua script file, without the .lua
  */
-struct frrscript *frrscript_load(const char *name,
+struct frrscript *frrscript_new(const char *scriptname);
-				 int (*load_cb)(struct frrscript *));
 
 /*
- * Destroy FRR script.
+ * Load a function into frrscript, run callback if any
  */
-void frrscript_unload(struct frrscript *fs);
+int frrscript_load(struct frrscript *fs, const char *function_name,
+		   int (*load_cb)(struct frrscript *));
+
+/*
+ * Delete Lua function states and frrscript
+ */
+void frrscript_delete(struct frrscript *fs);
 
 /*
  * Register a Lua codec for a type.
@@ -97,16 +122,31 @@ void frrscript_register_type_codecs(struct frrscript_codec *codecs);
  */
 void frrscript_init(const char *scriptdir);
 
-#define ENCODE_ARGS(name, value)                                               \
+/*
-	do {                                                                   \
+ * This macro is mapped to every (name, value) in frrscript_call,
-		ENCODE_ARGS_WITH_STATE(L, value);                              \
+ * so this in turn maps them onto their encoders
-		lua_setglobal(L, name);                                        \
+ */
-	} while (0)
+#define ENCODE_ARGS(name, value) ENCODE_ARGS_WITH_STATE(lfs->L, (value))
 
+/*
+ * This macro is also mapped to every (name, value) in frrscript_call, but
+ * not every value can be mapped to its decoder - only those that appear
+ * in the returned table will. To find out if they appear in the returned
+ * table, first pop the value and check if its nil. Only call the decoder
+ * if non-nil.
+ *
+ * At the end, the only thing left on the stack should be the
+ * returned table.
+ */
 #define DECODE_ARGS(name, value)                                               \
 	do {                                                                   \
-		lua_getglobal(L, name);                                        \
+		lua_getfield(lfs->L, 1, (name));                               \
-		DECODE_ARGS_WITH_STATE(L, value);                              \
+		if (lua_isnil(lfs->L, 2)) {                                    \
+			lua_pop(lfs->L, 1);                                    \
+		} else {                                                       \
+			DECODE_ARGS_WITH_STATE(lfs->L, (value));               \
+		}                                                              \
+		assert(lua_gettop(lfs->L) == 1);                               \
 	} while (0)
 
 /*
@@ -120,6 +160,7 @@ void frrscript_init(const char *scriptdir);
  */
 #define ENCODE_ARGS_WITH_STATE(L, value)                                       \
 	_Generic((value), \
+int : lua_pushinteger,                                          \
 long long * : lua_pushintegerp,                                 \
 struct prefix * : lua_pushprefix,                               \
 struct interface * : lua_pushinterface,                         \
@@ -131,10 +172,11 @@ char * : lua_pushstring_wrapper,                                \
 struct attr * : lua_pushattr,                                   \
 struct peer * : lua_pushpeer,                                   \
 const struct prefix * : lua_pushprefix                          \
-)(L, value)
+)((L), (value))
 
 #define DECODE_ARGS_WITH_STATE(L, value)                                       \
 	_Generic((value), \
+int : lua_decode_integer_noop,                                  \
 long long * : lua_decode_integerp,                              \
 struct prefix * : lua_decode_prefix,                            \
 struct interface * : lua_decode_interface,                      \
@@ -146,56 +188,84 @@ char * : lua_decode_stringp,                                    \
 struct attr * : lua_decode_attr,                                \
 struct peer * : lua_decode_noop,                                \
 const struct prefix * : lua_decode_noop                         \
-)(L, -1, value)
+)((L), -1, (value))
 
 /*
- * Call script.
+ * Call Lua function state (abstraction for a single Lua function)
  *
- * fs
+ * lfs
- *    The script to call; this is obtained from frrscript_load().
+ *    The Lua function to call; this should have been loaded in by
+ *    frrscript_load(). nargs Number of arguments the function accepts
  *
  * Returns:
  *    0 if the script ran successfully, nonzero otherwise.
  */
-int _frrscript_call(struct frrscript *fs);
+int _frrscript_call_lua(struct lua_function_state *lfs, int nargs);
 
 /*
- * Wrapper for call script. Maps values passed in to their encoder
+ * Wrapper for calling Lua function state. Maps values passed in to their
- * and decoder types.
+ * encoder and decoder types.
  *
  * fs
- *    The script to call; this is obtained from frrscript_load().
+ *    The struct frrscript in which the Lua fuction was loaded into
+ * f
+ *    Name of the Lua function.
  *
  * Returns:
  *    0 if the script ran successfully, nonzero otherwise.
  */
-#define frrscript_call(fs, ...)                                                \
+#define frrscript_call(fs, f, ...)                                                                                                                                 \
-	({                                                                     \
+	({                                                                                                                                                         \
-		lua_State *L = fs->L;                                          \
+		struct lua_function_state lookup = {.name = (f)};                                                                                                  \
-		MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__);                         \
+		struct lua_function_state *lfs;                                                                                                                    \
-		int ret = _frrscript_call(fs);                                 \
+		lfs = hash_lookup((fs)->lua_function_hash, &lookup);                                                                                               \
-		if (ret == 0) {                                                \
+		lfs == NULL ? ({                                                                                                                                   \
-			MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__);                 \
+			zlog_err(                                                                                                                                  \
-		}                                                              \
+				"frrscript: '%s.lua': '%s': tried to call this function but it was not loaded",                                                    \
-		ret;                                                           \
+				(fs)->name, (f));                                                                                                                  \
+			1;                                                                                                                                         \
+		})                                                                                                                                                 \
+			    : ({                                                                                                                                   \
+				      MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__);                                                                                       \
+				      _frrscript_call_lua(                                                                                                         \
+					      lfs, PP_NARG(__VA_ARGS__));                                                                                          \
+			      }) != 0                                                                                                                              \
+				      ? ({                                                                                                                         \
+						zlog_err(                                                                                                          \
+							"frrscript: '%s.lua': '%s': this function called but returned non-zero exit code. No variables modified.", \
+							(fs)->name, (f));                                                                                          \
+						1;                                                                                                                 \
+					})                                                                                                                         \
+				      : ({                                                                                                                         \
+						MAP_LISTS(DECODE_ARGS,                                                                                             \
+							  ##__VA_ARGS__);                                                                                          \
+						0;                                                                                                                 \
+					});                                                                                                                        \
 	})
 
 /*
- * Get result from finished script.
+ * Get result from finished function
  *
  * fs
  *    The script. This script must have been run already.
- *
+ * function_name
- * result
+ *    Name of the Lua function.
- *    The result to extract from the script.
+ * name
- *    This reuses the frrscript_env type, but only the typename and name fields
+ *    Name of the result.
- *    need to be set. The value is returned directly.
+ *    This will be used as a string key to retrieve from the table that the
+ *    Lua function returns.
+ *    The name here should *not* appear in frrscript_call.
+ * lua_to
+ *    Function pointer to a lua_to decoder function.
+ *    This function should allocate and decode a value from the Lua state.
  *
  * Returns:
- *    The script result of the specified name and type, or NULL.
+ *    A pointer to the decoded value from the Lua state, or NULL if no such
+ *    value.
  */
-void *frrscript_get_result(struct frrscript *fs,
+void *frrscript_get_result(struct frrscript *fs, const char *function_name,
-			   const struct frrscript_env *result);
+			   const char *name,
+			   void *(*lua_to)(lua_State *L, int idx));
 
 #ifdef __cplusplus
 }

tests/lib/script1.lua

@@ -1 +1,54 @@
-a = a + b
+
+-- Positive testing
+
+function foo(a, b)
+  a = a + 1
+  b = b + 1
+  return {
+    a = a,
+    b = b,
+  }
+end
+
+function bar(a, b)
+  a = a + 1
+  b = b + 1
+  c = 303
+  return {
+    b = b,
+    c = c,
+  }
+end
+
+function fact(n)
+  -- outer function must return a table
+  -- inner functions can be used to recurse or as helpers
+  function helper(m)
+    if m == 0 then
+      return 1
+    else
+      return m * helper(m - 1)
+    end
+  end
+  return {
+    ans = helper(n)
+  }
+end
+
+-- Negative testing
+
+function bad_return1()
+end
+
+function bad_return2()
+  return 123
+end
+
+function bad_return3()
+  return {}
+end
+
+function bad_return4()
+  error("Something bad!")
+end
+

tests/lib/test_frrscript.c

@@ -20,18 +20,85 @@
 #include <zebra.h>
 
 #include "lib/frrscript.h"
+#include "lib/frrlua.h"
 
 int main(int argc, char **argv)
 {
 	frrscript_init("./lib");
+	struct frrscript *fs = frrscript_new("script1");
+	int result;
+
+	/* Positive testing */
 
-	struct frrscript *fs = frrscript_load("script1", NULL);
 	long long a = 100, b = 200;
-	int result = frrscript_call(fs, ("a", &a), ("b", &b));
 
+	result = frrscript_load(fs, "foo", NULL);
 	assert(result == 0);
-	assert(a == 300);
+	result = frrscript_call(fs, "foo", ("a", &a), ("b", &b));
-	assert(b == 200);
+	assert(result == 0);
+	assert(a == 101);
+	assert(b == 201);
+
+	a = 100, b = 200;
+
+	result = frrscript_load(fs, "bar", NULL);
+	assert(result == 0);
+	result = frrscript_call(fs, "bar", ("a", &a), ("b", &b));
+	assert(result == 0);
+	long long *cptr = frrscript_get_result(fs, "bar", "c", lua_tointegerp);
+
+	/* a should not occur in the returned table in script */
+	assert(a == 100);
+	assert(b == 201);
+	assert(*cptr == 303);
+	XFREE(MTYPE_SCRIPT_RES, cptr);
+
+	long long n = 5;
+
+	result = frrscript_load(fs, "fact", NULL);
+	assert(result == 0);
+	result = frrscript_call(fs, "fact", ("n", &n));
+	assert(result == 0);
+	long long *ansptr =
+		frrscript_get_result(fs, "fact", "ans", lua_tointegerp);
+	assert(*ansptr == 120);
+	XFREE(MTYPE_SCRIPT_RES, ansptr);
+
+	/* Negative testing */
+
+	/* Function does not exist in script file*/
+	result = frrscript_load(fs, "does_not_exist", NULL);
+	assert(result == 1);
+
+	/* Function was not (successfully) loaded */
+	result = frrscript_call(fs, "does_not_exist", ("a", &a), ("b", &b));
+	assert(result == 1);
+
+	/* Get result from a function that was not loaded */
+	long long *llptr =
+		frrscript_get_result(fs, "does_not_exist", "c", lua_tointegerp);
+	assert(llptr == NULL);
+
+	/* Function returns void */
+	result = frrscript_call(fs, "bad_return1");
+	assert(result == 1);
+
+	/* Function returns number */
+	result = frrscript_call(fs, "bad_return2");
+	assert(result == 1);
+
+	/* Get non-existent result from a function */
+	result = frrscript_call(fs, "bad_return3");
+	assert(result == 1);
+	long long *cllptr =
+		frrscript_get_result(fs, "bad_return3", "c", lua_tointegerp);
+	assert(cllptr == NULL);
+
+	/* Function throws exception */
+	result = frrscript_call(fs, "bad_return4");
+	assert(result == 1);
+
+	frrscript_delete(fs);
 
 	return 0;
 }