From 105ba9af8f0bd9a116f2571625b2ea39c49c20c1 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 4 Jul 2021 22:58:21 +0800 Subject: [PATCH 01/45] lib: Change frrscript to hold many Lua states Instead of 1 frrscript : 1 lua state, it is changed to 1 : N. The states are hashed with their function names. Signed-off-by: Donald Lee --- lib/frrscript.c | 34 ++++++++++++++++++++++++++++++++++ lib/frrscript.h | 21 +++++++++++++++++++-- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index 1a9f3639dd..d86e6acb12 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -102,6 +102,40 @@ static void codec_free(struct codec *c) } #endif + +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 lua_function_state *lfs) +{ + XFREE(MTYPE_TMP, lfs->name); + lua_close(lfs->L); + XFREE(MTYPE_TMP, lfs); +} + /* Generic script APIs */ int _frrscript_call(struct frrscript *fs) diff --git a/lib/frrscript.h b/lib/frrscript.h index 8612c602f3..97e543eb00 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -40,14 +40,31 @@ struct frrscript_codec { decoder_func decoder; }; +struct lua_function_state { + const char *name; + lua_State *L; +}; + + struct frrscript { /* Script name */ char *name; - /* Lua state */ - struct lua_State *L; + /* Hash of Lua function name to Lua function state */ + 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; From f0cddf950f928b6f9a8d56c688235cea728fda41 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 4 Jul 2021 23:05:37 +0800 Subject: [PATCH 02/45] lib: create new frrscript_new frrscript_new now creates a new frrscript frrscript_load now loads a function (by allocating a new lua stack) Signed-off-by: Donald Lee --- lib/frrscript.c | 48 ++++++++++++++++++++++++++++++++++-------------- lib/frrscript.h | 9 +++++++-- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index d86e6acb12..ed9043d8d2 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -217,56 +217,76 @@ void frrscript_register_type_codecs(struct frrscript_codec *codecs) frrscript_register_type_codec(&codecs[i]); } -struct frrscript *frrscript_load(const char *name, - int (*load_cb)(struct frrscript *)) +struct frrscript *frrscript_new(const char *name) { struct frrscript *fs = XCALLOC(MTYPE_SCRIPT, sizeof(struct frrscript)); fs->name = XSTRDUP(MTYPE_SCRIPT, name); - fs->L = luaL_newstate(); - frrlua_export_logging(fs->L); + fs->lua_function_hash = + hash_create(lua_function_hash_key, lua_function_hash_cmp, + "Lua function state hash"); + return fs; +} + +int frrscript_load(struct frrscript *fs, const char *function_name, + int (*load_cb)(struct frrscript *)) +{ + + /* Set up the Lua script */ + lua_State *L = luaL_newstate(); + frrlua_export_logging(L); char fname[MAXPATHLEN * 2]; - snprintf(fname, sizeof(fname), "%s/%s.lua", scriptdir, fs->name); - int ret = luaL_loadfile(fs->L, fname); + snprintf(fname, sizeof(fname), "%s/%s.lua", scriptdir, fs->name); + int ret = luaL_dofile(L, fname); switch (ret) { case LUA_OK: break; case LUA_ERRSYNTAX: zlog_err("Failed loading script '%s': syntax error: %s", fname, - lua_tostring(fs->L, -1)); + lua_tostring(L, -1)); break; case LUA_ERRMEM: zlog_err("Failed loading script '%s': out-of-memory error: %s", - fname, lua_tostring(fs->L, -1)); + fname, lua_tostring(L, -1)); break; case LUA_ERRGCMM: zlog_err( "Failed loading script '%s': garbage collector error: %s", - fname, lua_tostring(fs->L, -1)); + fname, lua_tostring(L, -1)); break; case LUA_ERRFILE: zlog_err("Failed loading script '%s': file read error: %s", - fname, lua_tostring(fs->L, -1)); + fname, lua_tostring(L, -1)); break; default: zlog_err("Failed loading script '%s': unknown error: %s", fname, - lua_tostring(fs->L, -1)); + lua_tostring(L, -1)); break; } if (ret != LUA_OK) goto fail; + /* Push the Lua function we want */ + lua_getglobal(L, function_name); + if (lua_isfunction(L, lua_gettop(L)) == 0) + goto fail; + if (load_cb && (*load_cb)(fs) != 0) goto fail; - return fs; + /* 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); - return NULL; + lua_close(L); + return 1; } void frrscript_unload(struct frrscript *fs) diff --git a/lib/frrscript.h b/lib/frrscript.h index 97e543eb00..df72bba4cd 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -79,8 +79,13 @@ struct frrscript_env { /* * Create new FRR script. */ -struct frrscript *frrscript_load(const char *name, - int (*load_cb)(struct frrscript *)); +struct frrscript *frrscript_new(const char *name); + +/* + * Load a function into frrscript, run callback if any + */ +int frrscript_load(struct frrscript *fs, const char *function_name, + int (*load_cb)(struct frrscript *)); /* * Destroy FRR script. From 40d038d2a1c8329c408c4fffd88e674413dfd427 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 4 Jul 2021 23:08:18 +0800 Subject: [PATCH 03/45] lib: Change frrscript_call to call function instead There is some rather heavy error checking logic in frrscript_call. Normally I'd put this in the _frrscript_call function, but the error checking needs to happen before the encoders/decoders in the macro are called. The error checking looks messy but its really just nested ternary expressions insite a larger statement expression. Signed-off-by: Donald Lee --- lib/frrscript.c | 42 +++++++++++++++++++++++++++++------------- lib/frrscript.h | 47 ++++++++++++++++++++++++++++++----------------- 2 files changed, 59 insertions(+), 30 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index ed9043d8d2..8a93d36da1 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -138,41 +138,57 @@ static void lua_function_free(struct lua_function_state *lfs) /* Generic script APIs */ -int _frrscript_call(struct frrscript *fs) +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, - lua_tostring(fs->L, -1)); + zlog_err("Lua hook call '%s' : runtime error: %s", lfs->name, + lua_tostring(lfs->L, -1)); break; case LUA_ERRMEM: - zlog_err("Script '%s' memory error: %s", fs->name, - lua_tostring(fs->L, -1)); + zlog_err("Lua hook call '%s' : memory error: %s", lfs->name, + lua_tostring(lfs->L, -1)); break; case LUA_ERRERR: - zlog_err("Script '%s' error handler error: %s", fs->name, - lua_tostring(fs->L, -1)); + zlog_err("Lua hook call '%s' : error handler error: %s", + lfs->name, lua_tostring(lfs->L, -1)); break; case LUA_ERRGCMM: - zlog_err("Script '%s' garbage collector error: %s", fs->name, - lua_tostring(fs->L, -1)); + zlog_err("Lua hook call '%s' : garbage collector error: %s", + lfs->name, lua_tostring(lfs->L, -1)); break; default: - zlog_err("Script '%s' unknown error: %s", fs->name, - lua_tostring(fs->L, -1)); + zlog_err("Lua hook call '%s' : unknown error: %s", lfs->name, + 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; diff --git a/lib/frrscript.h b/lib/frrscript.h index df72bba4cd..a0e1176ea7 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -119,16 +119,12 @@ void frrscript_register_type_codecs(struct frrscript_codec *codecs); */ void frrscript_init(const char *scriptdir); -#define ENCODE_ARGS(name, value) \ - do { \ - ENCODE_ARGS_WITH_STATE(L, value); \ - lua_setglobal(L, name); \ - } while (0) +#define ENCODE_ARGS(name, value) ENCODE_ARGS_WITH_STATE(lfs->L, value) #define DECODE_ARGS(name, value) \ do { \ - lua_getglobal(L, name); \ - DECODE_ARGS_WITH_STATE(L, value); \ + lua_getfield(lfs->L, 1, name); \ + DECODE_ARGS_WITH_STATE(lfs->L, value); \ } while (0) /* @@ -179,7 +175,7 @@ const struct prefix * : lua_decode_noop \ * 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 @@ -191,15 +187,32 @@ int _frrscript_call(struct frrscript *fs); * Returns: * 0 if the script ran successfully, nonzero otherwise. */ -#define frrscript_call(fs, ...) \ - ({ \ - lua_State *L = fs->L; \ - MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \ - int ret = _frrscript_call(fs); \ - if (ret == 0) { \ - MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \ - } \ - ret; \ + +#define frrscript_call(fs, f, ...) \ + ({ \ + struct lua_function_state lookup = {.name = f}; \ + struct lua_function_state *lfs; \ + lfs = hash_lookup(fs->lua_function_hash, &lookup); \ + lfs == NULL ? ({ \ + zlog_err( \ + "Lua script call: tried to call '%s' in '%s' which was not loaded", \ + f, fs->name); \ + 1; \ + }) \ + : ({ \ + MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \ + _frrscript_call_lua(lfs, PP_NARG(__VA_ARGS__)); \ + }) != 0 \ + ? ({ \ + zlog_err( \ + "Lua script call: '%s' in '%s' returned non-zero exit code", \ + f, fs->name); \ + 1; \ + }) \ + : ({ \ + MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \ + 0; \ + }); \ }) /* From a71096fb0a327baf9400ec6df964bfb696b18cfb Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 4 Jul 2021 23:12:07 +0800 Subject: [PATCH 04/45] lib: Update Script command example to call function Signed-off-by: Donald Lee --- lib/command.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/command.c b/lib/command.c index 9dac60599c..b2b80daddd 100644 --- a/lib/command.c +++ b/lib/command.c @@ -2428,18 +2428,20 @@ DEFUN(script, 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) { vty_out(vty, "Script '/etc/frr/scripts/%s.lua' not found\n", argv[1]->arg); - } else { - 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); } + if (frrscript_load(fs, argv[2]->arg, NULL)) + vty_out(vty, "Script function '/%s' not found\n", argv[2]->arg); + + 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); return CMD_SUCCESS; } From bf938fdb9dafa3c330026394d9b6d291559ec993 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 4 Jul 2021 23:12:52 +0800 Subject: [PATCH 05/45] bgpd: Update bgpd example to call function Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 529abcbea0..0b22caaee7 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -367,10 +367,17 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) const char *scriptname = rule; 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) { - zlog_err("Issue loading script rule; defaulting to no match"); + zlog_err("Issue loading script file; defaulting to no match"); + return RMAP_NOMATCH; + } + + if (frrscript_load(fs, "route_match", NULL)) { + zlog_err( + "Issue loading script function; defaulting to no match"); return RMAP_NOMATCH; } @@ -382,7 +389,7 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) struct attr newattr = *path->attr; int result = frrscript_call( - fs, ("RM_FAILURE", (long long *)&lrm_status), + fs, "route_match", ("RM_FAILURE", (long long *)&lrm_status), ("RM_NOMATCH", (long long *)&status_nomatch), ("RM_MATCH", (long long *)&status_match), ("RM_MATCH_AND_CHANGE", (long long *)&status_match_and_change), From 4535b6113cf105287e060042b956251d4cbf4198 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 4 Jul 2021 23:13:32 +0800 Subject: [PATCH 06/45] tests: Add test for calling Lua function Signed-off-by: Donald Lee --- tests/lib/script1.lua | 21 ++++++++++++++++++++- tests/lib/test_frrscript.c | 20 ++++++++++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/tests/lib/script1.lua b/tests/lib/script1.lua index e9ebc29bd9..f7cd8a90b9 100644 --- a/tests/lib/script1.lua +++ b/tests/lib/script1.lua @@ -1 +1,20 @@ -a = a + b +function foo(a, b) + a = a + b + return { + a = a, + b = b, + } +end + +function fact(n) + function helper(m) + if m == 0 then + return 1 + else + return m * helper(m - 1) + end + end + return { + n = helper(n) + } +end diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c index bd75cc5552..4d34b58832 100644 --- a/tests/lib/test_frrscript.c +++ b/tests/lib/test_frrscript.c @@ -24,14 +24,30 @@ int main(int argc, char **argv) { frrscript_init("./lib"); + struct frrscript *fs = frrscript_new("script1"); + int result; - 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); + result = frrscript_call(fs, "foo", ("a", &a), ("b", &b)); assert(result == 0); assert(a == 300); assert(b == 200); + result = frrscript_load(fs, "does_not_exist", NULL); + assert(result == 1); + + result = frrscript_call(fs, "does_not_exist", ("a", &a), ("b", &b)); + assert(result == 1); + + frrscript_load(fs, "fact", NULL); + long long n = 5; + + result = frrscript_call(fs, "fact", ("n", &n)); + assert(result == 0); + assert(n == 120); + return 0; } From cb5de2314056e81854715c1835612445736e0a03 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Mon, 5 Jul 2021 05:11:24 +0800 Subject: [PATCH 07/45] lib: update frrscript unload Signed-off-by: Donald Lee --- lib/frrscript.c | 4 ---- lib/frrscript.h | 1 - 2 files changed, 5 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index 8a93d36da1..1d4fe58dd1 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -208,9 +208,6 @@ void *frrscript_get_result(struct frrscript *fs, return NULL; } - lua_getglobal(fs->L, result->name); - r = codec->decoder(fs->L, -1); - lua_pop(fs->L, 1); return r; } @@ -307,7 +304,6 @@ fail: void frrscript_unload(struct frrscript *fs) { - lua_close(fs->L); XFREE(MTYPE_SCRIPT, fs->name); XFREE(MTYPE_SCRIPT, fs); } diff --git a/lib/frrscript.h b/lib/frrscript.h index a0e1176ea7..0d5568d342 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -45,7 +45,6 @@ struct lua_function_state { lua_State *L; }; - struct frrscript { /* Script name */ char *name; From 7948c5d27ac5313841a0f8e0b88ecc090932dffe Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Mon, 5 Jul 2021 05:25:49 +0800 Subject: [PATCH 08/45] tests: Add errorneous test cases Signed-off-by: Donald Lee --- tests/lib/script1.lua | 12 ++++++++++++ tests/lib/test_frrscript.c | 30 ++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/tests/lib/script1.lua b/tests/lib/script1.lua index f7cd8a90b9..e911a1c941 100644 --- a/tests/lib/script1.lua +++ b/tests/lib/script1.lua @@ -18,3 +18,15 @@ function fact(n) n = helper(n) } end + +function bad_return1() +end + +function bad_return2() + return 1 +end + +function bad_return3() + error("Something bad!") +end + diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c index 4d34b58832..3bb45a2651 100644 --- a/tests/lib/test_frrscript.c +++ b/tests/lib/test_frrscript.c @@ -36,12 +36,6 @@ int main(int argc, char **argv) assert(a == 300); assert(b == 200); - result = frrscript_load(fs, "does_not_exist", NULL); - assert(result == 1); - - result = frrscript_call(fs, "does_not_exist", ("a", &a), ("b", &b)); - assert(result == 1); - frrscript_load(fs, "fact", NULL); long long n = 5; @@ -49,5 +43,29 @@ int main(int argc, char **argv) assert(result == 0); assert(n == 120); + /* Function does not exist in script file*/ + result = frrscript_load(fs, "does_not_exist", NULL); + assert(result == 1); + + /* 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); + + /* Function returns void */ + result = frrscript_call(fs, "bad_return1"); + assert(result == 1); + + /* Function returns number */ + result = frrscript_call(fs, "bad_return2"); + assert(result == 1); + + /* Function throws exception */ + result = frrscript_call(fs, "bad_return3"); + assert(result == 1); + return 0; } From 24ff8520af692c538f3ff8b3339a307c21a7fa38 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Wed, 7 Jul 2021 21:53:10 +0800 Subject: [PATCH 09/45] lib: frrscript_call check name before decode Signed-off-by: Donald Lee --- lib/frrscript.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/frrscript.h b/lib/frrscript.h index 0d5568d342..1e35e2ee41 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -123,7 +123,12 @@ void frrscript_init(const char *scriptdir); #define DECODE_ARGS(name, value) \ do { \ lua_getfield(lfs->L, 1, name); \ - DECODE_ARGS_WITH_STATE(lfs->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) /* From 06947ddeac654c76940c09d38cb0e5f435e28495 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Wed, 7 Jul 2021 21:53:38 +0800 Subject: [PATCH 10/45] lib: Add frrscript_get_result Signed-off-by: Donald Lee --- lib/frrscript.c | 27 ++++++++++++++++++--------- lib/frrscript.h | 5 +++-- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index 1d4fe58dd1..5352c6470d 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -194,22 +194,31 @@ done: return ret; } -void *frrscript_get_result(struct frrscript *fs, - const struct frrscript_env *result) +void *frrscript_get_result(struct frrscript *fs, const char *function_name, + const char *name, + void *(*lua_to)(lua_State *L, int idx)) { - void *r; - struct frrscript_codec c = {.typename = result->typename}; + void *p; + struct lua_function_state *lfs; + struct lua_function_state lookup = {.name = function_name}; - struct frrscript_codec *codec = hash_lookup(codec_hash, &c); - assert(codec && "No encoder for type"); + lfs = hash_lookup(fs->lua_function_hash, &lookup); - if (!codec->decoder) { - zlog_err("No script decoder for type '%s'", result->typename); + if (lfs == NULL) { return NULL; } + /* results table is idx 1 on the stack, getfield pushes our item to idx + * 2*/ + lua_getfield(lfs->L, 1, name); + if (lua_isnil(lfs->L, -1)) { + lua_pop(lfs->L, 1); + zlog_err("No result in results table with that name %s", name); + return NULL; + } + p = lua_to(lfs->L, 2); - return r; + return p; } void frrscript_register_type_codec(struct frrscript_codec *codec) diff --git a/lib/frrscript.h b/lib/frrscript.h index 1e35e2ee41..d2791a5a8b 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -233,8 +233,9 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs); * Returns: * The script result of the specified name and type, or NULL. */ -void *frrscript_get_result(struct frrscript *fs, - const struct frrscript_env *result); +void *frrscript_get_result(struct frrscript *fs, const char *function_name, + const char *name, + void *(*lua_to)(lua_State *L, int idx)); #ifdef __cplusplus } From 5090d7249fd4b4fd65945a6f3282855e8b21bcfb Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Wed, 7 Jul 2021 21:54:00 +0800 Subject: [PATCH 11/45] tests: Add test for frrscript_get_result Signed-off-by: Donald Lee --- tests/lib/script1.lua | 2 +- tests/lib/test_frrscript.c | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/lib/script1.lua b/tests/lib/script1.lua index e911a1c941..5029a921bc 100644 --- a/tests/lib/script1.lua +++ b/tests/lib/script1.lua @@ -15,7 +15,7 @@ function fact(n) end end return { - n = helper(n) + ans = helper(n) } end diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c index 3bb45a2651..e8b7927e15 100644 --- a/tests/lib/test_frrscript.c +++ b/tests/lib/test_frrscript.c @@ -20,6 +20,7 @@ #include #include "lib/frrscript.h" +#include "lib/frrlua.h" int main(int argc, char **argv) { @@ -36,12 +37,16 @@ int main(int argc, char **argv) assert(a == 300); assert(b == 200); - frrscript_load(fs, "fact", NULL); long long n = 5; + result = frrscript_load(fs, "fact", NULL); + assert(result == 0); result = frrscript_call(fs, "fact", ("n", &n)); assert(result == 0); - assert(n == 120); + long long *ansptr = + frrscript_get_result(fs, "fact", "ans", lua_tointegerp); + assert(*ansptr == 120); + XFREE(MTYPE_TMP, ansptr); /* Function does not exist in script file*/ result = frrscript_load(fs, "does_not_exist", NULL); From 8a79921ca350faea6576c49ed1e41f8a3cf4ee0f Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 8 Jul 2021 00:28:19 +0800 Subject: [PATCH 12/45] lib: format macro slashes Signed-off-by: Donald Lee --- lib/frrscript.h | 52 +++++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/lib/frrscript.h b/lib/frrscript.h index d2791a5a8b..85e7b15cd3 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -192,31 +192,33 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs); * 0 if the script ran successfully, nonzero otherwise. */ -#define frrscript_call(fs, f, ...) \ - ({ \ - struct lua_function_state lookup = {.name = f}; \ - struct lua_function_state *lfs; \ - lfs = hash_lookup(fs->lua_function_hash, &lookup); \ - lfs == NULL ? ({ \ - zlog_err( \ - "Lua script call: tried to call '%s' in '%s' which was not loaded", \ - f, fs->name); \ - 1; \ - }) \ - : ({ \ - MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \ - _frrscript_call_lua(lfs, PP_NARG(__VA_ARGS__)); \ - }) != 0 \ - ? ({ \ - zlog_err( \ - "Lua script call: '%s' in '%s' returned non-zero exit code", \ - f, fs->name); \ - 1; \ - }) \ - : ({ \ - MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \ - 0; \ - }); \ +#define frrscript_call(fs, f, ...) \ + ({ \ + struct lua_function_state lookup = {.name = f}; \ + struct lua_function_state *lfs; \ + lfs = hash_lookup(fs->lua_function_hash, &lookup); \ + lfs == NULL ? ({ \ + zlog_err( \ + "Lua script call: tried to call '%s' in '%s' which was not loaded", \ + f, fs->name); \ + 1; \ + }) \ + : ({ \ + MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \ + _frrscript_call_lua( \ + lfs, PP_NARG(__VA_ARGS__)); \ + }) != 0 \ + ? ({ \ + zlog_err( \ + "Lua script call: '%s' in '%s' returned non-zero exit code", \ + f, fs->name); \ + 1; \ + }) \ + : ({ \ + MAP_LISTS(DECODE_ARGS, \ + ##__VA_ARGS__); \ + 0; \ + }); \ }) /* From fae19fa56d110b27bc4f507221573b432cdfc9a8 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 8 Jul 2021 01:31:43 +0800 Subject: [PATCH 13/45] lib: frrscript unload deallocates Lua function Signed-off-by: Donald Lee --- lib/frrscript.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index 5352c6470d..9b497a431d 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -102,6 +102,7 @@ static void codec_free(struct codec *c) } #endif +/* Lua function hash utils */ unsigned int lua_function_hash_key(const void *data) { @@ -129,11 +130,11 @@ void *lua_function_alloc(void *arg) return lfs; } -static void lua_function_free(struct lua_function_state *lfs) +static void lua_function_free(struct hash_bucket *b, void *data) { - XFREE(MTYPE_TMP, lfs->name); + struct lua_function_state *lfs = (struct lua_function_state *)b->data; lua_close(lfs->L); - XFREE(MTYPE_TMP, lfs); + XFREE(MTYPE_SCRIPT, lfs); } /* Generic script APIs */ @@ -313,6 +314,7 @@ fail: void frrscript_unload(struct frrscript *fs) { + hash_iterate(fs->lua_function_hash, lua_function_free, NULL); XFREE(MTYPE_SCRIPT, fs->name); XFREE(MTYPE_SCRIPT, fs); } From ad6e9b854d64aa91eef2d965b7778188d2d7811a Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 8 Jul 2021 01:32:30 +0800 Subject: [PATCH 14/45] test: Use frrscript_unload Signed-off-by: Donald Lee --- lib/command.c | 2 ++ tests/lib/test_frrscript.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/command.c b/lib/command.c index b2b80daddd..7808f57594 100644 --- a/lib/command.c +++ b/lib/command.c @@ -2443,6 +2443,8 @@ DEFUN(script, vty_out(vty, "p: %s\n", buf); vty_out(vty, "Script result: %d\n", ret); + frrscript_unload(fs); + return CMD_SUCCESS; } #endif diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c index e8b7927e15..399b950799 100644 --- a/tests/lib/test_frrscript.c +++ b/tests/lib/test_frrscript.c @@ -72,5 +72,7 @@ int main(int argc, char **argv) result = frrscript_call(fs, "bad_return3"); assert(result == 1); + frrscript_unload(fs); + return 0; } From b664092990bddc95562626847396e067d50fae85 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 8 Jul 2021 02:40:18 +0800 Subject: [PATCH 15/45] lib: standardize zlog error messages Signed-off-by: Donald Lee --- lib/frrscript.c | 37 +++++++++++++++++++++-------------- lib/frrscript.h | 52 ++++++++++++++++++++++++------------------------- 2 files changed, 47 insertions(+), 42 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index 9b497a431d..8252455fd9 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -137,7 +137,7 @@ static void lua_function_free(struct hash_bucket *b, void *data) XFREE(MTYPE_SCRIPT, lfs); } -/* Generic script APIs */ +/* internal frrscript APIs */ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs) { @@ -214,7 +214,9 @@ void *frrscript_get_result(struct frrscript *fs, const char *function_name, lua_getfield(lfs->L, 1, name); if (lua_isnil(lfs->L, -1)) { lua_pop(lfs->L, 1); - zlog_err("No result in results table with that name %s", name); + 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); @@ -259,34 +261,39 @@ int frrscript_load(struct frrscript *fs, const char *function_name, lua_State *L = luaL_newstate(); frrlua_export_logging(L); - char fname[MAXPATHLEN * 2]; + char script_name[MAXPATHLEN * 2]; - snprintf(fname, sizeof(fname), "%s/%s.lua", scriptdir, fs->name); - int ret = luaL_dofile(L, fname); + snprintf(script_name, sizeof(script_name), "%s/%s.lua", scriptdir, + fs->name); + 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, - lua_tostring(L, -1)); + zlog_err( + "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", - fname, lua_tostring(L, -1)); + zlog_err( + "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", - fname, lua_tostring(L, -1)); + "frrscript: failed loading script '%s.lua': garbage collector error: %s", + script_name, lua_tostring(L, -1)); break; case LUA_ERRFILE: - zlog_err("Failed loading script '%s': file read error: %s", - fname, lua_tostring(L, -1)); + zlog_err( + "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, - lua_tostring(L, -1)); + zlog_err( + "frrscript: failed loading script '%s.lua': unknown error: %s", + script_name, lua_tostring(L, -1)); break; } diff --git a/lib/frrscript.h b/lib/frrscript.h index 85e7b15cd3..1f234a38a9 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -192,33 +192,31 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs); * 0 if the script ran successfully, nonzero otherwise. */ -#define frrscript_call(fs, f, ...) \ - ({ \ - struct lua_function_state lookup = {.name = f}; \ - struct lua_function_state *lfs; \ - lfs = hash_lookup(fs->lua_function_hash, &lookup); \ - lfs == NULL ? ({ \ - zlog_err( \ - "Lua script call: tried to call '%s' in '%s' which was not loaded", \ - f, fs->name); \ - 1; \ - }) \ - : ({ \ - MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \ - _frrscript_call_lua( \ - lfs, PP_NARG(__VA_ARGS__)); \ - }) != 0 \ - ? ({ \ - zlog_err( \ - "Lua script call: '%s' in '%s' returned non-zero exit code", \ - f, fs->name); \ - 1; \ - }) \ - : ({ \ - MAP_LISTS(DECODE_ARGS, \ - ##__VA_ARGS__); \ - 0; \ - }); \ +#define frrscript_call(fs, f, ...) \ + ({ \ + struct lua_function_state lookup = {.name = f}; \ + struct lua_function_state *lfs; \ + lfs = hash_lookup(fs->lua_function_hash, &lookup); \ + lfs == NULL ? ({ \ + zlog_err( \ + "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \ + 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; \ + }); \ }) /* From 8a04c1e74e0a21d34a626c378a80c3d5b6302971 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 8 Jul 2021 03:07:06 +0800 Subject: [PATCH 16/45] tests: Add more examples to get_result Signed-off-by: Donald Lee --- tests/lib/script1.lua | 26 ++++++++++++++++++++++++-- tests/lib/test_frrscript.c | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/tests/lib/script1.lua b/tests/lib/script1.lua index 5029a921bc..6361c960a7 100644 --- a/tests/lib/script1.lua +++ b/tests/lib/script1.lua @@ -1,12 +1,28 @@ + +-- Positive testing + function foo(a, b) - a = 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 @@ -19,14 +35,20 @@ function fact(n) } end +-- Negative testing + function bad_return1() end function bad_return2() - return 1 + return 123 end function bad_return3() + return {} +end + +function bad_return4() error("Something bad!") end diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c index 399b950799..e01807f193 100644 --- a/tests/lib/test_frrscript.c +++ b/tests/lib/test_frrscript.c @@ -28,14 +28,30 @@ int main(int argc, char **argv) struct frrscript *fs = frrscript_new("script1"); int result; + /* Positive testing */ + long long a = 100, b = 200; result = frrscript_load(fs, "foo", NULL); assert(result == 0); result = frrscript_call(fs, "foo", ("a", &a), ("b", &b)); assert(result == 0); - assert(a == 300); - assert(b == 200); + assert(a == 101); + assert(b == 202); + + 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 == 202); + assert(*cptr == 303); + XFREE(MTYPE_TMP, cptr); long long n = 5; @@ -48,9 +64,7 @@ int main(int argc, char **argv) assert(*ansptr == 120); XFREE(MTYPE_TMP, ansptr); - /* Function does not exist in script file*/ - result = frrscript_load(fs, "does_not_exist", NULL); - assert(result == 1); + /* Negative testing */ /* Function does not exist in script file*/ result = frrscript_load(fs, "does_not_exist", NULL); @@ -60,6 +74,11 @@ int main(int argc, char **argv) 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); @@ -68,9 +87,16 @@ int main(int argc, char **argv) result = frrscript_call(fs, "bad_return2"); assert(result == 1); - /* Function throws exception */ + /* 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_unload(fs); From 64d457d7ac22d643afb5638bacb0b9713274689e Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 8 Jul 2021 17:51:14 +0800 Subject: [PATCH 17/45] lib: Rename frrscript_unload to delete frrscript_load now loads a function instead of a file, so frrscript_unload should be renamed since it does not unload a function. Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 2 +- lib/command.c | 2 +- lib/frrscript.c | 2 +- lib/frrscript.h | 2 +- tests/lib/test_frrscript.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 0b22caaee7..c2062a5f60 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -434,7 +434,7 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) break; } - frrscript_unload(fs); + frrscript_delete(fs); return status; } diff --git a/lib/command.c b/lib/command.c index 7808f57594..ceea186a9d 100644 --- a/lib/command.c +++ b/lib/command.c @@ -2443,7 +2443,7 @@ DEFUN(script, vty_out(vty, "p: %s\n", buf); vty_out(vty, "Script result: %d\n", ret); - frrscript_unload(fs); + frrscript_delete(fs); return CMD_SUCCESS; } diff --git a/lib/frrscript.c b/lib/frrscript.c index 8252455fd9..b3f8a9f6f5 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -319,7 +319,7 @@ fail: return 1; } -void frrscript_unload(struct frrscript *fs) +void frrscript_delete(struct frrscript *fs) { hash_iterate(fs->lua_function_hash, lua_function_free, NULL); XFREE(MTYPE_SCRIPT, fs->name); diff --git a/lib/frrscript.h b/lib/frrscript.h index 1f234a38a9..14a922a741 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -89,7 +89,7 @@ int frrscript_load(struct frrscript *fs, const char *function_name, /* * Destroy FRR script. */ -void frrscript_unload(struct frrscript *fs); +void frrscript_delete(struct frrscript *fs); /* * Register a Lua codec for a type. diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c index e01807f193..2d5746b587 100644 --- a/tests/lib/test_frrscript.c +++ b/tests/lib/test_frrscript.c @@ -98,7 +98,7 @@ int main(int argc, char **argv) result = frrscript_call(fs, "bad_return4"); assert(result == 1); - frrscript_unload(fs); + frrscript_delete(fs); return 0; } From aed6f883a0da2eb2d466ad98f7fcdb4f2c6d17f7 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Fri, 9 Jul 2021 01:42:23 +0800 Subject: [PATCH 18/45] doc: Update documentation Signed-off-by: Donald Lee --- doc/developer/scripting.rst | 578 +++++++++++++++++++++++------------- 1 file changed, 376 insertions(+), 202 deletions(-) diff --git a/doc/developer/scripting.rst b/doc/developer/scripting.rst index 1757d41feb..232c20f61c 100644 --- a/doc/developer/scripting.rst +++ b/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 -scripts. It is possible to pass C functions as well. +a encoding/decoding system. In this way, arbitrary data from FRR may be passed to +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 -Lua underneath. A script in has two things: - -- name -- state - -In code: +FRR's scripting functionality is provided in the form of Lua functions in Lua +scripts (``.lua`` files). One Lua script may contain many Lua functions. These +are respectively encapsulated in the following structures: .. code-block:: c struct frrscript { - /* Script name */ - char *name; + /* Lua file name */ + char *name; - /* Lua state */ - struct lua_State *L; + /* hash of lua_function_states */ + 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 -Lua library object (``lua_State``). This is an opaque struct that is -manipulated using ``lua_*`` functions. The basic ones are imported from -``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. +`struct frrscript`: Since all Lua functions are contained within scripts, the +following APIs manipulates this structure. ``name`` contains the +Lua script name and a hash of Lua functions to their function names. -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 -- execute -- query state -- unload +In general, to run a Lua function, these steps must take place: -They are typically done in this order. +- Initialization +- Load +- Call +- Delete - -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 +Initialization ^^^^^^^^^^^^^^ -.. 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 - updated to work with the new ``frrscript_call`` and the rest of the new API. - - -Unloading -^^^^^^^^^ - -To destroy a script and its associated state: +For example, to create ``frrscript`` for ``/etc/frr/scripts/bingus.lua``: .. 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 exection 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_TMP, 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 -typed, garbage collected language, cannot directly use C values without some -kind of marshalling / unmarshalling system to translate types between the two -runtimes. +``frrscript_call()`` and how data is passed between C and Lua. Lua, as a +dynamically typed, garbage collected language, cannot directly use C values +without some kind of encoding / decoding system to +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 -value from the stack and converts it to the corresponding C type. These -functions are known as encoders and decoders in FRR. +Lua must provide a corresponding decoder function that retrieves a Lua +value from the stack and converts it to the corresponding C type. -An encoder is a function that takes a ``lua_State *`` and a C type and pushes -onto the Lua stack a value representing the C type. For C structs, the usual -case, this will typically be a Lua table (tables are the only datastructure Lua -has). For example, here is the encoder function for ``struct prefix``: +Encoders and decoders are provided for common data types. +Developers wishing to pass their own data structures between C and Lua need to +create encoders and decoders for that data type. +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 -unmarshalls a Lua value into that C type. -Again, for ``struct prefix``: +There are two: ``lua_decode*`` and ``lua_to*``. The former does no mememory +allocation and is needed for ``frrscript_call``. +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,22 +385,45 @@ 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 - when they return. - For decoders that unmarshall fields from tables, remember to pop the table - at the end. + ``lua_decode*`` functions should pop all values that ``lua_to*`` pushed onto + the Lua stack. + For encoders that pushed a table, its decoder should pop the table 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 -returns a pointer to the newly allocated C type. + +``int`` is not a non const-qualified pointer, so for ``int``: + +.. 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 @@ -274,18 +442,11 @@ allocated with ``MTYPE_TMP``. 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 -``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. +Registering encoders and decoders for frrscript_call +"""""""""""""""""""""""""""""""""""""""""""""""""""" -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 @@ -331,11 +492,12 @@ For that, use ``lua_decode_noop``: .. note:: - Marshalled types are not restricted to simple values like integers, strings - and tables. It is possible to marshall a type such that the resultant object - in Lua is an actual object-oriented object, complete with methods that call - 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. + Encodable/decodable types are not restricted to simple values like integers, + strings and tables. + It is possible to encode a type such that the resultant object in Lua + is an actual object-oriented object, complete with methods that call + 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 to return a match / no match / match and update result. +script with a function named ``route_match``, +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: - -- action - -- Set to the appropriate status code to indicate what we did - -- attributes - -- Setting fields on here will propagate them back up to the caller if - -- '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 + -- route_match returns table with following keys: + -- integer action, required + -- resultant status code. Should be one of RM_* + -- table attributes, optional + -- updated route attributes + -- + 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 From 0972af957af79989cad173948d4a9f810a9e7bf2 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Fri, 9 Jul 2021 01:42:56 +0800 Subject: [PATCH 19/45] bgpd: Update bgpd example with get_result Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index c2062a5f60..0f4d096641 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -365,23 +365,23 @@ 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_new(scriptname); - if (!fs) { zlog_err("Issue loading script file; defaulting to no match"); return RMAP_NOMATCH; } - if (frrscript_load(fs, "route_match", NULL)) { + if (frrscript_load(fs, routematch_function, NULL)) { zlog_err( "Issue loading script function; defaulting to no match"); return RMAP_NOMATCH; } - enum frrlua_rm_status lrm_status = LUA_RM_FAILURE, + enum frrlua_rm_status status_failure = LUA_RM_FAILURE, status_nomatch = LUA_RM_NOMATCH, status_match = LUA_RM_MATCH, status_match_and_change = LUA_RM_MATCH_AND_CHANGE; @@ -389,21 +389,24 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) struct attr newattr = *path->attr; int result = frrscript_call( - fs, "route_match", ("RM_FAILURE", (long long *)&lrm_status), + fs, routematch_function, + ("prefix", prefix), ("attributes", &newattr), ("peer", path->peer), + ("RM_FAILURE", (long long *)&status_failure), ("RM_NOMATCH", (long long *)&status_nomatch), ("RM_MATCH", (long long *)&status_match), - ("RM_MATCH_AND_CHANGE", (long long *)&status_match_and_change), - ("action", (long long *)&lrm_status), ("prefix", prefix), - ("attributes", &newattr), ("peer", path->peer)); + ("RM_MATCH_AND_CHANGE", (long long *)&status_match_and_change) + ); 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", @@ -434,6 +437,8 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) break; } + XFREE(MTYPE_TMP, action); + frrscript_delete(fs); return status; From 8878080b1b5185c45ce1ec6f633d628ca41643ad Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Wed, 14 Jul 2021 05:41:29 +0800 Subject: [PATCH 20/45] lib: Remove ../ include Signed-off-by: Donald Lee --- lib/frrscript.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/frrscript.h b/lib/frrscript.h index 14a922a741..905cda1a95 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -25,7 +25,7 @@ #include #include "frrlua.h" -#include "../bgpd/bgp_script.h" +#include "bgpd/bgp_script.h" #ifdef __cplusplus extern "C" { From 2b67227e6e69a0d5ce1831185f95182f8874ee37 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 17 Jul 2021 20:46:10 +0800 Subject: [PATCH 21/45] lib: Add int encoder/decoder Signed-off-by: Donald Lee --- lib/frrlua.c | 8 ++++++++ lib/frrlua.h | 4 +++- lib/frrscript.h | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/frrlua.c b/lib/frrlua.c index e97e48121c..710d9ece00 100644 --- a/lib/frrlua.c +++ b/lib/frrlua.c @@ -309,6 +309,14 @@ void lua_decode_noop(lua_State *L, int idx, const void *ptr) { } + +/* + * Noop decoder for int. + */ +void lua_decode_int_noop(lua_State *L, int idx, int i) +{ +} + /* * Logging. * diff --git a/lib/frrlua.h b/lib/frrlua.h index c4de82740c..2c86d87cbd 100644 --- a/lib/frrlua.h +++ b/lib/frrlua.h @@ -162,10 +162,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 decocders */ void lua_decode_noop(lua_State *L, int idx, const void *ptr); +void lua_decode_int_noop(lua_State *L, int idx, int i); + /* * Retrieve an integer from table on the top of the stack. * diff --git a/lib/frrscript.h b/lib/frrscript.h index 905cda1a95..be6820bed5 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -142,6 +142,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, \ @@ -157,6 +158,7 @@ const struct prefix * : lua_pushprefix \ #define DECODE_ARGS_WITH_STATE(L, value) \ _Generic((value), \ +int : lua_decode_int_noop, \ long long * : lua_decode_integerp, \ struct prefix * : lua_decode_prefix, \ struct interface * : lua_decode_interface, \ From 62435f8ce1609ab44b2e050f210aae2a099129c5 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 17 Jul 2021 21:14:27 +0800 Subject: [PATCH 22/45] bgpd: Use int encoder/decoder Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 0f4d096641..abcd5e7900 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -389,13 +389,12 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) struct attr newattr = *path->attr; int result = frrscript_call( - fs, routematch_function, - ("prefix", prefix), ("attributes", &newattr), ("peer", path->peer), - ("RM_FAILURE", (long long *)&status_failure), - ("RM_NOMATCH", (long long *)&status_nomatch), - ("RM_MATCH", (long long *)&status_match), - ("RM_MATCH_AND_CHANGE", (long long *)&status_match_and_change) - ); + fs, routematch_function, ("prefix", prefix), + ("attributes", &newattr), ("peer", path->peer), + ("RM_FAILURE", (int)status_failure), + ("RM_NOMATCH", (int)status_nomatch), + ("RM_MATCH", (int)status_match), + ("RM_MATCH_AND_CHANGE", (int)status_match_and_change)); if (result) { zlog_err("Issue running script rule; defaulting to no match"); From 7472871eefd0bd8f9b46606372ab816511aac0d2 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 18 Jul 2021 06:20:20 +0800 Subject: [PATCH 23/45] lib: Comment functions Signed-off-by: Donald Lee --- lib/frrscript.h | 47 ++++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/lib/frrscript.h b/lib/frrscript.h index be6820bed5..d756b49533 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -76,9 +76,13 @@ 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_new(const char *name); +struct frrscript *frrscript_new(const char *scriptname); /* * Load a function into frrscript, run callback if any @@ -87,7 +91,7 @@ int frrscript_load(struct frrscript *fs, const char *function_name, int (*load_cb)(struct frrscript *)); /* - * Destroy FRR script. + * Delete Lua function states and frrscript */ void frrscript_delete(struct frrscript *fs); @@ -173,10 +177,12 @@ const struct prefix * : lua_decode_noop \ )(L, -1, value) /* - * Call script. + * Call Lua function state (abstraction for a single Lua function) * - * fs - * The script to call; this is obtained from frrscript_load(). + * lfs + * 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. @@ -184,16 +190,17 @@ const struct prefix * : lua_decode_noop \ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs); /* - * Wrapper for call script. Maps values passed in to their encoder - * and decoder types. + * Wrapper for calling Lua function state. Maps values passed in to their + * 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, f, ...) \ ({ \ struct lua_function_state lookup = {.name = f}; \ @@ -222,18 +229,24 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs); }) /* - * Get result from finished script. + * Get result from finished function * * fs * The script. This script must have been run already. - * - * result - * The result to extract from the script. - * This reuses the frrscript_env type, but only the typename and name fields - * need to be set. The value is returned directly. + * function_name + * Name of the Lua function. + * name + * Name of the result. + * 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, const char *function_name, const char *name, From 9d6204020daf5d1f97bcc7f21d0c8776aa686672 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 18 Jul 2021 06:22:52 +0800 Subject: [PATCH 24/45] bgpd: Remove warning about not finding script Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index abcd5e7900..9146e90fbf 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -370,14 +370,9 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) struct frrscript *fs = frrscript_new(scriptname); - if (!fs) { - zlog_err("Issue loading script file; defaulting to no match"); - return RMAP_NOMATCH; - } - if (frrscript_load(fs, routematch_function, NULL)) { zlog_err( - "Issue loading script function; defaulting to no match"); + "Issue loading script or function function; defaulting to no match"); return RMAP_NOMATCH; } From 596b44af8a51c79a7e23447c8c45baf0d6638a63 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 18 Jul 2021 06:25:15 +0800 Subject: [PATCH 25/45] lib: Remove warning about script not found Signed-off-by: Donald Lee --- lib/command.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/command.c b/lib/command.c index ceea186a9d..4da59c9dd6 100644 --- a/lib/command.c +++ b/lib/command.c @@ -2430,12 +2430,11 @@ DEFUN(script, (void)str2prefix("1.2.3.4/24", &p); struct frrscript *fs = frrscript_new(argv[1]->arg); - if (fs == NULL) { - vty_out(vty, "Script '/etc/frr/scripts/%s.lua' not found\n", - argv[1]->arg); + if (frrscript_load(fs, argv[2]->arg, NULL)) { + vty_out(vty, + "/etc/frr/scripts%s.lua or function '/%s' not found\n", + argv[1]->arg, argv[2]->arg); } - if (frrscript_load(fs, argv[2]->arg, NULL)) - vty_out(vty, "Script function '/%s' not found\n", argv[2]->arg); int ret = frrscript_call(fs, argv[2]->arg, ("p", &p)); char buf[40]; From 2ce634e2ad6cdfddb79e00cda01f21280d521279 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sun, 18 Jul 2021 06:25:54 +0800 Subject: [PATCH 26/45] lib: formatting Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 3 ++- lib/frrscript.c | 7 +++--- lib/frrscript.h | 59 +++++++++++++++++++++++---------------------- 3 files changed, 36 insertions(+), 33 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 9146e90fbf..5bdc37c9ad 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -396,7 +396,8 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) return RMAP_NOMATCH; } - long long* action = frrscript_get_result(fs, routematch_function, "action", lua_tointegerp); + long long *action = frrscript_get_result(fs, routematch_function, + "action", lua_tointegerp); int status = RMAP_NOMATCH; diff --git a/lib/frrscript.c b/lib/frrscript.c index b3f8a9f6f5..b385ad7eaa 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -205,12 +205,12 @@ void *frrscript_get_result(struct frrscript *fs, const char *function_name, lfs = hash_lookup(fs->lua_function_hash, &lookup); - if (lfs == NULL) { + if (lfs == NULL) return NULL; - } /* results table is idx 1 on the stack, getfield pushes our item to idx - * 2*/ + * 2 + */ lua_getfield(lfs->L, 1, name); if (lua_isnil(lfs->L, -1)) { lua_pop(lfs->L, 1); @@ -259,6 +259,7 @@ int frrscript_load(struct frrscript *fs, const char *function_name, /* Set up the Lua script */ lua_State *L = luaL_newstate(); + frrlua_export_logging(L); char script_name[MAXPATHLEN * 2]; diff --git a/lib/frrscript.h b/lib/frrscript.h index d756b49533..e76d14c1f4 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -146,7 +146,7 @@ void frrscript_init(const char *scriptdir); */ #define ENCODE_ARGS_WITH_STATE(L, value) \ _Generic((value), \ -int: lua_pushinteger, \ +int : lua_pushinteger, \ long long * : lua_pushintegerp, \ struct prefix * : lua_pushprefix, \ struct interface * : lua_pushinterface, \ @@ -180,9 +180,8 @@ const struct prefix * : lua_decode_noop \ * Call Lua function state (abstraction for a single Lua function) * * lfs - * The Lua function to call; this should have been loaded in by frrscript_load(). - * nargs - * Number of arguments the function accepts + * 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. @@ -201,31 +200,33 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs); * Returns: * 0 if the script ran successfully, nonzero otherwise. */ -#define frrscript_call(fs, f, ...) \ - ({ \ - struct lua_function_state lookup = {.name = f}; \ - struct lua_function_state *lfs; \ - lfs = hash_lookup(fs->lua_function_hash, &lookup); \ - lfs == NULL ? ({ \ - zlog_err( \ - "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \ - 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; \ - }); \ +#define frrscript_call(fs, f, ...) \ + ({ \ + struct lua_function_state lookup = {.name = f}; \ + struct lua_function_state *lfs; \ + lfs = hash_lookup(fs->lua_function_hash, &lookup); \ + lfs == NULL ? ({ \ + zlog_err( \ + "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \ + 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; \ + }); \ }) /* From bca62fe0457c158c695d4d2e4b1693a2cfb4e301 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Fri, 23 Jul 2021 16:37:43 +0800 Subject: [PATCH 27/45] bgpd: fix typo Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 5bdc37c9ad..478e9cc26b 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -372,7 +372,7 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) if (frrscript_load(fs, routematch_function, NULL)) { zlog_err( - "Issue loading script or function function; defaulting to no match"); + "Issue loading script or function; defaulting to no match"); return RMAP_NOMATCH; } From 59a35b667d4f592f284e7c13db9123b7eb6379b6 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Fri, 23 Jul 2021 16:38:16 +0800 Subject: [PATCH 28/45] bgpd: Use enum as input to Lua script Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 478e9cc26b..b01186e657 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -376,20 +376,14 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) return RMAP_NOMATCH; } - enum frrlua_rm_status status_failure = 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, routematch_function, ("prefix", prefix), ("attributes", &newattr), ("peer", path->peer), - ("RM_FAILURE", (int)status_failure), - ("RM_NOMATCH", (int)status_nomatch), - ("RM_MATCH", (int)status_match), - ("RM_MATCH_AND_CHANGE", (int)status_match_and_change)); + ("RM_FAILURE", LUA_RM_FAILURE), ("RM_NOMATCH", LUA_RM_NOMATCH), + ("RM_MATCH", LUA_RM_MATCH), + ("RM_MATCH_AND_CHANGE", LUA_RM_MATCH_AND_CHANGE)); if (result) { zlog_err("Issue running script rule; defaulting to no match"); From 70d995abd4e6938dcdaccc61a6f9cd66946d99d1 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Fri, 23 Jul 2021 17:03:42 +0800 Subject: [PATCH 29/45] lib: Comment on how macro works Signed-off-by: Donald Lee --- lib/frrscript.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/frrscript.h b/lib/frrscript.h index e76d14c1f4..8f5b9f9cf8 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -122,8 +122,22 @@ void frrscript_register_type_codecs(struct frrscript_codec *codecs); */ void frrscript_init(const char *scriptdir); +/* + * This macro is mapped to every (name, value) in frrscript_call, + * so this in turn maps them onto their encoders + */ #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_getfield(lfs->L, 1, name); \ From 08bf2b41b82f3bc46c052044a4630a8534b819e9 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Fri, 23 Jul 2021 17:04:27 +0800 Subject: [PATCH 30/45] lib: formatting Signed-off-by: Donald Lee --- lib/frrscript.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/frrscript.h b/lib/frrscript.h index 8f5b9f9cf8..03a8b9ec6a 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -195,7 +195,7 @@ const struct prefix * : lua_decode_noop \ * * lfs * The Lua function to call; this should have been loaded in by - * frrscript_load(). nargs Number of arguments the function accepts + * frrscript_load(). nargs Number of arguments the function accepts * * Returns: * 0 if the script ran successfully, nonzero otherwise. From 67b64027b2a5fc97d58f1298e5b99da1bbe78a68 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Fri, 23 Jul 2021 17:15:38 +0800 Subject: [PATCH 31/45] lib: parens around macro args Signed-off-by: Donald Lee --- lib/frrscript.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/frrscript.h b/lib/frrscript.h index 03a8b9ec6a..905ffe1623 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -216,13 +216,13 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs); */ #define frrscript_call(fs, f, ...) \ ({ \ - struct lua_function_state lookup = {.name = f}; \ + struct lua_function_state lookup = {.name = (f)}; \ struct lua_function_state *lfs; \ - lfs = hash_lookup(fs->lua_function_hash, &lookup); \ + lfs = hash_lookup((fs)->lua_function_hash, &lookup); \ lfs == NULL ? ({ \ zlog_err( \ "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \ - fs->name, f); \ + (fs)->name, (f)); \ 1; \ }) \ : ({ \ @@ -233,7 +233,7 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs); ? ({ \ zlog_err( \ "frrscript: '%s.lua': '%s': this function called but returned non-zero exit code. No variables modified.", \ - fs->name, f); \ + (fs)->name, (f)); \ 1; \ }) \ : ({ \ From 1763ed26559eda64663f1224a07b46001084d985 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Fri, 23 Jul 2021 23:25:20 +0800 Subject: [PATCH 32/45] lib: Cap script_name length Signed-off-by: Donald Lee --- lib/frrscript.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index b385ad7eaa..3f2544f4a0 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -262,10 +262,15 @@ int frrscript_load(struct frrscript *fs, const char *function_name, frrlua_export_logging(L); - char script_name[MAXPATHLEN * 2]; + char script_name[MAXPATHLEN]; - snprintf(script_name, sizeof(script_name), "%s/%s.lua", scriptdir, - fs->name); + if (snprintf(script_name, sizeof(script_name), "%s/%s.lua", scriptdir, + fs->name) + < 0) { + 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) { From 1da9c4bdbb5ffdaf83af482af031fa97ed47eb15 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 24 Jul 2021 00:17:33 +0800 Subject: [PATCH 33/45] lib: Use negative indices and add comments Signed-off-by: Donald Lee --- lib/frrscript.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index 3f2544f4a0..8fe8a93f05 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -208,10 +208,14 @@ void *frrscript_get_result(struct frrscript *fs, const char *function_name, if (lfs == NULL) return NULL; - /* results table is idx 1 on the stack, getfield pushes our item to idx - * 2 + /* 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. */ - lua_getfield(lfs->L, 1, name); + 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( @@ -221,6 +225,11 @@ void *frrscript_get_result(struct frrscript *fs, const char *function_name, } p = lua_to(lfs->L, 2); + /* At the end, the Lua state should be same as it was at the start + * i.e. containing soley the returned table. */ + assert(lua_gettop(lfs->L) == 1); + assert(lua_istable(lfs->L, -1) == 1); + return p; } From 8be973f5c29fdadd1a156fd60bcb66a0d58d695c Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 24 Jul 2021 00:29:21 +0800 Subject: [PATCH 34/45] lib: Add function name to script command Signed-off-by: Donald Lee --- lib/command.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/command.c b/lib/command.c index 4da59c9dd6..122e189342 100644 --- a/lib/command.c +++ b/lib/command.c @@ -2419,11 +2419,10 @@ DEFUN(find, } #if defined(DEV_BUILD) && defined(HAVE_SCRIPTING) -DEFUN(script, - script_cmd, - "script SCRIPT", - "Test command - execute a script\n" - "Script name (same as filename in /etc/frr/scripts/\n") +DEFUN(script, script_cmd, "script SCRIPT FUNCTION", + "Test command - execute a function in a script\n" + "Script name (same as filename in /etc/frr/scripts/)\n" + "Function name (in the script)\n") { struct prefix p; @@ -2432,7 +2431,7 @@ DEFUN(script, if (frrscript_load(fs, argv[2]->arg, NULL)) { vty_out(vty, - "/etc/frr/scripts%s.lua or function '/%s' not found\n", + "/etc/frr/scripts/%s.lua or function '%s' not found\n", argv[1]->arg, argv[2]->arg); } From 26693b3afce6abfaf77ac4833ce9f594ef7118b1 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 24 Jul 2021 03:09:49 +0800 Subject: [PATCH 35/45] lib: typo in tests Signed-off-by: Donald Lee --- tests/lib/test_frrscript.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c index 2d5746b587..44e644c52a 100644 --- a/tests/lib/test_frrscript.c +++ b/tests/lib/test_frrscript.c @@ -37,7 +37,7 @@ int main(int argc, char **argv) result = frrscript_call(fs, "foo", ("a", &a), ("b", &b)); assert(result == 0); assert(a == 101); - assert(b == 202); + assert(b == 201); a = 100, b = 200; @@ -49,7 +49,7 @@ int main(int argc, char **argv) /* a should not occur in the returned table in script */ assert(a == 100); - assert(b == 202); + assert(b == 201); assert(*cptr == 303); XFREE(MTYPE_TMP, cptr); From fe80e6019b5fbca6d3acf4d3be92f45f4a53ad18 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 24 Jul 2021 03:16:18 +0800 Subject: [PATCH 36/45] doc: typo Signed-off-by: Donald Lee --- doc/developer/scripting.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/developer/scripting.rst b/doc/developer/scripting.rst index 232c20f61c..e40029c2b4 100644 --- a/doc/developer/scripting.rst +++ b/doc/developer/scripting.rst @@ -245,7 +245,7 @@ To delete a script and the all Lua states associated with it: A complete example """""""""""""""""" -So, a typical exection call, with error checking, looks something like this: +So, a typical execution call, with error checking, looks something like this: .. code-block:: c From 6d28552d2da20852a3580da17894d7b7db6756ba Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 24 Jul 2021 03:16:25 +0800 Subject: [PATCH 37/45] lib: comment typo Signed-off-by: Donald Lee --- lib/frrlua.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/frrlua.h b/lib/frrlua.h index 2c86d87cbd..d7668abca0 100644 --- a/lib/frrlua.h +++ b/lib/frrlua.h @@ -162,7 +162,7 @@ void lua_decode_stringp(lua_State *L, int idx, char *str); void *lua_tostringp(lua_State *L, int idx); /* - * No-op decocders + * No-op decoders */ void lua_decode_noop(lua_State *L, int idx, const void *ptr); From 4093300ee891bd7559b93e2b7c94fb6fc68b5ddc Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 24 Jul 2021 03:34:28 +0800 Subject: [PATCH 38/45] lib: zlog err in error cases during lua loading Signed-off-by: Donald Lee --- lib/frrscript.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index 8fe8a93f05..59fce36c7a 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -317,11 +317,18 @@ int frrscript_load(struct frrscript *fs, const char *function_name, /* Push the Lua function we want */ lua_getglobal(L, function_name); - if (lua_isfunction(L, lua_gettop(L)) == 0) + 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; + } - if (load_cb && (*load_cb)(fs) != 0) + 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}; From 36cf58c78594aed9fc79ab408d4a00e79e1ba483 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Sat, 24 Jul 2021 03:41:12 +0800 Subject: [PATCH 39/45] lib: Rename decode_int Signed-off-by: Donald Lee --- lib/frrlua.c | 2 +- lib/frrlua.h | 2 +- lib/frrscript.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/frrlua.c b/lib/frrlua.c index 710d9ece00..5d0732eac0 100644 --- a/lib/frrlua.c +++ b/lib/frrlua.c @@ -313,7 +313,7 @@ void lua_decode_noop(lua_State *L, int idx, const void *ptr) /* * Noop decoder for int. */ -void lua_decode_int_noop(lua_State *L, int idx, int i) +void lua_decode_integer_noop(lua_State *L, int idx, int i) { } diff --git a/lib/frrlua.h b/lib/frrlua.h index d7668abca0..98bff00fd6 100644 --- a/lib/frrlua.h +++ b/lib/frrlua.h @@ -166,7 +166,7 @@ void *lua_tostringp(lua_State *L, int idx); */ void lua_decode_noop(lua_State *L, int idx, const void *ptr); -void lua_decode_int_noop(lua_State *L, int idx, int i); +void lua_decode_integer_noop(lua_State *L, int idx, int i); /* * Retrieve an integer from table on the top of the stack. diff --git a/lib/frrscript.h b/lib/frrscript.h index 905ffe1623..fba124909a 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -176,7 +176,7 @@ const struct prefix * : lua_pushprefix \ #define DECODE_ARGS_WITH_STATE(L, value) \ _Generic((value), \ -int : lua_decode_int_noop, \ +int : lua_decode_integer_noop, \ long long * : lua_decode_integerp, \ struct prefix * : lua_decode_prefix, \ struct interface * : lua_decode_interface, \ From 6ffeb9f79fb2efc2b87767491ab7d8a32e4532a2 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Mon, 26 Jul 2021 22:45:40 +0800 Subject: [PATCH 40/45] bgpd: Comment for included Signed-off-by: Donald Lee --- lib/frrscript.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/frrscript.h b/lib/frrscript.h index fba124909a..953c458145 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -25,7 +25,7 @@ #include #include "frrlua.h" -#include "bgpd/bgp_script.h" +#include "bgpd/bgp_script.h" // for peer and attr encoders/decoders #ifdef __cplusplus extern "C" { From 78f1ac25744f3db6d64570037066380f9b347abe Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Mon, 26 Jul 2021 23:27:43 +0800 Subject: [PATCH 41/45] lib: Add new MTYPE for script results Signed-off-by: Donald Lee --- bgpd/bgp_routemap.c | 2 +- doc/developer/scripting.rst | 6 +++--- lib/frrlua.c | 22 ++++++++++++++-------- lib/frrlua.h | 2 ++ tests/lib/test_frrscript.c | 4 ++-- 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index b01186e657..4b71ba30ce 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -426,7 +426,7 @@ route_match_script(void *rule, const struct prefix *prefix, void *object) break; } - XFREE(MTYPE_TMP, action); + XFREE(MTYPE_SCRIPT_RES, action); frrscript_delete(fs); diff --git a/doc/developer/scripting.rst b/doc/developer/scripting.rst index e40029c2b4..fa1c521fc6 100644 --- a/doc/developer/scripting.rst +++ b/doc/developer/scripting.rst @@ -274,7 +274,7 @@ So, a typical execution call, with error checking, looks something like this: goto DONE; // "d" might not have been in returned table assert(*d == 800); - XFREE(MTYPE_TMP, d); // caller responsible for free + XFREE(MTYPE_SCRIPT_RES, d); // caller responsible for free DONE: frrscript_delete(fs); @@ -430,7 +430,7 @@ This function can and should be implemented using ``lua_decode_*``: 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; @@ -438,7 +438,7 @@ 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. diff --git a/lib/frrlua.c b/lib/frrlua.c index 5d0732eac0..96d7269440 100644 --- a/lib/frrlua.c +++ b/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; } diff --git a/lib/frrlua.h b/lib/frrlua.h index 98bff00fd6..3e16c82e22 100644 --- a/lib/frrlua.h +++ b/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 diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c index 44e644c52a..7b23045978 100644 --- a/tests/lib/test_frrscript.c +++ b/tests/lib/test_frrscript.c @@ -51,7 +51,7 @@ int main(int argc, char **argv) assert(a == 100); assert(b == 201); assert(*cptr == 303); - XFREE(MTYPE_TMP, cptr); + XFREE(MTYPE_SCRIPT_RES, cptr); long long n = 5; @@ -62,7 +62,7 @@ int main(int argc, char **argv) long long *ansptr = frrscript_get_result(fs, "fact", "ans", lua_tointegerp); assert(*ansptr == 120); - XFREE(MTYPE_TMP, ansptr); + XFREE(MTYPE_SCRIPT_RES, ansptr); /* Negative testing */ From 84c92002d25b7a498f9210356874dbe45a306e8a Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Mon, 26 Jul 2021 23:35:17 +0800 Subject: [PATCH 42/45] lib: formatting Signed-off-by: Donald Lee --- doc/developer/scripting.rst | 14 +++++++------- lib/frrscript.c | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/doc/developer/scripting.rst b/doc/developer/scripting.rst index fa1c521fc6..7251bafb05 100644 --- a/doc/developer/scripting.rst +++ b/doc/developer/scripting.rst @@ -62,18 +62,18 @@ are respectively encapsulated in the following structures: .. code-block:: c struct frrscript { - /* Lua file name */ - char *name; + /* Lua file name */ + char *name; - /* hash of lua_function_states */ - struct hash *lua_function_hash; + /* hash of lua_function_states */ + struct hash *lua_function_hash; }; struct lua_function_state { - /* Lua function name */ - char *name; + /* Lua function name */ + char *name; - lua_State *L; + lua_State *L; }; diff --git a/lib/frrscript.c b/lib/frrscript.c index 59fce36c7a..3e1e184bc6 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -226,7 +226,8 @@ void *frrscript_get_result(struct frrscript *fs, const char *function_name, p = lua_to(lfs->L, 2); /* At the end, the Lua state should be same as it was at the start - * i.e. containing soley the returned table. */ + * i.e. containing soley the returned table. + */ assert(lua_gettop(lfs->L) == 1); assert(lua_istable(lfs->L, -1) == 1); From 9e3a277b0442861482a2dae9070fb5a1dd887aeb Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 29 Jul 2021 05:41:09 +0800 Subject: [PATCH 43/45] lib: Fix condition for snprintf Signed-off-by: Donald Lee --- lib/frrscript.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/frrscript.c b/lib/frrscript.c index 3e1e184bc6..d00b84ccbb 100644 --- a/lib/frrscript.c +++ b/lib/frrscript.c @@ -276,7 +276,7 @@ int frrscript_load(struct frrscript *fs, const char *function_name, if (snprintf(script_name, sizeof(script_name), "%s/%s.lua", scriptdir, fs->name) - < 0) { + >= (int)sizeof(script_name)) { zlog_err("frrscript: path to script %s/%s.lua is too long", scriptdir, fs->name); goto fail; From b5e790ee4d2d94f9dead7914e0de97ac0e0a470e Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 29 Jul 2021 05:50:21 +0800 Subject: [PATCH 44/45] lib: Add parens around macro args Signed-off-by: Donald Lee --- lib/frrscript.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/frrscript.h b/lib/frrscript.h index 953c458145..540676c099 100644 --- a/lib/frrscript.h +++ b/lib/frrscript.h @@ -126,7 +126,7 @@ void frrscript_init(const char *scriptdir); * This macro is mapped to every (name, value) in frrscript_call, * so this in turn maps them onto their encoders */ -#define ENCODE_ARGS(name, value) ENCODE_ARGS_WITH_STATE(lfs->L, value) +#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 @@ -140,11 +140,11 @@ void frrscript_init(const char *scriptdir); */ #define DECODE_ARGS(name, value) \ do { \ - lua_getfield(lfs->L, 1, name); \ + lua_getfield(lfs->L, 1, (name)); \ if (lua_isnil(lfs->L, 2)) { \ lua_pop(lfs->L, 1); \ } else { \ - DECODE_ARGS_WITH_STATE(lfs->L, value); \ + DECODE_ARGS_WITH_STATE(lfs->L, (value)); \ } \ assert(lua_gettop(lfs->L) == 1); \ } while (0) @@ -172,7 +172,7 @@ 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), \ @@ -188,7 +188,7 @@ 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 Lua function state (abstraction for a single Lua function) From c5f9744c33df5a9d335691d73fbeb1ad8d0e58d4 Mon Sep 17 00:00:00 2001 From: Donald Lee Date: Thu, 29 Jul 2021 05:51:58 +0800 Subject: [PATCH 45/45] docs: update parens Signed-off-by: Donald Lee --- doc/developer/scripting.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/developer/scripting.rst b/doc/developer/scripting.rst index 7251bafb05..d543ed3560 100644 --- a/doc/developer/scripting.rst +++ b/doc/developer/scripting.rst @@ -457,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), \ @@ -465,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