From 08c4c73be6073282e73a9d7074212df39e27aa5c Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Aug 2017 09:00:28 +0200 Subject: [PATCH 1/8] lib: hooks: support priority ordering & reversing Allow registering callbacks with a priority value used to order them relative to each other. Plus a reverse variant that just flips the direction on priorities. Signed-off-by: David Lamparter --- lib/hook.c | 16 ++++++++++++---- lib/hook.h | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/lib/hook.c b/lib/hook.c index 2c877cbf45..1468c4d329 100644 --- a/lib/hook.c +++ b/lib/hook.c @@ -26,17 +26,25 @@ DEFINE_MTYPE_STATIC(LIB, HOOK_ENTRY, "Hook entry") void _hook_register(struct hook *hook, void *funcptr, void *arg, bool has_arg, - struct frrmod_runtime *module, const char *funcname) + struct frrmod_runtime *module, const char *funcname, + int priority) { - struct hookent *he = XCALLOC(MTYPE_HOOK_ENTRY, sizeof(*he)); + struct hookent *he = XCALLOC(MTYPE_HOOK_ENTRY, sizeof(*he)), **pos; he->hookfn = funcptr; he->hookarg = arg; he->has_arg = has_arg; he->module = module; he->fnname = funcname; + he->priority = priority; - he->next = hook->entries; - hook->entries = he; + for (pos = &hook->entries; *pos; pos = &(*pos)->next) + if (hook->reverse + ? (*pos)->priority < priority + : (*pos)->priority >= priority) + break; + + he->next = *pos; + *pos = he; } void _hook_unregister(struct hook *hook, void *funcptr, void *arg, bool has_arg) diff --git a/lib/hook.h b/lib/hook.h index 4a5cee2fd3..5f45e113e7 100644 --- a/lib/hook.h +++ b/lib/hook.h @@ -74,6 +74,29 @@ * hook_register_arg (some_update_event, event_handler, addonptr); * * (addonptr isn't typesafe, but that should be manageable.) + * + * Hooks also support a "priority" value for ordering registered calls + * relative to each other. The priority is a signed integer where lower + * values are called earlier. There is also "Koohs", which is hooks with + * reverse priority ordering (for cleanup/deinit hooks, so you can use the + * same priority value). + * + * Recommended priority value ranges are: + * + * -999 ... 0 ... 999 - main executable / daemon, or library + * -1999 ... -1000 - modules registering calls that should run before + * the daemon's bits + * 1000 ... 1999 - modules calls that should run after daemon's + * + * Note: the default value is 1000, based on the following 2 expectations: + * - most hook_register() usage will be in loadable modules + * - usage of hook_register() in the daemon itself may need relative ordering + * to itself, making an explicit value the expected case + * + * The priority value is passed as extra argument on hook_register_prio() / + * hook_register_arg_prio(). Whether a hook runs in reverse is determined + * solely by the code defining / calling the hook. (DECLARE_KOOH is actually + * the same thing as DECLARE_HOOK, it's just there to make it obvious.) */ /* TODO: @@ -94,6 +117,7 @@ struct hookent { void *hookfn; /* actually a function pointer */ void *hookarg; bool has_arg; + int priority; struct frrmod_runtime *module; const char *fnname; }; @@ -101,8 +125,11 @@ struct hookent { struct hook { const char *name; struct hookent *entries; + bool reverse; }; +#define HOOK_DEFAULT_PRIORITY 1000 + /* subscribe/add callback function to a hook * * always use hook_register(), which uses the static inline helper from @@ -110,14 +137,21 @@ struct hook { */ extern void _hook_register(struct hook *hook, void *funcptr, void *arg, bool has_arg, struct frrmod_runtime *module, - const char *funcname); + const char *funcname, int priority); #define hook_register(hookname, func) \ _hook_register(&_hook_##hookname, _hook_typecheck_##hookname(func), \ - NULL, false, THIS_MODULE, #func) + NULL, false, THIS_MODULE, #func, HOOK_DEFAULT_PRIORITY) #define hook_register_arg(hookname, func, arg) \ _hook_register(&_hook_##hookname, \ _hook_typecheck_arg_##hookname(func), arg, true, \ - THIS_MODULE, #func) + THIS_MODULE, #func, HOOK_DEFAULT_PRIORITY) +#define hook_register_prio(hookname, prio, func) \ + _hook_register(&_hook_##hookname, _hook_typecheck_##hookname(func), \ + NULL, false, THIS_MODULE, #func, prio) +#define hook_register_arg_prio(hookname, prio, func, arg) \ + _hook_register(&_hook_##hookname, \ + _hook_typecheck_arg_##hookname(func), \ + arg, true, THIS_MODULE, #func, prio) extern void _hook_unregister(struct hook *hook, void *funcptr, void *arg, bool has_arg); @@ -156,12 +190,14 @@ extern void _hook_unregister(struct hook *hook, void *funcptr, void *arg, { \ return (void *)funcptr; \ } +#define DECLARE_KOOH(hookname, arglist, passlist) \ + DECLARE_HOOK(hookname, arglist, passlist) /* use in source file - contains hook-related definitions. */ -#define DEFINE_HOOK(hookname, arglist, passlist) \ +#define DEFINE_HOOK_INT(hookname, arglist, passlist, rev) \ struct hook _hook_##hookname = { \ - .name = #hookname, .entries = NULL, \ + .name = #hookname, .entries = NULL, .reverse = rev, \ }; \ static int hook_call_##hookname arglist \ { \ @@ -184,4 +220,9 @@ extern void _hook_unregister(struct hook *hook, void *funcptr, void *arg, return hooksum; \ } +#define DEFINE_HOOK(hookname, arglist, passlist) \ + DEFINE_HOOK_INT(hookname, arglist, passlist, false) +#define DEFINE_KOOH(hookname, arglist, passlist) \ + DEFINE_HOOK_INT(hookname, arglist, passlist, true) + #endif /* _FRR_HOOK_H */ From 03951374770da5be620e48768588928fbd809c83 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 6 Aug 2017 08:28:16 +0200 Subject: [PATCH 2/8] *: centralize some exit cleanup into libfrr Start creating a counterpart to frr_init and frr_late_init. Unfortunately, some daemons don't do any exit handling, this doesn't change that just yet. Signed-off-by: David Lamparter --- bgpd/bgp_main.c | 12 ++++-------- ldpd/ldpd.c | 8 +++----- lib/libfrr.c | 21 +++++++++++++++++++++ lib/libfrr.h | 8 ++++++++ nhrpd/nhrp_main.c | 9 ++------- ospf6d/ospf6_main.c | 10 +++------- zebra/main.c | 8 +++----- 7 files changed, 44 insertions(+), 32 deletions(-) diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c index 25a562ed68..a80dff271d 100644 --- a/bgpd/bgp_main.c +++ b/bgpd/bgp_main.c @@ -172,6 +172,8 @@ static __attribute__((__noreturn__)) void bgp_exit(int status) /* it only makes sense for this to be called on a clean exit */ assert(status == 0); + frr_early_fini(); + bfd_gbl_exit(); bgp_close(); @@ -214,22 +216,16 @@ static __attribute__((__noreturn__)) void bgp_exit(int status) community_list_terminate(bgp_clist); bgp_vrf_terminate(); - cmd_terminate(); - vty_terminate(); #if ENABLE_BGP_VNC vnc_zebra_destroy(); #endif bgp_zebra_destroy(); - /* reverse bgp_master_init */ - if (bm->master) - thread_master_free(bm->master); - - closezlog(); - list_delete(bm->bgp); memset(bm, 0, sizeof(*bm)); + frr_fini(); + if (bgp_debug_count()) log_memstats_stderr("bgpd"); exit(status); diff --git a/ldpd/ldpd.c b/ldpd/ldpd.c index 80af2b14e5..0a586ec1c5 100644 --- a/ldpd/ldpd.c +++ b/ldpd/ldpd.c @@ -392,6 +392,8 @@ ldpd_shutdown(void) pid_t pid; int status; + frr_early_fini(); + /* close pipes */ msgbuf_clear(&iev_ldpe->ibuf.w); close(iev_ldpe->ibuf.fd); @@ -423,13 +425,9 @@ ldpd_shutdown(void) vrf_terminate(); access_list_reset(); - cmd_terminate(); - vty_terminate(); ldp_zebra_destroy(); - zprivs_terminate(&ldpd_privs); - thread_master_free(master); - closezlog(); + frr_fini(); exit(0); } diff --git a/lib/libfrr.c b/lib/libfrr.c index a5c87e6edc..255f91ec71 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -37,6 +37,8 @@ #include "network.h" DEFINE_HOOK(frr_late_init, (struct thread_master * tm), (tm)) +DEFINE_KOOH(frr_early_fini, (), ()) +DEFINE_KOOH(frr_fini, (), ()) const char frr_sysconfdir[] = SYSCONFDIR; const char frr_vtydir[] = DAEMON_VTY_DIR; @@ -831,3 +833,22 @@ void frr_run(struct thread_master *master) while (thread_fetch(master, &thread)) thread_call(&thread); } + +void frr_early_fini(void) +{ + hook_call(frr_early_fini); +} + +void frr_fini(void) +{ + hook_call(frr_fini); + + /* memory_init -> nothing needed */ + vty_terminate(); + cmd_terminate(); + zprivs_terminate(di->privs); + /* signal_init -> nothing needed */ + thread_master_free(master); + closezlog(); + /* frrmod_init -> nothing needed / hooks */ +} diff --git a/lib/libfrr.h b/lib/libfrr.h index 1710fc9a84..8a15d168a1 100644 --- a/lib/libfrr.h +++ b/lib/libfrr.h @@ -104,6 +104,14 @@ extern void frr_run(struct thread_master *master); extern bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len, const char *path); +/* these two are before the protocol daemon does its own shutdown + * it's named this way being the counterpart to frr_late_init */ +DECLARE_KOOH(frr_early_fini, (), ()) +extern void frr_early_fini(void); +/* and these two are after the daemon did its own cleanup */ +DECLARE_KOOH(frr_fini, (), ()) +extern void frr_fini(void); + extern char config_default[256]; extern char frr_zclientpath[256]; extern const char frr_sysconfdir[]; diff --git a/nhrpd/nhrp_main.c b/nhrpd/nhrp_main.c index 012d5cd87c..3a7186c1d7 100644 --- a/nhrpd/nhrp_main.c +++ b/nhrpd/nhrp_main.c @@ -81,6 +81,7 @@ static void nhrp_sigusr1(void) static void nhrp_request_stop(void) { debugf(NHRP_DEBUG_COMMON, "Exiting..."); + frr_early_fini(); nhrp_shortcut_terminate(); nhrp_nhs_terminate(); @@ -89,15 +90,9 @@ static void nhrp_request_stop(void) evmgr_terminate(); nhrp_vc_terminate(); vrf_terminate(); - /* memory_terminate(); */ - /* vty_terminate(); */ - cmd_terminate(); - /* signal_terminate(); */ - zprivs_terminate(&nhrpd_privs); debugf(NHRP_DEBUG_COMMON, "Done."); - - closezlog(); + frr_fini(); exit(0); } diff --git a/ospf6d/ospf6_main.c b/ospf6d/ospf6_main.c index 28bb956c40..e582737f94 100644 --- a/ospf6d/ospf6_main.c +++ b/ospf6d/ospf6_main.c @@ -82,6 +82,8 @@ static void __attribute__((noreturn)) ospf6_exit(int status) struct listnode *node; struct interface *ifp; + frr_early_fini(); + if (ospf6) ospf6_delete(ospf6); @@ -96,19 +98,13 @@ static void __attribute__((noreturn)) ospf6_exit(int status) ospf6_lsa_terminate(); vrf_terminate(); - vty_terminate(); - cmd_terminate(); if (zclient) { zclient_stop(zclient); zclient_free(zclient); } - if (master) - thread_master_free(master); - - closezlog(); - + frr_fini(); exit(status); } diff --git a/zebra/main.c b/zebra/main.c index 538c2f0663..c3a7d3635f 100644 --- a/zebra/main.c +++ b/zebra/main.c @@ -125,6 +125,7 @@ static void sigint(void) zlog_notice("Terminating on signal"); + frr_early_fini(); #ifdef HAVE_IRDP irdp_finish(); #endif @@ -147,17 +148,14 @@ static void sigint(void) access_list_reset(); prefix_list_reset(); route_map_finish(); - cmd_terminate(); - vty_terminate(); - zprivs_terminate(&zserv_privs); + list_delete(zebrad.client_list); work_queue_free(zebrad.ribq); if (zebrad.lsp_process_q) work_queue_free(zebrad.lsp_process_q); meta_queue_free(zebrad.mq); - thread_master_free(zebrad.master); - closezlog(); + frr_fini(); exit(0); } From 2a6a7a656d0d1e0afff091515737cdee2a3dbfe8 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Aug 2017 10:30:37 +0200 Subject: [PATCH 3/8] doc: sample code-doc in .rst+sphinx Signed-off-by: David Lamparter --- doc/code/.gitignore | 3 + doc/code/Makefile | 216 +++++++++++++++++++++++++++++++ doc/code/conf.py | 293 ++++++++++++++++++++++++++++++++++++++++++ doc/code/hooks.rst | 171 ++++++++++++++++++++++++ doc/code/index.rst | 18 +++ doc/code/library.rst | 10 ++ doc/code/memtypes.rst | 117 +++++++++++++++++ 7 files changed, 828 insertions(+) create mode 100644 doc/code/.gitignore create mode 100644 doc/code/Makefile create mode 100644 doc/code/conf.py create mode 100644 doc/code/hooks.rst create mode 100644 doc/code/index.rst create mode 100644 doc/code/library.rst create mode 100644 doc/code/memtypes.rst diff --git a/doc/code/.gitignore b/doc/code/.gitignore new file mode 100644 index 0000000000..0505537159 --- /dev/null +++ b/doc/code/.gitignore @@ -0,0 +1,3 @@ +/_templates +/_build +!/Makefile diff --git a/doc/code/Makefile b/doc/code/Makefile new file mode 100644 index 0000000000..056b78e68e --- /dev/null +++ b/doc/code/Makefile @@ -0,0 +1,216 @@ +# Makefile for Sphinx documentation +# + +# You can set these variables from the command line. +SPHINXOPTS = +SPHINXBUILD = sphinx-build +PAPER = +BUILDDIR = _build + +# User-friendly check for sphinx-build +ifeq ($(shell which $(SPHINXBUILD) >/dev/null 2>&1; echo $$?), 1) +$(error The '$(SPHINXBUILD)' command was not found. Make sure you have Sphinx installed, then set the SPHINXBUILD environment variable to point to the full path of the '$(SPHINXBUILD)' executable. Alternatively you can add the directory with the executable to your PATH. If you don't have Sphinx installed, grab it from http://sphinx-doc.org/) +endif + +# Internal variables. +PAPEROPT_a4 = -D latex_paper_size=a4 +PAPEROPT_letter = -D latex_paper_size=letter +ALLSPHINXOPTS = -d $(BUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) . +# the i18n builder cannot share the environment and doctrees with the others +I18NSPHINXOPTS = $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) . + +.PHONY: help +help: + @echo "Please use \`make ' where is one of" + @echo " html to make standalone HTML files" + @echo " dirhtml to make HTML files named index.html in directories" + @echo " singlehtml to make a single large HTML file" + @echo " pickle to make pickle files" + @echo " json to make JSON files" + @echo " htmlhelp to make HTML files and a HTML help project" + @echo " qthelp to make HTML files and a qthelp project" + @echo " applehelp to make an Apple Help Book" + @echo " devhelp to make HTML files and a Devhelp project" + @echo " epub to make an epub" + @echo " latex to make LaTeX files, you can set PAPER=a4 or PAPER=letter" + @echo " latexpdf to make LaTeX files and run them through pdflatex" + @echo " latexpdfja to make LaTeX files and run them through platex/dvipdfmx" + @echo " text to make text files" + @echo " man to make manual pages" + @echo " texinfo to make Texinfo files" + @echo " info to make Texinfo files and run them through makeinfo" + @echo " gettext to make PO message catalogs" + @echo " changes to make an overview of all changed/added/deprecated items" + @echo " xml to make Docutils-native XML files" + @echo " pseudoxml to make pseudoxml-XML files for display purposes" + @echo " linkcheck to check all external links for integrity" + @echo " doctest to run all doctests embedded in the documentation (if enabled)" + @echo " coverage to run coverage check of the documentation (if enabled)" + +.PHONY: clean +clean: + rm -rf $(BUILDDIR)/* + +.PHONY: html +html: + $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html + @echo + @echo "Build finished. The HTML pages are in $(BUILDDIR)/html." + +.PHONY: dirhtml +dirhtml: + $(SPHINXBUILD) -b dirhtml $(ALLSPHINXOPTS) $(BUILDDIR)/dirhtml + @echo + @echo "Build finished. The HTML pages are in $(BUILDDIR)/dirhtml." + +.PHONY: singlehtml +singlehtml: + $(SPHINXBUILD) -b singlehtml $(ALLSPHINXOPTS) $(BUILDDIR)/singlehtml + @echo + @echo "Build finished. The HTML page is in $(BUILDDIR)/singlehtml." + +.PHONY: pickle +pickle: + $(SPHINXBUILD) -b pickle $(ALLSPHINXOPTS) $(BUILDDIR)/pickle + @echo + @echo "Build finished; now you can process the pickle files." + +.PHONY: json +json: + $(SPHINXBUILD) -b json $(ALLSPHINXOPTS) $(BUILDDIR)/json + @echo + @echo "Build finished; now you can process the JSON files." + +.PHONY: htmlhelp +htmlhelp: + $(SPHINXBUILD) -b htmlhelp $(ALLSPHINXOPTS) $(BUILDDIR)/htmlhelp + @echo + @echo "Build finished; now you can run HTML Help Workshop with the" \ + ".hhp project file in $(BUILDDIR)/htmlhelp." + +.PHONY: qthelp +qthelp: + $(SPHINXBUILD) -b qthelp $(ALLSPHINXOPTS) $(BUILDDIR)/qthelp + @echo + @echo "Build finished; now you can run "qcollectiongenerator" with the" \ + ".qhcp project file in $(BUILDDIR)/qthelp, like this:" + @echo "# qcollectiongenerator $(BUILDDIR)/qthelp/FRR.qhcp" + @echo "To view the help file:" + @echo "# assistant -collectionFile $(BUILDDIR)/qthelp/FRR.qhc" + +.PHONY: applehelp +applehelp: + $(SPHINXBUILD) -b applehelp $(ALLSPHINXOPTS) $(BUILDDIR)/applehelp + @echo + @echo "Build finished. The help book is in $(BUILDDIR)/applehelp." + @echo "N.B. You won't be able to view it unless you put it in" \ + "~/Library/Documentation/Help or install it in your application" \ + "bundle." + +.PHONY: devhelp +devhelp: + $(SPHINXBUILD) -b devhelp $(ALLSPHINXOPTS) $(BUILDDIR)/devhelp + @echo + @echo "Build finished." + @echo "To view the help file:" + @echo "# mkdir -p $$HOME/.local/share/devhelp/FRR" + @echo "# ln -s $(BUILDDIR)/devhelp $$HOME/.local/share/devhelp/FRR" + @echo "# devhelp" + +.PHONY: epub +epub: + $(SPHINXBUILD) -b epub $(ALLSPHINXOPTS) $(BUILDDIR)/epub + @echo + @echo "Build finished. The epub file is in $(BUILDDIR)/epub." + +.PHONY: latex +latex: + $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) $(BUILDDIR)/latex + @echo + @echo "Build finished; the LaTeX files are in $(BUILDDIR)/latex." + @echo "Run \`make' in that directory to run these through (pdf)latex" \ + "(use \`make latexpdf' here to do that automatically)." + +.PHONY: latexpdf +latexpdf: + $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) $(BUILDDIR)/latex + @echo "Running LaTeX files through pdflatex..." + $(MAKE) -C $(BUILDDIR)/latex all-pdf + @echo "pdflatex finished; the PDF files are in $(BUILDDIR)/latex." + +.PHONY: latexpdfja +latexpdfja: + $(SPHINXBUILD) -b latex $(ALLSPHINXOPTS) $(BUILDDIR)/latex + @echo "Running LaTeX files through platex and dvipdfmx..." + $(MAKE) -C $(BUILDDIR)/latex all-pdf-ja + @echo "pdflatex finished; the PDF files are in $(BUILDDIR)/latex." + +.PHONY: text +text: + $(SPHINXBUILD) -b text $(ALLSPHINXOPTS) $(BUILDDIR)/text + @echo + @echo "Build finished. The text files are in $(BUILDDIR)/text." + +.PHONY: man +man: + $(SPHINXBUILD) -b man $(ALLSPHINXOPTS) $(BUILDDIR)/man + @echo + @echo "Build finished. The manual pages are in $(BUILDDIR)/man." + +.PHONY: texinfo +texinfo: + $(SPHINXBUILD) -b texinfo $(ALLSPHINXOPTS) $(BUILDDIR)/texinfo + @echo + @echo "Build finished. The Texinfo files are in $(BUILDDIR)/texinfo." + @echo "Run \`make' in that directory to run these through makeinfo" \ + "(use \`make info' here to do that automatically)." + +.PHONY: info +info: + $(SPHINXBUILD) -b texinfo $(ALLSPHINXOPTS) $(BUILDDIR)/texinfo + @echo "Running Texinfo files through makeinfo..." + make -C $(BUILDDIR)/texinfo info + @echo "makeinfo finished; the Info files are in $(BUILDDIR)/texinfo." + +.PHONY: gettext +gettext: + $(SPHINXBUILD) -b gettext $(I18NSPHINXOPTS) $(BUILDDIR)/locale + @echo + @echo "Build finished. The message catalogs are in $(BUILDDIR)/locale." + +.PHONY: changes +changes: + $(SPHINXBUILD) -b changes $(ALLSPHINXOPTS) $(BUILDDIR)/changes + @echo + @echo "The overview file is in $(BUILDDIR)/changes." + +.PHONY: linkcheck +linkcheck: + $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(BUILDDIR)/linkcheck + @echo + @echo "Link check complete; look for any errors in the above output " \ + "or in $(BUILDDIR)/linkcheck/output.txt." + +.PHONY: doctest +doctest: + $(SPHINXBUILD) -b doctest $(ALLSPHINXOPTS) $(BUILDDIR)/doctest + @echo "Testing of doctests in the sources finished, look at the " \ + "results in $(BUILDDIR)/doctest/output.txt." + +.PHONY: coverage +coverage: + $(SPHINXBUILD) -b coverage $(ALLSPHINXOPTS) $(BUILDDIR)/coverage + @echo "Testing of coverage in the sources finished, look at the " \ + "results in $(BUILDDIR)/coverage/python.txt." + +.PHONY: xml +xml: + $(SPHINXBUILD) -b xml $(ALLSPHINXOPTS) $(BUILDDIR)/xml + @echo + @echo "Build finished. The XML files are in $(BUILDDIR)/xml." + +.PHONY: pseudoxml +pseudoxml: + $(SPHINXBUILD) -b pseudoxml $(ALLSPHINXOPTS) $(BUILDDIR)/pseudoxml + @echo + @echo "Build finished. The pseudo-XML files are in $(BUILDDIR)/pseudoxml." diff --git a/doc/code/conf.py b/doc/code/conf.py new file mode 100644 index 0000000000..38be7f2fca --- /dev/null +++ b/doc/code/conf.py @@ -0,0 +1,293 @@ +# -*- coding: utf-8 -*- +# +# FRR documentation build configuration file, created by +# sphinx-quickstart on Tue Jan 31 16:00:52 2017. +# +# This file is execfile()d with the current directory set to its +# containing dir. +# +# Note that not all possible configuration values are present in this +# autogenerated file. +# +# All configuration values have a default; values that are commented out +# serve to show the default. + +import sys +import os +import re + +# If extensions (or modules to document with autodoc) are in another directory, +# add these directories to sys.path here. If the directory is relative to the +# documentation root, use os.path.abspath to make it absolute, like shown here. +#sys.path.insert(0, os.path.abspath('.')) + +# -- General configuration ------------------------------------------------ + +# If your documentation needs a minimal Sphinx version, state it here. +#needs_sphinx = '1.0' + +# Add any Sphinx extension module names here, as strings. They can be +# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom +# ones. +extensions = ['sphinx.ext.todo'] + +# Add any paths that contain templates here, relative to this directory. +templates_path = ['_templates'] + +# The suffix(es) of source filenames. +# You can specify multiple suffix as a list of string: +# source_suffix = ['.rst', '.md'] +source_suffix = '.rst' + +# The encoding of source files. +#source_encoding = 'utf-8-sig' + +# The master toctree document. +master_doc = 'index' + +# General information about the project. +project = u'FRR' +copyright = u'2017, FRR' +author = u'FRR' + +# The version info for the project you're documenting, acts as replacement for +# |version| and |release|, also used in various other places throughout the +# built documents. + +# The short X.Y version. +version = u'?.?' +# The full version, including alpha/beta/rc tags. +release = u'?.?-?' + +val = re.compile('^S\["([^"]+)"\]="(.*)"$') +with open('../../config.status', 'r') as cfgstatus: + for ln in cfgstatus.readlines(): + m = val.match(ln) + if m is None: continue + if m.group(1) == 'PACKAGE_VERSION': + release = m.group(2) + version = release.split('-')[0] + +# The language for content autogenerated by Sphinx. Refer to documentation +# for a list of supported languages. +# +# This is also used if you do content translation via gettext catalogs. +# Usually you set "language" from the command line for these cases. +language = None + +# There are two options for replacing |today|: either, you set today to some +# non-false value, then it is used: +#today = '' +# Else, today_fmt is used as the format for a strftime call. +#today_fmt = '%B %d, %Y' + +# List of patterns, relative to source directory, that match files and +# directories to ignore when looking for source files. +exclude_patterns = ['_build'] + +# The reST default role (used for this markup: `text`) to use for all +# documents. +#default_role = None + +# If true, '()' will be appended to :func: etc. cross-reference text. +#add_function_parentheses = True + +# If true, the current module name will be prepended to all description +# unit titles (such as .. function::). +#add_module_names = True + +# If true, sectionauthor and moduleauthor directives will be shown in the +# output. They are ignored by default. +#show_authors = False + +# The name of the Pygments (syntax highlighting) style to use. +pygments_style = 'sphinx' + +# A list of ignored prefixes for module index sorting. +#modindex_common_prefix = [] + +# If true, keep warnings as "system message" paragraphs in the built documents. +#keep_warnings = False + +# If true, `todo` and `todoList` produce output, else they produce nothing. +todo_include_todos = True + + +# -- Options for HTML output ---------------------------------------------- + +# The theme to use for HTML and HTML Help pages. See the documentation for +# a list of builtin themes. +html_theme = 'sphinx_rtd_theme' + +# Theme options are theme-specific and customize the look and feel of a theme +# further. For a list of options available for each theme, see the +# documentation. +#html_theme_options = {} + +# Add any paths that contain custom themes here, relative to this directory. +#html_theme_path = [] + +# The name for this set of Sphinx documents. If None, it defaults to +# " v documentation". +#html_title = None + +# A shorter title for the navigation bar. Default is the same as html_title. +#html_short_title = None + +# The name of an image file (relative to this directory) to place at the top +# of the sidebar. +#html_logo = None + +# The name of an image file (within the static path) to use as favicon of the +# docs. This file should be a Windows icon file (.ico) being 16x16 or 32x32 +# pixels large. +#html_favicon = None + +# Add any paths that contain custom static files (such as style sheets) here, +# relative to this directory. They are copied after the builtin static files, +# so a file named "default.css" will overwrite the builtin "default.css". +html_static_path = ['_static'] + +# Add any extra paths that contain custom files (such as robots.txt or +# .htaccess) here, relative to this directory. These files are copied +# directly to the root of the documentation. +#html_extra_path = [] + +# If not '', a 'Last updated on:' timestamp is inserted at every page bottom, +# using the given strftime format. +#html_last_updated_fmt = '%b %d, %Y' + +# If true, SmartyPants will be used to convert quotes and dashes to +# typographically correct entities. +#html_use_smartypants = True + +# Custom sidebar templates, maps document names to template names. +#html_sidebars = {} + +# Additional templates that should be rendered to pages, maps page names to +# template names. +#html_additional_pages = {} + +# If false, no module index is generated. +#html_domain_indices = True + +# If false, no index is generated. +#html_use_index = True + +# If true, the index is split into individual pages for each letter. +#html_split_index = False + +# If true, links to the reST sources are added to the pages. +#html_show_sourcelink = True + +# If true, "Created using Sphinx" is shown in the HTML footer. Default is True. +#html_show_sphinx = True + +# If true, "(C) Copyright ..." is shown in the HTML footer. Default is True. +#html_show_copyright = True + +# If true, an OpenSearch description file will be output, and all pages will +# contain a tag referring to it. The value of this option must be the +# base URL from which the finished HTML is served. +#html_use_opensearch = '' + +# This is the file name suffix for HTML files (e.g. ".xhtml"). +#html_file_suffix = None + +# Language to be used for generating the HTML full-text search index. +# Sphinx supports the following languages: +# 'da', 'de', 'en', 'es', 'fi', 'fr', 'hu', 'it', 'ja' +# 'nl', 'no', 'pt', 'ro', 'ru', 'sv', 'tr' +#html_search_language = 'en' + +# A dictionary with options for the search language support, empty by default. +# Now only 'ja' uses this config value +#html_search_options = {'type': 'default'} + +# The name of a javascript file (relative to the configuration directory) that +# implements a search results scorer. If empty, the default will be used. +#html_search_scorer = 'scorer.js' + +# Output file base name for HTML help builder. +htmlhelp_basename = 'FRRdoc' + +# -- Options for LaTeX output --------------------------------------------- + +latex_elements = { +# The paper size ('letterpaper' or 'a4paper'). +#'papersize': 'letterpaper', + +# The font size ('10pt', '11pt' or '12pt'). +#'pointsize': '10pt', + +# Additional stuff for the LaTeX preamble. +#'preamble': '', + +# Latex figure (float) alignment +#'figure_align': 'htbp', +} + +# Grouping the document tree into LaTeX files. List of tuples +# (source start file, target name, title, +# author, documentclass [howto, manual, or own class]). +latex_documents = [ + (master_doc, 'FRR.tex', u'FRR Documentation', + u'FRR', 'manual'), +] + +# The name of an image file (relative to this directory) to place at the top of +# the title page. +#latex_logo = None + +# For "manual" documents, if this is true, then toplevel headings are parts, +# not chapters. +#latex_use_parts = False + +# If true, show page references after internal links. +#latex_show_pagerefs = False + +# If true, show URL addresses after external links. +#latex_show_urls = False + +# Documents to append as an appendix to all manuals. +#latex_appendices = [] + +# If false, no module index is generated. +#latex_domain_indices = True + + +# -- Options for manual page output --------------------------------------- + +# One entry per manual page. List of tuples +# (source start file, name, description, authors, manual section). +man_pages = [ + (master_doc, 'frr', u'FRR Documentation', + [author], 1) +] + +# If true, show URL addresses after external links. +#man_show_urls = False + + +# -- Options for Texinfo output ------------------------------------------- + +# Grouping the document tree into Texinfo files. List of tuples +# (source start file, target name, title, author, +# dir menu entry, description, category) +texinfo_documents = [ + (master_doc, 'FRR', u'FRR Documentation', + author, 'FRR', 'One line description of project.', + 'Miscellaneous'), +] + +# Documents to append as an appendix to all manuals. +#texinfo_appendices = [] + +# If false, no module index is generated. +#texinfo_domain_indices = True + +# How to display URL addresses: 'footnote', 'no', or 'inline'. +#texinfo_show_urls = 'footnote' + +# If true, do not generate a @detailmenu in the "Top" node's menu. +#texinfo_no_detailmenu = False diff --git a/doc/code/hooks.rst b/doc/code/hooks.rst new file mode 100644 index 0000000000..0afa297aa7 --- /dev/null +++ b/doc/code/hooks.rst @@ -0,0 +1,171 @@ +.. highlight:: c + +Hooks +===== + +Libfrr provides type-safe subscribable hook points where other pieces of +code can add one or more callback functions. "type-safe" in this case +applies to the function pointers used for subscriptions. The +implementations checks (at compile-time) wheter a callback to be added has +the appropriate function signature (parameters) for the hook. + +Example: + +.. code-block:: c + :caption: mydaemon.h + + #include "hook.h" + DECLARE_HOOK(some_update_event, (struct eventinfo *info), (info)) + +.. code-block:: c + :caption: mydaemon.c + + #include "mydaemon.h" + DEFINE_HOOK(some_update_event, (struct eventinfo *info), (info)) + ... + hook_call(some_update_event, info); + +.. code-block:: c + :caption: mymodule.c + + #include "mydaemon.h" + static int event_handler(struct eventinfo *info); + ... + hook_register(some_update_event, event_handler); + +Do not use parameter names starting with "hook", these can collide with +names used by the hook code itself. + + +Return values +------------- + +Callbacks to be placed on hooks always return "int" for now; hook_call will +sum up the return values from each called function. (The default is 0 if no +callbacks are registered.) + +There are no pre-defined semantics for the value, in most cases it is +ignored. For success/failure indication, 0 should be success, and +handlers should make sure to only return 0 or 1 (not -1 or other values). + +There is no built-in way to abort executing a chain after a failure of one +of the callbacks. If this is needed, the hook can use an extra +``bool *aborted`` argument. + + +Priorities +---------- + +Hooks support a "priority" value for ordering registered calls +relative to each other. The priority is a signed integer where lower +values are called earlier. There are also "Koohs", which is hooks with +reverse priority ordering (for cleanup/deinit hooks, so you can use the +same priority value). + +Recommended priority value ranges are: + +======================== =================================================== +Range Usage +------------------------ --------------------------------------------------- + -999 ... 0 ... 999 main executable / daemon, or library + +-1999 ... -1000 modules registering calls that should run before + the daemon's bits + +1000 ... 1999 modules' calls that should run after daemon's + (includes default value: 1000) +======================== =================================================== + +Note: the default value is 1000, based on the following 2 expectations: + +- most hook_register() usage will be in loadable modules +- usage of hook_register() in the daemon itself may need relative ordering + to itself, making an explicit value the expected case + +The priority value is passed as extra argument on hook_register_prio() / +hook_register_arg_prio(). Whether a hook runs in reverse is determined +solely by the code defining / calling the hook. (DECLARE_KOOH is actually +the same thing as DECLARE_HOOK, it's just there to make it obvious.) + + +Definition +---------- + +.. c:macro:: DECLARE_HOOK(name, arglist, passlist) +.. c:macro:: DECLARE_KOOH(name, arglist, passlist) + + :param name: Name of the hook to be defined + :param arglist: Function definition style parameter list in braces. + :param passlist: List of the same parameters without their types. + + Note: the second and third macro args must be the hook function's + parameter list, with the same names for each parameter. The second + macro arg is with types (used for defining things), the third arg is + just the names (used for passing along parameters). + + This macro must be placed in a header file; this header file must be + included to register a callback on the hook. + + Examples: + + .. code-block:: c + + DECLARE_HOOK(foo, (), ()) + DECLARE_HOOK(bar, (int arg), (arg)) + DECLARE_HOOK(baz, (const void *x, in_addr_t y), (x, y)) + +.. c:macro:: DEFINE_HOOK(name, arglist, passlist) + + Implements an hook. Each ``DECLARE_HOOK`` must have be accompanied by + exactly one ``DEFINE_HOOK``, which needs to be placed in a source file. + **The hook can only be called from this source file.** This is intentional + to avoid overloading and/or misusing hooks for distinct purposes. + + The compiled source file will include a global symbol with the name of the + hook prefixed by `_hook_`. Trying to register a callback for a hook that + doesn't exist will therefore result in a linker error, or a module + load-time error for dynamic modules. + +.. c:macro:: DEFINE_KOOH(name, arglist, passlist) + + Same as ``DEFINE_HOOK``, but the sense of priorities / order of callbacks + is reversed. This should be used for cleanup hooks. + +.. c:function:: int hook_call(name, ...) + + Calls the specified named hook. Parameters to the hook are passed right + after the hook name, e.g.: + + .. code-block:: c + + hook_call(foo); + hook_call(bar, 0); + hook_call(baz, NULL, INADDR_ANY); + + Returns the sum of return values from all callbacks. The ``DEFINE_HOOK`` + statement for the hook must be placed in the file before any ``hook_call`` + use of the hook. + + +Callback registration +--------------------- + +.. c:function:: void hook_register(name, int (*callback)(...)) +.. c:function:: void hook_register_prio(name, int priority, int (*callback)(...)) +.. c:function:: void hook_register_arg(name, int (*callback)(void *arg, ...), void *arg) +.. c:function:: void hook_register_arg_prio(name, int priority, int (*callback)(void *arg, ...), void *arg) + + Register a callback with an hook. If the caller needs to pass an extra + argument to the callback, the _arg variant can be used and the extra + parameter will be passed as first argument to the callback. There is no + typechecking for this argument. + + The priority value is used as described above. The variants without a + priority parameter use 1000 as priority value. + +.. c:function:: void hook_unregister(name, int (*callback)(...)) +.. c:function:: void hook_unregister_arg(name, int (*callback)(void *arg, ...), void *arg) + + Removes a previously registered callback from a hook. Note that there + is no _prio variant of these calls. The priority value is only used during + registration. diff --git a/doc/code/index.rst b/doc/code/index.rst new file mode 100644 index 0000000000..79647d0b92 --- /dev/null +++ b/doc/code/index.rst @@ -0,0 +1,18 @@ +Welcome to FRR's documentation! +=============================== + +Contents: + +.. toctree:: + :maxdepth: 2 + + library + + +Indices and tables +================== + +* :ref:`genindex` +* :ref:`modindex` +* :ref:`search` + diff --git a/doc/code/library.rst b/doc/code/library.rst new file mode 100644 index 0000000000..dd46021db8 --- /dev/null +++ b/doc/code/library.rst @@ -0,0 +1,10 @@ +libfrr library facilities +========================= + +.. toctree:: + :maxdepth: 2 + + memtypes + hooks + + diff --git a/doc/code/memtypes.rst b/doc/code/memtypes.rst new file mode 100644 index 0000000000..62d211e864 --- /dev/null +++ b/doc/code/memtypes.rst @@ -0,0 +1,117 @@ +.. highlight:: c + +Memtypes +======== + +FRR includes wrappers arround ``malloc()`` and ``free()`` that count the number +of objects currently allocated, for each of a defined ``MTYPE``. + +To this extent, there are `memory groups` and `memory types`. Each memory +type must belong to a memory group, this is used just to provide some basic +structure. + +Example: + +.. code-block:: c + :caption: mydaemon.h + + DECLARE_MGROUP(MYDAEMON) + DECLARE_MTYPE(MYNEIGHBOR) + +.. code-block:: c + :caption: mydaemon.c + + DEFINE_MGROUP( MYDAEMON, "My daemon's memory") + DEFINE_MTYPE( MYDAEMON, MYNEIGHBOR, "Neighbor entry") + DEFINE_MTYPE_STATIC(MYDAEMON, MYNEIGHBORNAME, "Neighbor name") + + struct neigh *neighbor_new(const char *name) + { + struct neigh *n = XMALLOC(MYNEIGHBOR, sizeof(*n)); + n->name = XSTRDUP(MYNEIGHBORNAME, name); + return n; + } + + void neighbor_free(struct neigh *n) + { + XFREE(MYNEIGHBORNAME, n->name); + XFREE(MYNEIGHBOR, n); + } + + +Definition +---------- + +.. c:macro:: DECLARE_MGROUP(name) + + This macro forward-declares a memory group and should be placed in a + ``.h`` file. It expands to an ``extern struct memgroup`` statement. + +.. c:macro:: DEFINE_MGROUP(mname, description) + + Defines/implements a memory group. Must be placed into exactly one ``.c`` + file (multiple inclusion will result in a link-time symbol conflict). + + Contains additional logic (constructor and destructor) to register the + memory group in a global list. + +.. c:macro:: DECLARE_MTYPE(name) + + Forward-declares a memory type and makes ``MTYPE_name`` available for use. + Note that the ``MTYPE_`` prefix must not be included in the name, it is + automatically prefixed. + + ``MTYPE_name`` is created as a `static const` symbol, i.e. a compile-time + constant. It refers to an ``extern struct memtype _mt_name``, where `name` + is replaced with the actual name. + +.. c:macro:: DEFINE_MTYPE(group, name, description) + + Define/implement a memory type, must be placed into exactly one ``.c`` + file (multiple inclusion will result in a link-time symbol conflict). + + Like ``DEFINE_MGROUP``, this contains actual code to register the MTYPE + under its group. + +.. c:macro:: DEFINE_MTYPE_STATIC(group, name, description) + + Same as ``DEFINE_MTYPE``, but the ``DEFINE_MTYPE_STATIC`` variant places + the C ``static`` keyword on the definition, restricting the MTYPE's + availability to the current source file. This should be appropriate in + >80% of cases. + + .. todo:: + + Daemons currently have ``daemon_memory.[ch]`` files listing all of + their MTYPEs. This is not how it should be, most of these types + should be moved into the appropriate files where they are used. + Only a few MTYPEs should remain non-static after that. + + +Usage +----- + +.. c:function:: void *XMALLOC(struct memtype *mtype, size_t size) + +.. c:function:: void *XCALLOC(struct memtype *mtype, size_t size) + +.. c:function:: void *XSTRDUP(struct memtype *mtype, size_t size) + + Allocation wrappers for malloc/calloc/realloc/strdup, taking an extra + mtype parameter. + +.. c:function:: void *XREALLOC(struct memtype *mtype, void *ptr, size_t size) + + Wrapper around realloc() with MTYPE tracking. Note that ``ptr`` may + be NULL, in which case the function does the same as XMALLOC (regardless + of whether the system realloc() supports this.) + +.. c:function:: void XFREE(struct memtype *mtype, void *ptr) + + Wrapper around free(), again taking an extra mtype parameter. This is + actually a macro, with the following additional properties: + + - the macro contains ``ptr = NULL`` + - if ptr is NULL, no operation is performed (as is guaranteed by system + implementations.) Do not surround XFREE with ``if (ptr != NULL)`` + checks. From ce19a04aea80f45ca1da80e28bf3a1253138c691 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Aug 2017 10:50:43 +0200 Subject: [PATCH 4/8] lib: replace if_add_hook with hook_* logic This allows modules to register their own additional hooks on interface creation/deletion. Signed-off-by: David Lamparter --- babeld/babel_interface.c | 4 ++-- bgpd/bgp_main.c | 3 --- eigrpd/eigrp_interface.c | 4 ++-- isisd/isis_circuit.c | 4 ++-- lib/if.c | 35 +++++------------------------------ lib/if.h | 17 ++++++++++++----- nhrpd/nhrp_interface.c | 4 ++-- ospfd/ospf_interface.c | 4 ++-- ripd/rip_interface.c | 4 ++-- ripngd/ripng_interface.c | 4 ++-- zebra/interface.c | 4 ++-- 11 files changed, 33 insertions(+), 54 deletions(-) diff --git a/babeld/babel_interface.c b/babeld/babel_interface.c index 1ae33b3a27..9fa32ee6fa 100644 --- a/babeld/babel_interface.c +++ b/babeld/babel_interface.c @@ -1256,8 +1256,8 @@ void babel_if_init () { /* initialize interface list */ - if_add_hook (IF_NEW_HOOK, babel_if_new_hook); - if_add_hook (IF_DELETE_HOOK, babel_if_delete_hook); + hook_register_prio(if_add, 0, babel_if_new_hook); + hook_register_prio(if_del, 0, babel_if_delete_hook); babel_enable_if = vector_init (1); diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c index a80dff271d..d1359402df 100644 --- a/bgpd/bgp_main.c +++ b/bgpd/bgp_main.c @@ -178,9 +178,6 @@ static __attribute__((__noreturn__)) void bgp_exit(int status) bgp_close(); - if (retain_mode) - if_add_hook(IF_DELETE_HOOK, NULL); - /* reverse bgp_master_init */ for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) bgp_delete(bgp); diff --git a/eigrpd/eigrp_interface.c b/eigrpd/eigrp_interface.c index 7f05e14703..22b6fa394f 100644 --- a/eigrpd/eigrp_interface.c +++ b/eigrpd/eigrp_interface.c @@ -150,8 +150,8 @@ struct list *eigrp_iflist; void eigrp_if_init() { /* Initialize Zebra interface data structure. */ - if_add_hook(IF_NEW_HOOK, eigrp_if_new_hook); - if_add_hook(IF_DELETE_HOOK, eigrp_if_delete_hook); + hook_register_prio(if_add, 0, eigrp_if_new_hook); + hook_register_prio(if_del, 0, eigrp_if_delete_hook); } int eigrp_if_new_hook(struct interface *ifp) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index 9622dcdbc4..a1aa87e396 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -1336,8 +1336,8 @@ int isis_if_delete_hook(struct interface *ifp) void isis_circuit_init() { /* Initialize Zebra interface data structure */ - if_add_hook(IF_NEW_HOOK, isis_if_new_hook); - if_add_hook(IF_DELETE_HOOK, isis_if_delete_hook); + hook_register_prio(if_add, 0, isis_if_new_hook); + hook_register_prio(if_del, 0, isis_if_delete_hook); /* Install interface node */ install_node(&interface_node, isis_interface_config_write); diff --git a/lib/if.c b/lib/if.c index 4e4534851c..43c382beaa 100644 --- a/lib/if.c +++ b/lib/if.c @@ -42,17 +42,12 @@ DEFINE_MTYPE_STATIC(LIB, IF_LINK_PARAMS, "Informational Link Parameters") DEFINE_QOBJ_TYPE(interface) +DEFINE_HOOK(if_add, (struct interface *ifp), (ifp)) +DEFINE_KOOH(if_del, (struct interface *ifp), (ifp)) + /* List of interfaces in only the default VRF */ int ptm_enable = 0; -/* One for each program. This structure is needed to store hooks. */ -struct if_master { - int (*if_new_hook)(struct interface *); - int (*if_delete_hook)(struct interface *); -} if_master = { - 0, -}; - /* Compare interface names, returning an integer greater than, equal to, or * less than 0, (following the strcmp convention), according to the * relationship between ifp1 and ifp2. Interface names consist of an @@ -150,10 +145,7 @@ struct interface *if_create(const char *name, int namelen, vrf_id_t vrf_id) SET_FLAG(ifp->status, ZEBRA_INTERFACE_LINKDETECTION); QOBJ_REG(ifp, interface); - - if (if_master.if_new_hook) - (*if_master.if_new_hook)(ifp); - + hook_call(if_add, ifp); return ifp; } @@ -182,9 +174,7 @@ void if_update_to_new_vrf(struct interface *ifp, vrf_id_t vrf_id) /* Delete interface structure. */ void if_delete_retain(struct interface *ifp) { - if (if_master.if_delete_hook) - (*if_master.if_delete_hook)(ifp); - + hook_call(if_del, ifp); QOBJ_UNREG(ifp); /* Free connected address list */ @@ -209,21 +199,6 @@ void if_delete(struct interface *ifp) XFREE(MTYPE_IF, ifp); } -/* Add hook to interface master. */ -void if_add_hook(int type, int (*func)(struct interface *ifp)) -{ - switch (type) { - case IF_NEW_HOOK: - if_master.if_new_hook = func; - break; - case IF_DELETE_HOOK: - if_master.if_delete_hook = func; - break; - default: - break; - } -} - /* Interface existance check by index. */ struct interface *if_lookup_by_index(ifindex_t ifindex, vrf_id_t vrf_id) { diff --git a/lib/if.h b/lib/if.h index f80ac19179..a592e0ff85 100644 --- a/lib/if.h +++ b/lib/if.h @@ -25,6 +25,7 @@ #include "linklist.h" #include "memory.h" #include "qobj.h" +#include "hook.h" DECLARE_MTYPE(IF) DECLARE_MTYPE(CONNECTED_LABEL) @@ -283,6 +284,17 @@ struct interface { }; DECLARE_QOBJ_TYPE(interface) +/* called from the library code whenever interfaces are created/deleted + * note: interfaces may not be fully realized at that point; also they + * may not exist in the system (ifindex = IFINDEX_INTERNAL) + * + * priority values are important here, daemons should be at 0 while modules + * can use 1000+ so they run after the daemon has initialised daemon-specific + * interface data + */ +DECLARE_HOOK(if_add, (struct interface *ifp), (ifp)) +DECLARE_KOOH(if_del, (struct interface *ifp), (ifp)) + /* Connected address structure. */ struct connected { /* Attached interface. */ @@ -355,10 +367,6 @@ struct nbr_connected { ? (C)->destination \ : (C)->address) -/* Interface hook sort. */ -#define IF_NEW_HOOK 0 -#define IF_DELETE_HOOK 1 - /* There are some interface flags which are only supported by some operating system. */ @@ -442,7 +450,6 @@ extern int if_is_loopback(struct interface *); extern int if_is_broadcast(struct interface *); extern int if_is_pointopoint(struct interface *); extern int if_is_multicast(struct interface *); -extern void if_add_hook(int, int (*)(struct interface *)); extern void if_init(struct list **); extern void if_cmd_init(void); extern void if_terminate(struct list **); diff --git a/nhrpd/nhrp_interface.c b/nhrpd/nhrp_interface.c index 58ad167549..a46962c91a 100644 --- a/nhrpd/nhrp_interface.c +++ b/nhrpd/nhrp_interface.c @@ -48,8 +48,8 @@ static int nhrp_if_delete_hook(struct interface *ifp) void nhrp_interface_init(void) { - if_add_hook(IF_NEW_HOOK, nhrp_if_new_hook); - if_add_hook(IF_DELETE_HOOK, nhrp_if_delete_hook); + hook_register_prio(if_add, 0, nhrp_if_new_hook); + hook_register_prio(if_del, 0, nhrp_if_delete_hook); } void nhrp_interface_update_mtu(struct interface *ifp, afi_t afi) diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index 4ea8ec26f2..54639afd6c 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -1163,6 +1163,6 @@ void ospf_if_init() { /* Initialize Zebra interface data structure. */ om->iflist = vrf_iflist(VRF_DEFAULT); - if_add_hook(IF_NEW_HOOK, ospf_if_new_hook); - if_add_hook(IF_DELETE_HOOK, ospf_if_delete_hook); + hook_register_prio(if_add, 0, ospf_if_new_hook); + hook_register_prio(if_del, 0, ospf_if_delete_hook); } diff --git a/ripd/rip_interface.c b/ripd/rip_interface.c index a170471123..00b6d1cadd 100644 --- a/ripd/rip_interface.c +++ b/ripd/rip_interface.c @@ -1877,8 +1877,8 @@ static int rip_interface_delete_hook(struct interface *ifp) void rip_if_init(void) { /* Default initial size of interface vector. */ - if_add_hook(IF_NEW_HOOK, rip_interface_new_hook); - if_add_hook(IF_DELETE_HOOK, rip_interface_delete_hook); + hook_register_prio(if_add, 0, rip_interface_new_hook); + hook_register_prio(if_del, 0, rip_interface_delete_hook); /* RIP network init. */ rip_enable_interface = vector_init(1); diff --git a/ripngd/ripng_interface.c b/ripngd/ripng_interface.c index c762d8ace7..02fab68254 100644 --- a/ripngd/ripng_interface.c +++ b/ripngd/ripng_interface.c @@ -1121,8 +1121,8 @@ static struct cmd_node interface_node = { void ripng_if_init() { /* Interface initialize. */ - if_add_hook(IF_NEW_HOOK, ripng_if_new_hook); - if_add_hook(IF_DELETE_HOOK, ripng_if_delete_hook); + hook_register_prio(if_add, 0, ripng_if_new_hook); + hook_register_prio(if_del, 0, ripng_if_delete_hook); /* RIPng enable network init. */ ripng_enable_network = route_table_init(); diff --git a/zebra/interface.c b/zebra/interface.c index c4d0363994..48158c82c4 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -2930,8 +2930,8 @@ static int if_config_write(struct vty *vty) void zebra_if_init(void) { /* Initialize interface and new hook. */ - if_add_hook(IF_NEW_HOOK, if_zebra_new_hook); - if_add_hook(IF_DELETE_HOOK, if_zebra_delete_hook); + hook_register_prio(if_add, 0, if_zebra_new_hook); + hook_register_prio(if_del, 0, if_zebra_delete_hook); /* Install configuration write function. */ install_node(&interface_node, if_config_write); From 2eb27eecf011a06c01e58a735d8bf087d7519979 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 6 Aug 2017 08:08:39 +0200 Subject: [PATCH 5/8] zebra: start detangling rtadv & irdp Replace some cross-dependencies with hooks & move bits to where they belong. Signed-off-by: David Lamparter --- zebra/interface.c | 93 ++++-------------------------------------- zebra/interface.h | 16 ++++---- zebra/irdp.h | 4 +- zebra/irdp_interface.c | 7 +++- zebra/irdp_main.c | 9 +++- zebra/main.c | 3 -- zebra/rtadv.c | 77 +++++++++++++++++++++++++++++++++- zebra/rtadv.h | 2 - 8 files changed, 107 insertions(+), 104 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index 48158c82c4..c17e408ea0 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -52,11 +52,10 @@ #define ZEBRA_PTM_SUPPORT -#if defined(HAVE_RTADV) -/* Order is intentional. Matches RFC4191. This array is also used for - command matching, so only modify with care. */ -const char *rtadv_pref_strs[] = {"medium", "high", "INVALID", "low", 0}; -#endif /* HAVE_RTADV */ +DEFINE_HOOK(zebra_if_extra_info, (struct vty *vty, struct interface *ifp), + (vty, ifp)) +DEFINE_HOOK(zebra_if_config_wr, (struct vty *vty, struct interface *ifp), + (vty, ifp)) static void if_down_del_nbr_connected(struct interface *ifp); @@ -996,74 +995,6 @@ static void nbr_connected_dump_vty(struct vty *vty, vty_out(vty, "\n"); } -#if defined(HAVE_RTADV) -/* Dump interface ND information to vty. */ -static void nd_dump_vty(struct vty *vty, struct interface *ifp) -{ - struct zebra_if *zif; - struct rtadvconf *rtadv; - int interval; - - zif = (struct zebra_if *)ifp->info; - rtadv = &zif->rtadv; - - if (rtadv->AdvSendAdvertisements) { - vty_out(vty, - " ND advertised reachable time is %d milliseconds\n", - rtadv->AdvReachableTime); - vty_out(vty, - " ND advertised retransmit interval is %d milliseconds\n", - rtadv->AdvRetransTimer); - vty_out(vty, " ND router advertisements sent: %d rcvd: %d\n", - zif->ra_sent, zif->ra_rcvd); - interval = rtadv->MaxRtrAdvInterval; - if (interval % 1000) - vty_out(vty, - " ND router advertisements are sent every " - "%d milliseconds\n", - interval); - else - vty_out(vty, - " ND router advertisements are sent every " - "%d seconds\n", - interval / 1000); - if (rtadv->AdvDefaultLifetime != -1) - vty_out(vty, - " ND router advertisements live for %d seconds\n", - rtadv->AdvDefaultLifetime); - else - vty_out(vty, - " ND router advertisements lifetime tracks ra-interval\n"); - vty_out(vty, - " ND router advertisement default router preference is " - "%s\n", - rtadv_pref_strs[rtadv->DefaultPreference]); - if (rtadv->AdvManagedFlag) - vty_out(vty, - " Hosts use DHCP to obtain routable addresses.\n"); - else - vty_out(vty, - " Hosts use stateless autoconfig for addresses.\n"); - if (rtadv->AdvHomeAgentFlag) { - vty_out(vty, - " ND router advertisements with Home Agent flag bit set.\n"); - if (rtadv->HomeAgentLifetime != -1) - vty_out(vty, - " Home Agent lifetime is %u seconds\n", - rtadv->HomeAgentLifetime); - else - vty_out(vty, - " Home Agent lifetime tracks ra-lifetime\n"); - vty_out(vty, " Home Agent preference is %u\n", - rtadv->HomeAgentPreference); - } - if (rtadv->AdvIntervalOption) - vty_out(vty, - " ND router advertisements with Adv. Interval option.\n"); - } -} -#endif /* HAVE_RTADV */ - static const char *zebra_ziftype_2str(zebra_iftype_t zif_type) { switch (zif_type) { @@ -1277,12 +1208,8 @@ static void if_dump_vty(struct vty *vty, struct interface *ifp) inet_ntoa(iflp->rmt_ip), iflp->rmt_as); } -#ifdef RTADV - nd_dump_vty(vty, ifp); -#endif /* RTADV */ -#if defined(HAVE_RTADV) - nd_dump_vty(vty, ifp); -#endif /* HAVE_RTADV */ + hook_call(zebra_if_extra_info, vty, ifp); + if (listhead(ifp->nbr_connected)) vty_out(vty, " Neighbor address(s):\n"); for (ALL_LIST_ELEMENTS_RO(ifp->nbr_connected, node, nbr_connected)) @@ -2911,13 +2838,7 @@ static int if_config_write(struct vty *vty) : "no "); } -#if defined(HAVE_RTADV) - rtadv_config_write(vty, ifp); -#endif /* HAVE_RTADV */ - -#ifdef HAVE_IRDP - irdp_config_write(vty, ifp); -#endif /* IRDP */ + hook_call(zebra_if_config_wr, vty, ifp); link_params_config_write(vty, ifp); diff --git a/zebra/interface.h b/zebra/interface.h index 970c3c5292..7b56dcd4a4 100644 --- a/zebra/interface.h +++ b/zebra/interface.h @@ -23,10 +23,7 @@ #include "redistribute.h" #include "vrf.h" - -#ifdef HAVE_IRDP -#include "zebra/irdp.h" -#endif +#include "hook.h" #include "zebra/zebra_l2.h" @@ -202,6 +199,8 @@ typedef enum { ZEBRA_IF_SLAVE_OTHER, /* Something else - e.g., bond slave */ } zebra_slave_iftype_t; +struct irdp_interface; + /* `zebra' daemon local interface structure. */ struct zebra_if { /* Shutdown configuration. */ @@ -227,9 +226,7 @@ struct zebra_if { unsigned int ra_sent, ra_rcvd; #endif /* HAVE_RTADV */ -#ifdef HAVE_IRDP - struct irdp_interface irdp; -#endif + struct irdp_interface *irdp; #ifdef HAVE_STRUCT_SOCKADDR_DL union { @@ -273,6 +270,11 @@ struct zebra_if { struct interface *link; }; +DECLARE_HOOK(zebra_if_extra_info, (struct vty *vty, struct interface *ifp), + (vty, ifp)) +DECLARE_HOOK(zebra_if_config_wr, (struct vty *vty, struct interface *ifp), + (vty, ifp)) + static inline void zebra_if_set_ziftype(struct interface *ifp, zebra_iftype_t zif_type, zebra_slave_iftype_t zif_slave_type) diff --git a/zebra/irdp.h b/zebra/irdp.h index 01308b915b..f8f7811248 100644 --- a/zebra/irdp.h +++ b/zebra/irdp.h @@ -139,9 +139,9 @@ struct Adv { }; extern void irdp_init(void); +extern void irdp_if_init(void); extern int irdp_sock_init(void); -extern void irdp_finish(void); -extern void irdp_config_write(struct vty *, struct interface *); +extern int irdp_config_write(struct vty *, struct interface *); extern int irdp_send_thread(struct thread *t_advert); extern void irdp_advert_off(struct interface *ifp); extern void process_solicit(struct interface *ifp); diff --git a/zebra/irdp_interface.c b/zebra/irdp_interface.c index 5682e12e6f..032090adf2 100644 --- a/zebra/irdp_interface.c +++ b/zebra/irdp_interface.c @@ -316,7 +316,7 @@ static void irdp_if_no_shutdown(struct interface *ifp) /* Write configuration to user */ -void irdp_config_write(struct vty *vty, struct interface *ifp) +int irdp_config_write(struct vty *vty, struct interface *ifp) { struct zebra_if *zi = ifp->info; struct irdp_interface *irdp = &zi->irdp; @@ -348,6 +348,7 @@ void irdp_config_write(struct vty *vty, struct interface *ifp) vty_out(vty, " ip irdp maxadvertinterval %ld\n", irdp->MaxAdvertInterval); } + return 0; } @@ -678,8 +679,10 @@ DEFUN (ip_irdp_debug_disable, return CMD_SUCCESS; } -void irdp_init() +void irdp_if_init() { + hook_register(zebra_if_config_wr, irdp_config_write); + install_element(INTERFACE_NODE, &ip_irdp_broadcast_cmd); install_element(INTERFACE_NODE, &ip_irdp_multicast_cmd); install_element(INTERFACE_NODE, &no_ip_irdp_cmd); diff --git a/zebra/irdp_main.c b/zebra/irdp_main.c index 6220c9d81b..e463608af1 100644 --- a/zebra/irdp_main.c +++ b/zebra/irdp_main.c @@ -301,7 +301,7 @@ void process_solicit(struct interface *ifp) &irdp->t_advertise); } -void irdp_finish() +static int irdp_finish(void) { struct vrf *vrf; struct listnode *node, *nnode; @@ -328,4 +328,11 @@ void irdp_finish() } } +void irdp_init(void) +{ + irdp_if_init(); + + hook_register(frr_early_fini, irdp_finish); +} + #endif /* HAVE_IRDP */ diff --git a/zebra/main.c b/zebra/main.c index c3a7d3635f..72f96add86 100644 --- a/zebra/main.c +++ b/zebra/main.c @@ -126,9 +126,6 @@ static void sigint(void) zlog_notice("Terminating on signal"); frr_early_fini(); -#ifdef HAVE_IRDP - irdp_finish(); -#endif zebra_ptm_finish(); list_delete_all_node(zebrad.client_list); diff --git a/zebra/rtadv.c b/zebra/rtadv.c index b8cf2d490a..2182d6618c 100644 --- a/zebra/rtadv.c +++ b/zebra/rtadv.c @@ -62,6 +62,10 @@ extern struct zebra_privs_t zserv_privs; #define ALLNODE "ff02::1" #define ALLROUTER "ff02::2" +/* Order is intentional. Matches RFC4191. This array is also used for + command matching, so only modify with care. */ +const char *rtadv_pref_strs[] = {"medium", "high", "INVALID", "low", 0}; + enum rtadv_event { RTADV_START, RTADV_STOP, @@ -1456,9 +1460,76 @@ DEFUN (no_ipv6_nd_mtu, return CMD_SUCCESS; } +/* Dump interface ND information to vty. */ +static int nd_dump_vty(struct vty *vty, struct interface *ifp) +{ + struct zebra_if *zif; + struct rtadvconf *rtadv; + int interval; + + zif = (struct zebra_if *)ifp->info; + rtadv = &zif->rtadv; + + if (rtadv->AdvSendAdvertisements) { + vty_out(vty, + " ND advertised reachable time is %d milliseconds\n", + rtadv->AdvReachableTime); + vty_out(vty, + " ND advertised retransmit interval is %d milliseconds\n", + rtadv->AdvRetransTimer); + vty_out(vty, " ND router advertisements sent: %d rcvd: %d\n", + zif->ra_sent, zif->ra_rcvd); + interval = rtadv->MaxRtrAdvInterval; + if (interval % 1000) + vty_out(vty, + " ND router advertisements are sent every " + "%d milliseconds\n", + interval); + else + vty_out(vty, + " ND router advertisements are sent every " + "%d seconds\n", + interval / 1000); + if (rtadv->AdvDefaultLifetime != -1) + vty_out(vty, + " ND router advertisements live for %d seconds\n", + rtadv->AdvDefaultLifetime); + else + vty_out(vty, + " ND router advertisements lifetime tracks ra-interval\n"); + vty_out(vty, + " ND router advertisement default router preference is " + "%s\n", + rtadv_pref_strs[rtadv->DefaultPreference]); + if (rtadv->AdvManagedFlag) + vty_out(vty, + " Hosts use DHCP to obtain routable addresses.\n"); + else + vty_out(vty, + " Hosts use stateless autoconfig for addresses.\n"); + if (rtadv->AdvHomeAgentFlag) { + vty_out(vty, + " ND router advertisements with Home Agent flag bit set.\n"); + if (rtadv->HomeAgentLifetime != -1) + vty_out(vty, + " Home Agent lifetime is %u seconds\n", + rtadv->HomeAgentLifetime); + else + vty_out(vty, + " Home Agent lifetime tracks ra-lifetime\n"); + vty_out(vty, " Home Agent preference is %u\n", + rtadv->HomeAgentPreference); + } + if (rtadv->AdvIntervalOption) + vty_out(vty, + " ND router advertisements with Adv. Interval option.\n"); + } + return 0; +} + /* Write configuration about router advertisement. */ -void rtadv_config_write(struct vty *vty, struct interface *ifp) +static int rtadv_config_write(struct vty *vty, struct interface *ifp) { struct zebra_if *zif; struct listnode *node; @@ -1539,6 +1610,7 @@ void rtadv_config_write(struct vty *vty, struct interface *ifp) vty_out(vty, " router-address"); vty_out(vty, "\n"); } + return 0; } @@ -1600,6 +1672,9 @@ void rtadv_terminate(struct zebra_ns *zns) void rtadv_cmd_init(void) { + hook_register(zebra_if_extra_info, nd_dump_vty); + hook_register(zebra_if_config_wr, rtadv_config_write); + install_element(INTERFACE_NODE, &ipv6_nd_suppress_ra_cmd); install_element(INTERFACE_NODE, &no_ipv6_nd_suppress_ra_cmd); install_element(INTERFACE_NODE, &ipv6_nd_ra_interval_cmd); diff --git a/zebra/rtadv.h b/zebra/rtadv.h index 029c97cddc..dcaeb3ed28 100644 --- a/zebra/rtadv.h +++ b/zebra/rtadv.h @@ -55,8 +55,6 @@ struct rtadv_prefix { #endif }; -extern void rtadv_config_write(struct vty *, struct interface *); - /* RFC4584 Extension to Sockets API for Mobile IPv6 */ #ifndef ND_OPT_ADV_INTERVAL From ead4ee99acd63d2342e9e9dda7a8f5a103a6f550 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 6 Aug 2017 08:57:42 +0200 Subject: [PATCH 6/8] zebra: irdp: manage separate IRDP struct This allocates the per-interface IRDP data as needed; so the pointer in zebra_if is now really opaque. Signed-off-by: David Lamparter --- zebra/irdp_interface.c | 117 +++++++++++++++++++---------------------- zebra/irdp_main.c | 25 ++++++--- zebra/irdp_packet.c | 4 +- 3 files changed, 75 insertions(+), 71 deletions(-) diff --git a/zebra/irdp_interface.c b/zebra/irdp_interface.c index 032090adf2..3465bdebbf 100644 --- a/zebra/irdp_interface.c +++ b/zebra/irdp_interface.c @@ -63,6 +63,25 @@ extern int irdp_sock; +DEFINE_MTYPE_STATIC(ZEBRA, IRDP_IF, "IRDP interface data") + +static struct irdp_interface *irdp_if_get(struct interface *ifp) +{ + struct zebra_if *zi = ifp->info; + if (!zi->irdp) + zi->irdp = XCALLOC(MTYPE_IRDP_IF, sizeof(*zi->irdp)); + return zi->irdp; +} + +static int irdp_if_delete(struct interface *ifp) +{ + struct zebra_if *zi = ifp->info; + if (!zi) + return 0; + XFREE(MTYPE_IRDP_IF, zi->irdp); + return 0; +} + static const char *inet_2a(u_int32_t a, char *b) { sprintf(b, "%u.%u.%u.%u", (a)&0xFF, (a >> 8) & 0xFF, (a >> 16) & 0xFF, @@ -117,10 +136,13 @@ static int if_group(struct interface *ifp, int sock, u_int32_t group, static int if_add_group(struct interface *ifp) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; int ret; char b1[INET_ADDRSTRLEN]; + if (!irdp) + return -1; + ret = if_group(ifp, irdp_sock, INADDR_ALLRTRS_GROUP, IP_ADD_MEMBERSHIP); if (ret < 0) { return ret; @@ -135,10 +157,13 @@ static int if_add_group(struct interface *ifp) static int if_drop_group(struct interface *ifp) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; int ret; char b1[INET_ADDRSTRLEN]; + if (!irdp) + return -1; + ret = if_group(ifp, irdp_sock, INADDR_ALLRTRS_GROUP, IP_DROP_MEMBERSHIP); if (ret < 0) @@ -150,11 +175,8 @@ static int if_drop_group(struct interface *ifp) return 0; } -static void if_set_defaults(struct interface *ifp) +static void if_set_defaults(struct irdp_interface *irdp) { - struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; - irdp->MaxAdvertInterval = IRDP_MAXADVERTINTERVAL; irdp->MinAdvertInterval = IRDP_MINADVERTINTERVAL; irdp->Preference = IRDP_PREFERENCE; @@ -176,11 +198,13 @@ static void irdp_if_start(struct interface *ifp, int multicast, int set_defaults) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; struct listnode *node; struct connected *ifc; u_int32_t timer, seed; + assert(irdp); + if (irdp->flags & IF_ACTIVE) { zlog_warn("IRDP: Interface is already active %s", ifp->name); return; @@ -215,7 +239,7 @@ static void irdp_if_start(struct interface *ifp, int multicast, } if (set_defaults) - if_set_defaults(ifp); + if_set_defaults(irdp); irdp->irdp_sent = 0; @@ -254,7 +278,7 @@ static void irdp_if_start(struct interface *ifp, int multicast, static void irdp_if_stop(struct interface *ifp) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; if (irdp == NULL) { zlog_warn("Interface %s structure is NULL", ifp->name); @@ -281,8 +305,10 @@ static void irdp_if_stop(struct interface *ifp) static void irdp_if_shutdown(struct interface *ifp) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; + if (!irdp) + return; if (irdp->flags & IF_SHUTDOWN) { zlog_warn("IRDP: Interface is already shutdown %s", ifp->name); return; @@ -300,8 +326,7 @@ static void irdp_if_shutdown(struct interface *ifp) static void irdp_if_no_shutdown(struct interface *ifp) { - struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); if (!(irdp->flags & IF_SHUTDOWN)) { zlog_warn("IRDP: Interface is not shutdown %s", ifp->name); @@ -319,11 +344,14 @@ static void irdp_if_no_shutdown(struct interface *ifp) int irdp_config_write(struct vty *vty, struct interface *ifp) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; struct Adv *adv; struct listnode *node; char b1[INET_ADDRSTRLEN]; + if (!irdp) + return 0; + if (irdp->flags & IF_ACTIVE || irdp->flags & IF_SHUTDOWN) { if (irdp->flags & IF_SHUTDOWN) @@ -360,6 +388,7 @@ DEFUN (ip_irdp_multicast, "Use multicast mode\n") { VTY_DECLVAR_CONTEXT(interface, ifp); + irdp_if_get(ifp); irdp_if_start(ifp, TRUE, TRUE); return CMD_SUCCESS; @@ -373,6 +402,7 @@ DEFUN (ip_irdp_broadcast, "Use broadcast mode\n") { VTY_DECLVAR_CONTEXT(interface, ifp); + irdp_if_get(ifp); irdp_if_start(ifp, FALSE, TRUE); return CMD_SUCCESS; @@ -428,11 +458,7 @@ DEFUN (ip_irdp_holdtime, { int idx_number = 3; VTY_DECLVAR_CONTEXT(interface, ifp); - struct zebra_if *zi; - struct irdp_interface *irdp; - - zi = ifp->info; - irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); irdp->Lifetime = atoi(argv[idx_number]->arg); return CMD_SUCCESS; @@ -448,11 +474,7 @@ DEFUN (ip_irdp_minadvertinterval, { int idx_number = 3; VTY_DECLVAR_CONTEXT(interface, ifp); - struct zebra_if *zi; - struct irdp_interface *irdp; - - zi = ifp->info; - irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); if ((unsigned)atoi(argv[idx_number]->arg) <= irdp->MaxAdvertInterval) { irdp->MinAdvertInterval = atoi(argv[idx_number]->arg); @@ -475,11 +497,7 @@ DEFUN (ip_irdp_maxadvertinterval, { int idx_number = 3; VTY_DECLVAR_CONTEXT(interface, ifp); - struct zebra_if *zi; - struct irdp_interface *irdp; - - zi = ifp->info; - irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); if (irdp->MinAdvertInterval <= (unsigned)atoi(argv[idx_number]->arg)) { irdp->MaxAdvertInterval = atoi(argv[idx_number]->arg); @@ -507,11 +525,7 @@ DEFUN (ip_irdp_preference, { int idx_number = 3; VTY_DECLVAR_CONTEXT(interface, ifp); - struct zebra_if *zi; - struct irdp_interface *irdp; - - zi = ifp->info; - irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); irdp->Preference = atoi(argv[idx_number]->arg); return CMD_SUCCESS; @@ -530,17 +544,13 @@ DEFUN (ip_irdp_address_preference, int idx_ipv4 = 3; int idx_number = 5; VTY_DECLVAR_CONTEXT(interface, ifp); + struct irdp_interface *irdp = irdp_if_get(ifp); struct listnode *node; struct in_addr ip; int pref; int ret; - struct zebra_if *zi; - struct irdp_interface *irdp; struct Adv *adv; - zi = ifp->info; - irdp = &zi->irdp; - ret = inet_aton(argv[idx_ipv4]->arg, &ip); if (!ret) return CMD_WARNING_CONFIG_FAILED; @@ -572,16 +582,12 @@ DEFUN (no_ip_irdp_address_preference, { int idx_ipv4 = 4; VTY_DECLVAR_CONTEXT(interface, ifp); + struct irdp_interface *irdp = irdp_if_get(ifp); struct listnode *node, *nnode; struct in_addr ip; int ret; - struct zebra_if *zi; - struct irdp_interface *irdp; struct Adv *adv; - zi = ifp->info; - irdp = &zi->irdp; - ret = inet_aton(argv[idx_ipv4]->arg, &ip); if (!ret) return CMD_WARNING_CONFIG_FAILED; @@ -605,11 +611,7 @@ DEFUN (ip_irdp_debug_messages, "Enable debugging for IRDP messages\n") { VTY_DECLVAR_CONTEXT(interface, ifp); - struct zebra_if *zi; - struct irdp_interface *irdp; - - zi = ifp->info; - irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); irdp->flags |= IF_DEBUG_MESSAGES; @@ -625,11 +627,7 @@ DEFUN (ip_irdp_debug_misc, "Enable debugging for miscellaneous IRDP events\n") { VTY_DECLVAR_CONTEXT(interface, ifp); - struct zebra_if *zi; - struct irdp_interface *irdp; - - zi = ifp->info; - irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); irdp->flags |= IF_DEBUG_MISC; @@ -645,11 +643,7 @@ DEFUN (ip_irdp_debug_packet, "Enable debugging for IRDP packets\n") { VTY_DECLVAR_CONTEXT(interface, ifp); - struct zebra_if *zi; - struct irdp_interface *irdp; - - zi = ifp->info; - irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); irdp->flags |= IF_DEBUG_PACKET; @@ -666,11 +660,7 @@ DEFUN (ip_irdp_debug_disable, "Disable debugging for all IRDP events\n") { VTY_DECLVAR_CONTEXT(interface, ifp); - struct zebra_if *zi; - struct irdp_interface *irdp; - - zi = ifp->info; - irdp = &zi->irdp; + struct irdp_interface *irdp = irdp_if_get(ifp); irdp->flags &= ~IF_DEBUG_PACKET; irdp->flags &= ~IF_DEBUG_MESSAGES; @@ -682,6 +672,7 @@ DEFUN (ip_irdp_debug_disable, void irdp_if_init() { hook_register(zebra_if_config_wr, irdp_config_write); + hook_register(if_del, irdp_if_delete); install_element(INTERFACE_NODE, &ip_irdp_broadcast_cmd); install_element(INTERFACE_NODE, &ip_irdp_multicast_cmd); diff --git a/zebra/irdp_main.c b/zebra/irdp_main.c index e463608af1..73c6d8141a 100644 --- a/zebra/irdp_main.c +++ b/zebra/irdp_main.c @@ -52,6 +52,7 @@ #include "zclient.h" #include "thread.h" #include "privs.h" +#include "libfrr.h" #include "zebra/interface.h" #include "zebra/rtadv.h" #include "zebra/rib.h" @@ -143,7 +144,7 @@ static int make_advertisement_packet(struct interface *ifp, struct prefix *p, struct stream *s) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; int size; int pref; u_int16_t checksum; @@ -175,11 +176,13 @@ static int make_advertisement_packet(struct interface *ifp, struct prefix *p, static void irdp_send(struct interface *ifp, struct prefix *p, struct stream *s) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; char buf[PREFIX_STRLEN]; u_int32_t dst; u_int32_t ttl = 1; + if (!irdp) + return; if (!(ifp->flags & IFF_UP)) return; @@ -211,11 +214,14 @@ int irdp_send_thread(struct thread *t_advert) u_int32_t timer, tmp; struct interface *ifp = THREAD_ARG(t_advert); struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; struct prefix *p; struct listnode *node, *nnode; struct connected *ifc; + if (!irdp) + return 0; + irdp->flags &= ~IF_SOLICIT; if (ifp->connected) @@ -250,12 +256,15 @@ int irdp_send_thread(struct thread *t_advert) void irdp_advert_off(struct interface *ifp) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; struct listnode *node, *nnode; int i; struct connected *ifc; struct prefix *p; + if (!irdp) + return; + if (irdp->t_advertise) thread_cancel(irdp->t_advertise); irdp->t_advertise = NULL; @@ -279,9 +288,12 @@ void irdp_advert_off(struct interface *ifp) void process_solicit(struct interface *ifp) { struct zebra_if *zi = ifp->info; - struct irdp_interface *irdp = &zi->irdp; + struct irdp_interface *irdp = zi->irdp; u_int32_t timer; + if (!irdp) + return; + /* When SOLICIT is active we reject further incoming solicits this keeps down the answering rate so we don't have think about DoS attacks here. */ @@ -317,7 +329,7 @@ static int irdp_finish(void) if (!zi) continue; - irdp = &zi->irdp; + irdp = zi->irdp; if (!irdp) continue; @@ -326,6 +338,7 @@ static int irdp_finish(void) irdp_advert_off(ifp); } } + return 0; } void irdp_init(void) diff --git a/zebra/irdp_packet.c b/zebra/irdp_packet.c index 3bd093d97b..a64eac2ea4 100644 --- a/zebra/irdp_packet.c +++ b/zebra/irdp_packet.c @@ -84,7 +84,7 @@ static void parse_irdp_packet(char *p, int len, struct interface *ifp) if (!zi) return; - irdp = &zi->irdp; + irdp = zi->irdp; if (!irdp) return; @@ -240,7 +240,7 @@ int irdp_read_raw(struct thread *r) if (!zi) return ret; - irdp = &zi->irdp; + irdp = zi->irdp; if (!irdp) return ret; From 8dc1f7fc888a59c5b723befdd0dbeff5587afa56 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 6 Aug 2017 09:19:14 +0200 Subject: [PATCH 7/8] zebra: irdp: convert into module Signed-off-by: David Lamparter --- configure.ac | 38 ++++++++++++++++++++++---------------- zebra/irdp.h | 1 - zebra/irdp_interface.c | 4 ---- zebra/irdp_main.c | 19 +++++++++++++++---- zebra/irdp_packet.c | 5 ----- zebra/main.c | 3 --- zebra/subdir.am | 13 ++++++++++--- 7 files changed, 47 insertions(+), 36 deletions(-) diff --git a/configure.ac b/configure.ac index 574992342f..710a71f636 100755 --- a/configure.ac +++ b/configure.ac @@ -344,7 +344,7 @@ AC_ARG_ENABLE(shell_access, AC_ARG_ENABLE(rtadv, AS_HELP_STRING([--disable-rtadv], [disable IPV6 router advertisement feature])) AC_ARG_ENABLE(irdp, - AS_HELP_STRING([--enable-irdp], [enable IRDP server support in zebra])) + AS_HELP_STRING([--disable-irdp], [enable IRDP server support in zebra (default if supported)])) AC_ARG_ENABLE(capabilities, AS_HELP_STRING([--disable-capabilities], [disable using POSIX capabilities])) AC_ARG_ENABLE(rusage, @@ -570,10 +570,6 @@ else AC_MSG_RESULT(no) fi -if test "${enable_irdp}" = "yes"; then - AC_DEFINE(HAVE_IRDP,, IRDP ) -fi - if test x"${enable_user}" = x"no"; then enable_user="" else @@ -1479,17 +1475,27 @@ AC_CHECK_MEMBERS([struct sockaddr.sa_len, dnl --------------------------- dnl IRDP/pktinfo/icmphdr checks dnl --------------------------- -AC_CHECK_TYPES([struct in_pktinfo], - [AC_CHECK_TYPES([struct icmphdr], - [if test "${enable_irdp}" != "no"; then - AC_DEFINE(HAVE_IRDP,, IRDP) - fi], - [if test "${enable_irdp}" = "yes"; then - AC_MSG_ERROR(['IRDP requires in_pktinfo at the moment!']) - fi], [FRR_INCLUDES])], - [if test "${enable_irdp}" = "yes"; then - AC_MSG_ERROR(['IRDP requires in_pktinfo at the moment!']) - fi], [FRR_INCLUDES]) + +AC_CHECK_TYPES([struct in_pktinfo], [ + AC_CHECK_TYPES([struct icmphdr], [ + IRDP=true + ], [ + IRDP=false + ], [FRR_INCLUDES]) +], [ + IRDP=false +], [FRR_INCLUDES]) + +case "${enable_irdp}" in +yes) + $IRDP || AC_MSG_ERROR(['IRDP requires in_pktinfo at the moment!']) + ;; +no) + IRDP=false + ;; +esac + +AM_CONDITIONAL(IRDP, $IRDP) dnl ----------------------- dnl checking for IP_PKTINFO diff --git a/zebra/irdp.h b/zebra/irdp.h index f8f7811248..ea190b574d 100644 --- a/zebra/irdp.h +++ b/zebra/irdp.h @@ -138,7 +138,6 @@ struct Adv { int pref; }; -extern void irdp_init(void); extern void irdp_if_init(void); extern int irdp_sock_init(void); extern int irdp_config_write(struct vty *, struct interface *); diff --git a/zebra/irdp_interface.c b/zebra/irdp_interface.c index 3465bdebbf..34c78e2a48 100644 --- a/zebra/irdp_interface.c +++ b/zebra/irdp_interface.c @@ -35,8 +35,6 @@ #include -#ifdef HAVE_IRDP - #include "if.h" #include "vty.h" #include "sockunion.h" @@ -691,5 +689,3 @@ void irdp_if_init() install_element(INTERFACE_NODE, &ip_irdp_debug_packet_cmd); install_element(INTERFACE_NODE, &ip_irdp_debug_disable_cmd); } - -#endif /* HAVE_IRDP */ diff --git a/zebra/irdp_main.c b/zebra/irdp_main.c index 73c6d8141a..9dfa854725 100644 --- a/zebra/irdp_main.c +++ b/zebra/irdp_main.c @@ -35,8 +35,6 @@ #include -#ifdef HAVE_IRDP - #include "if.h" #include "vty.h" #include "sockunion.h" @@ -53,6 +51,7 @@ #include "thread.h" #include "privs.h" #include "libfrr.h" +#include "version.h" #include "zebra/interface.h" #include "zebra/rtadv.h" #include "zebra/rib.h" @@ -341,11 +340,23 @@ static int irdp_finish(void) return 0; } -void irdp_init(void) +static int irdp_init(struct thread_master *master) { irdp_if_init(); hook_register(frr_early_fini, irdp_finish); + return 0; } -#endif /* HAVE_IRDP */ +static int irdp_module_init(void) +{ + hook_register(frr_late_init, irdp_init); + return 0; +} + +FRR_MODULE_SETUP( + .name = "zebra_irdp", + .version = FRR_VERSION, + .description = "zebra IRDP module", + .init = irdp_module_init, +) diff --git a/zebra/irdp_packet.c b/zebra/irdp_packet.c index a64eac2ea4..0832245536 100644 --- a/zebra/irdp_packet.c +++ b/zebra/irdp_packet.c @@ -36,8 +36,6 @@ #include -#ifdef HAVE_IRDP - #include "if.h" #include "vty.h" #include "sockunion.h" @@ -353,6 +351,3 @@ void send_packet(struct interface *ifp, struct stream *s, u_int32_t dst, } /* printf("TX on %s idx %d\n", ifp->name, ifp->ifindex); */ } - - -#endif /* HAVE_IRDP */ diff --git a/zebra/main.c b/zebra/main.c index 72f96add86..bc7276817d 100644 --- a/zebra/main.c +++ b/zebra/main.c @@ -292,9 +292,6 @@ int main(int argc, char **argv) #if defined(HAVE_RTADV) rtadv_cmd_init(); #endif -#ifdef HAVE_IRDP - irdp_init(); -#endif /* PTM socket */ #ifdef ZEBRA_PTM_SUPPORT zebra_ptm_init(); diff --git a/zebra/subdir.am b/zebra/subdir.am index 0391cab9fd..3474823623 100644 --- a/zebra/subdir.am +++ b/zebra/subdir.am @@ -6,6 +6,9 @@ if ZEBRA sbin_PROGRAMS += zebra/zebra dist_examples_DATA += zebra/zebra.conf.sample +if IRDP +module_LTLIBRARIES += zebra/zebra_irdp.la +endif if SNMP module_LTLIBRARIES += zebra/zebra_snmp.la endif @@ -30,9 +33,6 @@ zebra_zebra_SOURCES = \ zebra/ipforward_proc.c \ zebra/ipforward_solaris.c \ zebra/ipforward_sysctl.c \ - zebra/irdp_interface.c \ - zebra/irdp_main.c \ - zebra/irdp_packet.c \ zebra/kernel_netlink.c \ zebra/kernel_socket.c \ zebra/label_manager.c \ @@ -106,6 +106,13 @@ noinst_HEADERS += \ zebra/zserv.h \ # end +zebra_zebra_irdp_la_SOURCES = \ + zebra/irdp_interface.c \ + zebra/irdp_main.c \ + zebra/irdp_packet.c \ + # end +zebra_zebra_irdp_la_LDFLAGS = -avoid-version -module -shared -export-dynamic + zebra_zebra_snmp_la_SOURCES = zebra/zebra_snmp.c zebra_zebra_snmp_la_CFLAGS = $(WERROR) $(SNMP_CFLAGS) zebra_zebra_snmp_la_LDFLAGS = -avoid-version -module -shared -export-dynamic From 0ed9196b0c196ad1b240a77709d1452a2fa42c80 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 15 Aug 2017 14:19:16 +0200 Subject: [PATCH 8/8] redhat: ship IRDP module No point in configuring IRDP (it's always available on Linux), just ship the module and let the user decide whether to enable it by way of module loading. Signed-off-by: David Lamparter --- redhat/frr.spec.in | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/redhat/frr.spec.in b/redhat/frr.spec.in index 12cdcf04fe..cc0632b562 100644 --- a/redhat/frr.spec.in +++ b/redhat/frr.spec.in @@ -13,7 +13,6 @@ %{!?with_pam: %global with_pam 0 } %{!?with_ospfclient: %global with_ospfclient 1 } %{!?with_ospfapi: %global with_ospfapi 1 } -%{!?with_irdp: %global with_irdp 1 } %{!?with_rtadv: %global with_rtadv 1 } %{!?with_ldpd: %global with_ldpd 1 } %{!?with_nhrpd: %global with_nhrpd 1 } @@ -221,6 +220,7 @@ developing OSPF-API and frr applications. --libexecdir=%{_libexecdir} \ --localstatedir=%{_localstatedir} \ --disable-werror \ + --enable-irdp \ %if !%{with_shared} --disable-shared \ %endif @@ -238,11 +238,6 @@ developing OSPF-API and frr applications. %else --enable-ospfapi=no \ %endif -%if %{with_irdp} - --enable-irdp=yes \ -%else - --enable-irdp=no \ -%endif %if %{with_rtadv} --enable-rtadv=yes \ %else @@ -325,6 +320,9 @@ rm -rf %{buildroot}/usr/share/info/dir # Remove debian init script if it was installed rm -f %{buildroot}%{_sbindir}/frr +# kill bogus libtool files for modules +rm -f %{buildroot}%{_libdir}/frr/modules/*.la + # install /etc sources %if "%{initsystem}" == "systemd" mkdir -p %{buildroot}%{_unitdir} @@ -554,6 +552,7 @@ rm -rf %{buildroot} %{_libdir}/lib*.so.0 %attr(755,root,root) %{_libdir}/lib*.so.0.* %endif +%attr(755,root,root) %{_libdir}/frr/modules/zebra_irdp.so %{_bindir}/* %config(noreplace) /etc/frr/[!v]*.conf* %config(noreplace) %attr(750,%frr_user,%frr_user) /etc/frr/daemons