diff options
Diffstat (limited to 'doc/developer')
| -rw-r--r-- | doc/developer/building-frr-for-ubuntu1804.rst | 4 | ||||
| -rw-r--r-- | doc/developer/building-frr-for-ubuntu2004.rst | 4 | ||||
| -rw-r--r-- | doc/developer/cli.rst | 171 | ||||
| -rw-r--r-- | doc/developer/grpc.rst | 1 | ||||
| -rw-r--r-- | doc/developer/logging.rst | 4 | ||||
| -rw-r--r-- | doc/developer/topotests-jsontopo.rst | 158 | ||||
| -rw-r--r-- | doc/developer/topotests.rst | 601 | ||||
| -rw-r--r-- | doc/developer/workflow.rst | 47 |
8 files changed, 685 insertions, 305 deletions
diff --git a/doc/developer/building-frr-for-ubuntu1804.rst b/doc/developer/building-frr-for-ubuntu1804.rst index 39a17fc01c..3e8c6c0d0b 100644 --- a/doc/developer/building-frr-for-ubuntu1804.rst +++ b/doc/developer/building-frr-for-ubuntu1804.rst @@ -12,8 +12,8 @@ Installing Dependencies sudo apt update sudo apt-get install \ git autoconf automake libtool make libreadline-dev texinfo \ - pkg-config libpam0g-dev libjson-c-dev bison flex python3-pytest \ - libc-ares-dev python3-dev python-ipaddress python3-sphinx \ + pkg-config libpam0g-dev libjson-c-dev bison flex \ + libc-ares-dev python3-dev python3-sphinx \ install-info build-essential libsnmp-dev perl libcap-dev \ libelf-dev diff --git a/doc/developer/building-frr-for-ubuntu2004.rst b/doc/developer/building-frr-for-ubuntu2004.rst index 92ddead4a5..28e7ca6518 100644 --- a/doc/developer/building-frr-for-ubuntu2004.rst +++ b/doc/developer/building-frr-for-ubuntu2004.rst @@ -12,8 +12,8 @@ Installing Dependencies sudo apt update sudo apt-get install \ git autoconf automake libtool make libreadline-dev texinfo \ - pkg-config libpam0g-dev libjson-c-dev bison flex python3-pytest \ - libc-ares-dev python3-dev python-ipaddress python3-sphinx \ + pkg-config libpam0g-dev libjson-c-dev bison flex \ + libc-ares-dev python3-dev python3-sphinx \ install-info build-essential libsnmp-dev perl \ libcap-dev python2 libelf-dev diff --git a/doc/developer/cli.rst b/doc/developer/cli.rst index edabe61d92..9254eb4739 100644 --- a/doc/developer/cli.rst +++ b/doc/developer/cli.rst @@ -139,6 +139,7 @@ by the parser. selector: "<" `selector_seq_seq` ">" `varname_token` : "{" `selector_seq_seq` "}" `varname_token` : "[" `selector_seq_seq` "]" `varname_token` + : "![" `selector_seq_seq` "]" `varname_token` selector_seq_seq: `selector_seq_seq` "|" `selector_token_seq` : `selector_token_seq` selector_token_seq: `selector_token_seq` `selector_token` @@ -218,6 +219,10 @@ one-or-more selection and repetition. provide mutual exclusion. User input matches at most one option. - ``[square brackets]`` -- Contains sequences of tokens that can be omitted. ``[<a|b>]`` can be shortened to ``[a|b]``. +- ``![exclamation square brackets]`` -- same as ``[square brackets]``, but + only allow skipping the contents if the command input starts with ``no``. + (For cases where the positive command needs a parameter, but the parameter + is optional for the negative case.) - ``{curly|braces}`` -- similar to angle brackets, but instead of mutual exclusion, curly braces indicate that one or more of the pipe-separated sequences may be provided in any order. @@ -767,6 +772,172 @@ User input: ``ip`` partially matches ``ipv6`` but exactly matches ``ip``, so ``ip`` will win. +Adding a CLI Node +----------------- + +To add a new CLI node, you should: + +- define a new numerical node constant +- define a node structure in the relevant daemon +- call ``install_node()`` in the relevant daemon +- define and install the new node in vtysh +- define corresponding node entry commands in daemon and vtysh +- add a new entry to the ``ctx_keywords`` dictionary in ``tools/frr-reload.py`` + +Defining the numerical node constant +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Add your new node value to the enum before ``NODE_TYPE_MAX`` in +``lib/command.h``: + +.. code-block:: c + + enum node_type { + AUTH_NODE, // Authentication mode of vty interface. + VIEW_NODE, // View node. Default mode of vty interface. + [...] + MY_NEW_NODE, + NODE_TYPE_MAX, // maximum + }; + +Defining a node structure +^^^^^^^^^^^^^^^^^^^^^^^^^ +In your daemon-specific code where you define your new commands that +attach to the new node, add a node definition: + +.. code-block:: c + + static struct cmd_node my_new_node = { + .name = "my new node name", + .node = MY_NEW_NODE, // enum node_type lib/command.h + .parent_node = CONFIG_NODE, + .prompt = "%s(my-new-node-prompt)# ", + .config_write = my_new_node_config_write, + }; + +You will need to define ``my_new_node_config_write(struct vty \*vty)`` +(or omit this field if you have no relevant configuration to save). + +Calling ``install_node()`` +^^^^^^^^^^^^^^^^^^^^^^^^^^ +In the daemon's initialization function, before installing your new commands +with ``install_element()``, add a call ``install_node(&my_new_node)``. + +Defining and installing the new node in vtysh +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +The build tools automatically collect command definitions for vtysh. +However, new nodes must be coded in vtysh specifically. + +In ``vtysh/vtysh.c``, define a stripped-down node structure and +call ``install_node()``: + +.. code-block:: c + + static struct cmd_node my_new_node = { + .name = "my new node name", + .node = MY_NEW_NODE, /* enum node_type lib/command.h */ + .parent_node = CONFIG_NODE, + .prompt = "%s(my-new-node-prompt)# ", + }; + [...] + void vtysh_init_vty(void) + { + [...] + install_node(&my_new_node) + [...] + } + +Defining corresponding node entry commands in daemon and vtysh +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +The command that descends into the new node is typically programmed +with ``VTY_PUSH_CONTEXT`` or equivalent in the daemon's CLI handler function. +(If the CLI has been updated to use the new northbound architecture, +``VTY_PUSH_XPATH`` is used instead.) + +In vtysh, you must implement a corresponding node change so that vtysh +tracks the daemon's movement through the node tree. + +Although the build tools typically scan daemon code for CLI definitions +to replicate their parsing in vtysh, the node-descent function in the +daemon must be blocked from this replication so that a hand-coded +skeleton can be written in ``vtysh.c``. + +Accordingly, use one of the ``*_NOSH`` macros such as ``DEFUN_NOSH``, +``DEFPY_NOSH``, or ``DEFUN_YANG_NOSH`` for the daemon's node-descent +CLI definition, and use ``DEFUNSH`` in ``vtysh.c`` for the vtysh equivalent. + +.. seealso:: :ref:`vtysh-special-defuns` + +Examples: + +``zebra_whatever.c`` + +.. code-block:: c + + DEFPY_NOSH(my_new_node, + my_new_node_cmd, + "my-new-node foo", + "New Thing\n" + "A foo\n") + { + [...] + VTY_PUSH_CONTEXT(MY_NEW_NODE, bar); + [...] + } + + +``ripd_whatever.c`` + +.. code-block:: c + + DEFPY_YANG_NOSH(my_new_node, + my_new_node_cmd, + "my-new-node foo", + "New Thing\n" + "A foo\n") + { + [...] + VTY_PUSH_XPATH(MY_NEW_NODE, xbar); + [...] + } + + +``vtysh.c`` + +.. code-block:: c + + DEFUNSH(VTYSH_ZEBRA, my_new_node, + my_new_node_cmd, + "my-new-node foo", + "New Thing\n" + "A foo\n") + { + vty->node = MY_NEW_NODE; + return CMD_SUCCESS; + } + [...] + install_element(CONFIG_NODE, &my_new_node_cmd); + + +Adding a new entry to the ``ctx_keywords`` dictionary +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +In file ``tools/frr-reload.py``, the ``ctx_keywords`` dictionary +describes the various node relationships. +Add a new node entry at the appropriate level in this dictionary. + +.. code-block:: python + + ctx_keywords = { + [...] + "key chain ": { + "key ": {} + }, + [...] + "my-new-node": {}, + [...] + } + + + Inspection & Debugging ---------------------- diff --git a/doc/developer/grpc.rst b/doc/developer/grpc.rst index cb164bdabf..4e81adf8b2 100644 --- a/doc/developer/grpc.rst +++ b/doc/developer/grpc.rst @@ -42,6 +42,7 @@ Generating C++ FRR Bindings Generating FRR northbound bindings for C++ example: :: + # Install gRPC (e.g., on Ubuntu 20.04) sudo apt-get install libgrpc++-dev libgrpc-dev diff --git a/doc/developer/logging.rst b/doc/developer/logging.rst index b827afd6cc..681fc1173c 100644 --- a/doc/developer/logging.rst +++ b/doc/developer/logging.rst @@ -191,6 +191,10 @@ Networking data types ``%pNHs``: :frrfmtout:`1.2.3.4 if 15` — same as :c:func:`nexthop2str()` + ``%pNHcg``: :frrfmtout:`1.2.3.4` — compact gateway only + + ``%pNHci``: :frrfmtout:`eth0` — compact interface only + .. frrfmt:: %pBD (struct bgp_dest *) :frrfmtout:`fe80::1234/64` diff --git a/doc/developer/topotests-jsontopo.rst b/doc/developer/topotests-jsontopo.rst index 07f1f05114..e2cc72cc56 100644 --- a/doc/developer/topotests-jsontopo.rst +++ b/doc/developer/topotests-jsontopo.rst @@ -23,19 +23,18 @@ On top of current topotests framework following enhancements are done: Logging of test case executions ------------------------------- -* The user can enable logging of testcases execution messages into log file by - adding ``frrtest_log_dir = /tmp/topotests/`` in :file:`pytest.ini`. -* Router's current configuration can be displyed on console or sent to logs by - adding ``show_router_config = True`` in :file:`pytest.ini`. +* The execution log for each test is saved in the test specific directory create + under `/tmp/topotests` (e.g., + `/tmp/topotests/<testdirname.testfilename>/exec.log`) -Log file name will be displayed when we start execution: +* Additionally all test logs are captured in the `topotest.xml` results file. + This file will be saved in `/tmp/topotests/topotests.xml`. In order to extract + the logs for a particular test one can use the `analyze.py` utility found in + the topotests base directory. -.. code-block:: console - - root@test:# python ./test_topo_json_single_link.py - - Logs will be sent to logfile: - /tmp/topotests/test_topo_json_single_link_11:57:01.353797 +* Router's current configuration, as it is changed during the test, can be + displayed on console or sent to logs by adding ``show_router_config = True`` in + :file:`pytest.ini`. Note: directory "/tmp/topotests/" is created by topotests by default, making use of same directory to save execution logs. @@ -51,18 +50,18 @@ topology test. This is the recommended test writing routine: -* Create a json file , which will have routers and protocol configurations -* Create topology from json -* Create configuration from json -* Write the tests +* Create a json file which will have routers and protocol configurations +* Write and debug the tests * Format the new code using `black <https://github.com/psf/black>`_ * Create a Pull Request .. Note:: - BGP tests MUST use generous convergence timeouts - you must ensure - that any test involving BGP uses a convergence timeout of at least - 130 seconds. + BGP tests MUST use generous convergence timeouts - you must ensure that any + test involving BGP uses a convergence timeout that is proportional to the + configured BGP timers. If the timers are not reduced from their defaults this + means 130 seconds; however, it is highly recommended that timers be reduced + from the default values unless the test requires they not be. File Hierarchy ^^^^^^^^^^^^^^ @@ -72,21 +71,17 @@ repository hierarchy looks like this: .. code-block:: console - $ cd path/to/topotests + $ cd frr/tests/topotests $ find ./* ... - ./example-topojson-test # the basic example test topology-1 - ./example-topojson-test/test_example_topojson.json # input json file, having - topology, interfaces, bgp and other configuration - ./example-topojson-test/test_example_topojson.py # test script to write and - execute testcases + ./example_test/ + ./example_test/test_template_json.json # input json file, having topology, interfaces, bgp and other configuration + ./example_test/test_template_json.py # test script to write and execute testcases ... ./lib # shared test/topology functions - ./lib/topojson.py # library to create topology and configurations dynamically - from json file - ./lib/common_config.py # library to create protocol's common configurations ex- - static_routes, prefix_lists, route_maps etc. - ./lib/bgp.py # library to create only bgp configurations + ./lib/topojson.py # library to create topology and configurations dynamically from json file + ./lib/common_config.py # library to create protocol's common configurations ex- static_routes, prefix_lists, route_maps etc. + ./lib/bgp.py # library to create and test bgp configurations Defining the Topology and initial configuration in JSON file ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -370,39 +365,32 @@ Optional keywords/options in JSON: Building topology and configurations """""""""""""""""""""""""""""""""""" -Topology and initial configuration will be created in setup_module(). Following -is the sample code:: +Topology and initial configuration as well as teardown are invoked through the +use of a pytest fixture:: - class TemplateTopo(Topo): - def build(self, *_args, **_opts): - "Build function" - tgen = get_topogen(self) - # Building topology from json file - build_topo_from_json(tgen, topo) + from lib import fixtures - def setup_module(mod): - tgen = Topogen(TemplateTopo, mod.__name__) + tgen = pytest.fixture(fixtures.tgen_json, scope="module") - # Starting topology, create tmp files which are loaded to routers - # to start deamons and then start routers - start_topology(tgen) - # Creating configuration from JSON - build_config_from_json(tgen, topo) + # tgen is defined above + # topo is a fixture defined in ../conftest.py and automatically available + def test_bgp_convergence(tgen, topo): + bgp_convergence = bgp.verify_bgp_convergence(tgen, topo) + assert bgp_convergence - def teardown_module(mod): - tgen = get_topogen() +The `fixtures.topo_json` function calls `topojson.setup_module_from_json()` to +create and return a new `topogen.Topogen()` object using the JSON config file +with the same base filename as the test (i.e., `test_file.py` -> +`test_file.json`). Additionally, the fixture calls `tgen.stop_topology()` after +all the tests have run to cleanup. The function is only invoked once per +file/module (scope="module"), but the resulting object is passed to each +function that has `tgen` as an argument. - # Stop toplogy and Remove tmp files - stop_topology(tgen) +For more info on the powerful pytest fixtures feature please see `FIXTURES`_. - -* Note: Topology will be created in setup module but routers will not be - started until we load zebra.conf and bgpd.conf to routers. For all routers - dirs will be created in /tmp/topotests/<test_folder_name>/<router_name> - zebra.conf and bgpd.conf empty files will be created and laoded to routers. - All folder and files are deleted in teardown module.. +.. _FIXTURES: https://docs.pytest.org/en/6.2.x/fixture.html Creating configuration files """""""""""""""""""""""""""" @@ -412,10 +400,12 @@ configurations are like, static routes, prefixlists and route maps etc configs, these configs can be used by any other protocols as it is. BGP config will be specific to BGP protocol testing. -* JSON file is passed to API build_config_from_json(), which looks for - configuration tags in JSON file. -* If tag is found in JSON, configuration is created as per input and written - to file frr_json.conf +* json file is passed to API Topogen() which saves the JSON object in + `self.json_topo` +* The Topogen object is then passed to API build_config_from_json(), which looks + for configuration tags in new JSON object. +* If tag is found in the JSON object, configuration is created as per input and + written to file frr_json.conf * Once JSON parsing is over, frr_json.conf is loaded onto respective router. Config loading is done using 'vtysh -f <file>'. Initial config at this point is also saved frr_json_initial.conf. This file can be used to reset @@ -428,49 +418,37 @@ Writing Tests """"""""""""" Test topologies should always be bootstrapped from the -example-test/test_example.py, because it contains important boilerplate code -that can't be avoided, like: - -imports: os, sys, pytest, topotest/topogen and mininet topology class - -The global variable CWD (Current Working directory): which is most likely going -to be used to reference the routers configuration file location +`example_test/test_template_json.py` when possible in order to take advantage of +the most recent infrastructure support code. Example: -* The topology class that inherits from Mininet Topo class; - - .. code-block:: python - - class TemplateTopo(Topo): - def build(self, *_args, **_opts): - tgen = get_topogen(self) - # topology build code - - -* pytest setup_module() and teardown_module() to start the topology: +* Define a module scoped fixture to setup/teardown and supply the tests with the + `Topogen` object. - .. code-block:: python +.. code-block:: python - def setup_module(_m): - tgen = Topogen(TemplateTopo) + import pytest + from lib import fixtures - # Starting topology, create tmp files which are loaded to routers - # to start deamons and then start routers - start_topology(tgen, CWD) + tgen = pytest.fixture(fixtures.tgen_json, scope="module") - def teardown_module(_m): - tgen = get_topogen() - # Stop toplogy and Remove tmp files - stop_topology(tgen, CWD) +* Define test functions using pytest fixtures +.. code-block:: python -* ``__main__`` initialization code (to support running the script directly) + from lib import bgp - .. code-block:: python + # tgen is defined above + # topo is a global available fixture defined in ../conftest.py + def test_bgp_convergence(tgen, topo): + "Test for BGP convergence." - if **name** == '\ **main**\ ': - sys.exit(pytest.main(["-s"])) + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + bgp_convergence = bgp.verify_bgp_convergence(tgen, topo) + assert bgp_convergence diff --git a/doc/developer/topotests.rst b/doc/developer/topotests.rst index 18317cd33c..b4f6ec521c 100644 --- a/doc/developer/topotests.rst +++ b/doc/developer/topotests.rst @@ -3,32 +3,37 @@ Topotests ========= -Topotests is a suite of topology tests for FRR built on top of Mininet. +Topotests is a suite of topology tests for FRR built on top of micronet. Installation and Setup ---------------------- -Only tested with Ubuntu 16.04 and Ubuntu 18.04 (which uses Mininet 2.2.x). +Topotests run under python3. Additionally, for ExaBGP (which is used in some of +the BGP tests) an older python2 version must be installed. + +Tested with Ubuntu 20.04 and Ubuntu 18.04 and Debian 11. Instructions are the same for all setups (i.e. ExaBGP is only used for BGP tests). -Installing Mininet Infrastructure -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Installing Topotest Requirements +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ .. code:: shell - apt-get install mininet - apt-get install python-pip - apt-get install iproute - apt-get install iperf - pip install ipaddr - pip install "pytest<5" - pip install "scapy>=2.4.2" - pip install exabgp==3.4.17 (Newer 4.0 version of exabgp is not yet - supported) + apt-get install iproute2 + apt-get install net-tools + apt-get install python3-pip + python3 -m pip install wheel + python3 -m pip install 'pytest>=6.2.4' + python3 -m pip install 'pytest-xdist>=2.3.0' + python3 -m pip install 'scapy>=2.4.5' + python3 -m pip install xmltodict + # Use python2 pip to install older ExaBGP + python2 -m pip install 'exabgp<4.0.0' useradd -d /var/run/exabgp/ -s /bin/false exabgp + Enable Coredumps """""""""""""""" @@ -125,20 +130,155 @@ And create ``frr`` user and ``frrvty`` group as follows: Executing Tests --------------- -Execute all tests with output to console -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Configure your sudo environment +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Topotests must be run as root. Normally this will be accomplished through the +use of the ``sudo`` command. In order for topotests to be able to open new +windows (either XTerm or byobu/screen/tmux windows) certain environment +variables must be passed through the sudo command. One way to do this is to +specify the :option:`-E` flag to ``sudo``. This will carry over most if not all +your environment variables include ``PATH``. For example: + +.. code:: shell + + sudo -E python3 -m pytest -s -v + +If you do not wish to use :option:`-E` (e.g., to avoid ``sudo`` inheriting +``PATH``) you can modify your `/etc/sudoers` config file to specifically pass +the environment variables required by topotests. Add the following commands to +your ``/etc/sudoers`` config file. + +.. code:: shell + + Defaults env_keep="TMUX" + Defaults env_keep+="TMUX_PANE" + Defaults env_keep+="STY" + Defaults env_keep+="DISPLAY" + +If there was already an ``env_keep`` configuration there be sure to use the +``+=`` rather than ``=`` on the first line above as well. + + +Execute all tests in distributed test mode +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ .. code:: shell - py.test -s -v --tb=no + sudo -E pytest -s -v -nauto --dist=loadfile The above command must be executed from inside the topotests directory. All test\_\* scripts in subdirectories are detected and executed (unless -disabled in ``pytest.ini`` file). +disabled in ``pytest.ini`` file). Pytest will execute up to N tests in parallel +where N is based on the number of cores on the host. + +Analyze Test Results (``analyze.py``) +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +By default router and execution logs are saved in ``/tmp/topotests`` and an XML +results file is saved in ``/tmp/topotests.xml``. An analysis tool ``analyze.py`` +is provided to archive and analyze these results after the run completes. + +After the test run completes one should pick an archive directory to store the +results in and pass this value to ``analyze.py``. On first execution the results +are copied to that directory from ``/tmp``, and subsequent runs use that +directory for analyzing the results. Below is an example of this which also +shows the default behavior which is to display all failed and errored tests in +the run. + +.. code:: shell + + ~/frr/tests/topotests# ./analyze.py -Ar run-save + bgp_multiview_topo1/test_bgp_multiview_topo1.py::test_bgp_converge + ospf_basic_functionality/test_ospf_lan.py::test_ospf_lan_tc1_p0 + bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2.py::test_BGP_GR_10_p2 + bgp_multiview_topo1/test_bgp_multiview_topo1.py::test_bgp_routingTable + +Here we see that 4 tests have failed. We an dig deeper by displaying the +captured logs and errors. First let's redisplay the results enumerated by adding +the :option:`-E` flag + +.. code:: shell + + ~/frr/tests/topotests# ./analyze.py -Ar run-save -E + 0 bgp_multiview_topo1/test_bgp_multiview_topo1.py::test_bgp_converge + 1 ospf_basic_functionality/test_ospf_lan.py::test_ospf_lan_tc1_p0 + 2 bgp_gr_functionality_topo2/test_bgp_gr_functionality_topo2.py::test_BGP_GR_10_p2 + 3 bgp_multiview_topo1/test_bgp_multiview_topo1.py::test_bgp_routingTable + +Now to look at the error message for a failed test we use ``-T N`` where N is +the number of the test we are interested in along with ``--errmsg`` option. + +.. code:: shell + + ~/frr/tests/topotests# ./analyze.py -Ar run-save -T0 --errmsg + bgp_multiview_topo1/test_bgp_multiview_topo1.py::test_bgp_converge: AssertionError: BGP did not converge: + + IPv4 Unicast Summary (VIEW 1): + BGP router identifier 172.30.1.1, local AS number 100 vrf-id -1 + BGP table version 1 + RIB entries 1, using 184 bytes of memory + Peers 3, using 2169 KiB of memory + + Neighbor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd PfxSnt Desc + 172.16.1.1 4 65001 0 0 0 0 0 never Connect 0 N/A + 172.16.1.2 4 65002 0 0 0 0 0 never Connect 0 N/A + 172.16.1.5 4 65005 0 0 0 0 0 never Connect 0 N/A + + Total number of neighbors 3 + + assert False + +Now to look at the full text of the error for a failed test we use ``-T N`` +where N is the number of the test we are interested in along with ``--errtext`` +option. + +.. code:: shell + + ~/frr/tests/topotests# ./analyze.py -Ar run-save -T0 --errtext + bgp_multiview_topo1/test_bgp_multiview_topo1.py::test_bgp_converge: def test_bgp_converge(): + "Check for BGP converged on all peers and BGP views" + + global fatal_error + global net + [...] + else: + # Bail out with error if a router fails to converge + bgpStatus = net["r%s" % i].cmd('vtysh -c "show ip bgp view %s summary"' % view) + > assert False, "BGP did not converge:\n%s" % bgpStatus + E AssertionError: BGP did not converge: + E + E IPv4 Unicast Summary (VIEW 1): + E BGP router identifier 172.30.1.1, local AS number 100 vrf-id -1 + [...] + E Neighbor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd PfxSnt Desc + E 172.16.1.1 4 65001 0 0 0 0 0 never Connect 0 N/A + E 172.16.1.2 4 65002 0 0 0 0 0 never Connect 0 N/A + [...] + +To look at the full capture for a test including the stdout and stderr which +includes full debug logs, just use the ``-T N`` option without the ``--errmsg`` +or ``--errtext`` options. + +.. code:: shell + + ~/frr/tests/topotests# ./analyze.py -Ar run-save -T0 + @classname: bgp_multiview_topo1.test_bgp_multiview_topo1 + @name: test_bgp_converge + @time: 141.401 + @message: AssertionError: BGP did not converge: + [...] + system-out: --------------------------------- Captured Log --------------------------------- + 2021-08-09 02:55:06,581 DEBUG: lib.micronet_compat.topo: Topo(unnamed): Creating + 2021-08-09 02:55:06,581 DEBUG: lib.micronet_compat.topo: Topo(unnamed): addHost r1 + [...] + 2021-08-09 02:57:16,932 DEBUG: topolog.r1: LinuxNamespace(r1): cmd_status("['/bin/bash', '-c', 'vtysh -c "show ip bgp view 1 summary" 2> /dev/null | grep ^[0-9] | grep -vP " 11\\s+(\\d+)"']", kwargs: {'encoding': 'utf-8', 'stdout': -1, 'stderr': -2, 'shell': False}) + 2021-08-09 02:57:22,290 DEBUG: topolog.r1: LinuxNamespace(r1): cmd_status("['/bin/bash', '-c', 'vtysh -c "show ip bgp view 1 summary" 2> /dev/null | grep ^[0-9] | grep -vP " 11\\s+(\\d+)"']", kwargs: {'encoding': 'utf-8', 'stdout': -1, 'stderr': -2, 'shell': False}) + 2021-08-09 02:57:27,636 DEBUG: topolog.r1: LinuxNamespace(r1): cmd_status("['/bin/bash', '-c', 'vtysh -c "show ip bgp view 1 summary"']", kwargs: {'encoding': 'utf-8', 'stdout': -1, 'stderr': -2, 'shell': False}) + --------------------------------- Captured Out --------------------------------- + system-err: --------------------------------- Captured Err --------------------------------- -``--tb=no`` disables the python traceback which might be irrelevant unless the -test script itself is debugged. Execute single test ^^^^^^^^^^^^^^^^^^^ @@ -161,9 +301,6 @@ Test will set exit code which can be used with ``git bisect``. For the simulated topology, see the description in the python file. -If you need to clear the mininet setup between tests (if it isn't cleanly -shutdown), then use the ``mn -c`` command to clean up the environment. - StdErr log from daemos after exit ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -235,57 +372,86 @@ and create ``frr`` user and ``frrvty`` group as shown above. Debugging Topotest Failures ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -For the below debugging options which launch programs, if the topotest is run -within screen_ or tmux_, ``gdb``, the shell or ``vtysh`` will be launched using -that windowing program, otherwise mininet's ``xterm`` functionality will be used -to launch the given program. +Install and run tests inside ``tmux`` or ``byobu`` for best results. -If you wish to force the use of ``xterm`` rather than ``tmux`` or ``screen``, or -wish to use ``gnome-terminal`` instead of ``xterm``, set the environment -variable ``FRR_TOPO_TERMINAL`` to either ``xterm`` or ``gnome-terminal``. +``XTerm`` is also fully supported. GNU ``screen`` can be used in most +situations; however, it does not work as well with launching ``vtysh`` or shell +on error. -.. _screen: https://www.gnu.org/software/screen/ -.. _tmux: https://github.com/tmux/tmux/wiki +For the below debugging options which launch programs or CLIs, topotest should +be run within ``tmux`` (or ``screen``)_, as ``gdb``, the shell or ``vtysh`` will +be launched using that windowing program, otherwise ``xterm`` will be attempted +to launch the given programs. -Spawning ``vtysh`` or Shells on Routers -""""""""""""""""""""""""""""""""""""""" +NOTE: you must run the topotest (pytest) such that your DISPLAY, STY or TMUX +environment variables are carried over. You can do this by passing the +:option:`-E` flag to ``sudo`` or you can modify your ``/etc/sudoers`` config to +automatically pass that environment variable through to the ``sudo`` +environment. -Topotest can automatically launch a shell or ``vtysh`` for any or all routers in -a test. This is enabled by specifying 1 of 2 CLI arguments ``--shell`` or -``--vtysh``. Both of these options can be set to a single router value, multiple -comma-seperated values, or ``all``. +.. _screen: https://www.gnu.org/software/screen/ +.. _tmux: https://github.com/tmux/tmux/wiki -When either of these options are specified topotest will pause after each test -to allow for inspection of the router state. +Spawning Debugging CLI, ``vtysh`` or Shells on Routers on Test Failure +"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" -Here's an example of launching ``vtysh`` on routers ``rt1`` and ``rt2``. +One can have a debugging CLI invoked on test failures by specifying the +``--cli-on-error`` CLI option as shown in the example below. .. code:: shell - pytest --vtysh=rt1,rt2 all-protocol-startup - -Spawning Mininet CLI, ``vtysh`` or Shells on Routers on Test Failure -"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""" + sudo -E pytest --cli-on-error all-protocol-startup -Similar to the previous section one can have ``vtysh`` or a shell launched on -routers, but in this case only when a test fails. To launch the given process on -each router after a test failure specify one of ``--shell-on-error`` or -``--vtysh-on-error``. +The debugging CLI can run shell or vtysh commands on any combination of routers +It can also open shells or vtysh in their own windows for any combination of +routers. This is usually the most useful option when debugging failures. Here is +the help command from within a CLI launched on error: +.. code:: shell -Here's an example of having ``vtysh`` launched on test failure. + test_bgp_multiview_topo1/test_bgp_routingTable> help + + Commands: + help :: this help + sh [hosts] <shell-command> :: execute <shell-command> on <host> + term [hosts] :: open shell terminals for hosts + vtysh [hosts] :: open vtysh terminals for hosts + [hosts] <vtysh-command> :: execute vtysh-command on hosts + + test_bgp_multiview_topo1/test_bgp_routingTable> r1 show int br + ------ Host: r1 ------ + Interface Status VRF Addresses + --------- ------ --- --------- + erspan0 down default + gre0 down default + gretap0 down default + lo up default + r1-eth0 up default 172.16.1.254/24 + r1-stub up default 172.20.0.1/28 + + ---------------------- + test_bgp_multiview_topo1/test_bgp_routingTable> + +Additionally, one can have ``vtysh`` or a shell launched on all routers when a +test fails. To launch the given process on each router after a test failure +specify one of ``--shell-on-error`` or ``--vtysh-on-error``. -.. code:: shell +Spawning ``vtysh`` or Shells on Routers +""""""""""""""""""""""""""""""""""""""" - pytest --vtysh-on-error all-protocol-startup +Topotest can automatically launch a shell or ``vtysh`` for any or all routers in +a test. This is enabled by specifying 1 of 2 CLI arguments ``--shell`` or +``--vtysh``. Both of these options can be set to a single router value, multiple +comma-seperated values, or ``all``. +When either of these options are specified topotest will pause after setup and +each test to allow for inspection of the router state. -Additionally, one can have the mininet CLI invoked on test failures by -specifying the ``--mininet-on-error`` CLI option as shown in the example below. +Here's an example of launching ``vtysh`` on routers ``rt1`` and ``rt2``. .. code:: shell - pytest --mininet-on-error all-protocol-startup + sudo -E pytest --vtysh=rt1,rt2 all-protocol-startup Debugging with GDB """""""""""""""""" @@ -306,7 +472,7 @@ Here's an example of launching ``zebra`` and ``bgpd`` inside ``gdb`` on router .. code:: shell - pytest --gdb-routers=r1 \ + sudo -E pytest --gdb-routers=r1 \ --gdb-daemons=bgpd,zebra \ --gdb-breakpoints=nb_config_diff \ all-protocol-startup @@ -323,7 +489,7 @@ memleak detection is enabled. .. code:: shell - pytest --valgrind-memleaks all-protocol-startup + sudo -E pytest --valgrind-memleaks all-protocol-startup .. _topotests_docker: @@ -424,22 +590,22 @@ top level directory of topotest: $ # Change to the top level directory of topotests. $ cd path/to/topotests - $ # Tests must be run as root, since Mininet requires it. - $ sudo pytest + $ # Tests must be run as root, since micronet requires it. + $ sudo -E pytest In order to run a specific test, you can use the following command: .. code:: shell $ # running a specific topology - $ sudo pytest ospf-topo1/ + $ sudo -E pytest ospf-topo1/ $ # or inside the test folder $ cd ospf-topo1 - $ sudo pytest # to run all tests inside the directory - $ sudo pytest test_ospf_topo1.py # to run a specific test + $ sudo -E pytest # to run all tests inside the directory + $ sudo -E pytest test_ospf_topo1.py # to run a specific test $ # or outside the test folder $ cd .. - $ sudo pytest ospf-topo1/test_ospf_topo1.py # to run a specific one + $ sudo -E pytest ospf-topo1/test_ospf_topo1.py # to run a specific one The output of the tested daemons will be available at the temporary folder of your machine: @@ -458,7 +624,7 @@ You can also run memory leak tests to get reports: .. code:: shell $ # Set the environment variable to apply to a specific test... - $ sudo env TOPOTESTS_CHECK_MEMLEAK="/tmp/memleak_report_" pytest ospf-topo1/test_ospf_topo1.py + $ sudo -E env TOPOTESTS_CHECK_MEMLEAK="/tmp/memleak_report_" pytest ospf-topo1/test_ospf_topo1.py $ # ...or apply to all tests adding this line to the configuration file $ echo 'memleak_path = /tmp/memleak_report_' >> pytest.ini $ # You can also use your editor @@ -493,15 +659,16 @@ Some things to keep in mind: - Avoid including unstable data in your test: don't rely on link-local addresses or ifindex values, for example, because these can change from run to run. -- Using sleep is almost never appropriate to wait for some convergence - event as the sole item done. As an example: if the test resets the peers - in BGP, the test should look for the peers reconverging instead of just - sleeping an arbitrary amount of time and continuing on. It is ok to - use sleep in a tight loop with appropriate show commands to ensure that - the protocol reaches the desired state. This should be bounded by - appropriate timeouts for the protocol in question though. See - verify_bgp_convergence as a good example of this. If you are having - troubles figuring out what to look for, please do not be afraid to ask. +- Using sleep is almost never appropriate. As an example: if the test resets the + peers in BGP, the test should look for the peers re-converging instead of just + sleeping an arbitrary amount of time and continuing on. See + ``verify_bgp_convergence`` as a good example of this. In particular look at + it's use of the ``@retry`` decorator. If you are having troubles figuring out + what to look for, please do not be afraid to ask. +- Don't duplicate effort. There exists many protocol utility functions that can + be found in their eponymous module under ``tests/topotests/lib/`` (e.g., + ``ospf.py``) + Topotest File Hierarchy @@ -661,25 +828,32 @@ Here is the template topology described in the previous section in python code: .. code:: py - class TemplateTopo(Topo): - "Test topology builder" - def build(self, *_args, **_opts): - "Build function" - tgen = get_topogen(self) + topodef = { + "s1": "r1" + "s2": ("r1", "r2") + } + +If more specialized topology definitions, or router initialization arguments are +required a build function can be used instead of a dictionary: + +.. code:: py + + def build_topo(tgen): + "Build function" - # Create 2 routers - for routern in range(1, 3): - tgen.add_router('r{}'.format(routern)) + # Create 2 routers + for routern in range(1, 3): + tgen.add_router("r{}".format(routern)) - # Create a switch with just one router connected to it to simulate a - # empty network. - switch = tgen.add_switch('s1') - switch.add_link(tgen.gears['r1']) + # Create a switch with just one router connected to it to simulate a + # empty network. + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1"]) - # Create a connection between r1 and r2 - switch = tgen.add_switch('s2') - switch.add_link(tgen.gears['r1']) - switch.add_link(tgen.gears['r2']) + # Create a connection between r1 and r2 + switch = tgen.add_switch("s2") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r2"]) - Run the topology @@ -689,11 +863,11 @@ that using the following example commands: .. code:: shell $ # Running your bootstraped topology - $ sudo pytest -s --topology-only new-topo/test_new_topo.py + $ sudo -E pytest -s --topology-only new-topo/test_new_topo.py $ # Running the test_template.py topology - $ sudo pytest -s --topology-only example-test/test_template.py + $ sudo -E pytest -s --topology-only example-test/test_template.py $ # Running the ospf_topo1.py topology - $ sudo pytest -s --topology-only ospf-topo1/test_ospf_topo1.py + $ sudo -E pytest -s --topology-only ospf-topo1/test_ospf_topo1.py Parameters explanation: @@ -701,8 +875,8 @@ Parameters explanation: .. option:: -s - Actives input/output capture. This is required by mininet in order to show - the interactive shell. + Actives input/output capture. If this is not specified a new window will be + opened for the interactive CLI, otherwise it will be activated inline. .. option:: --topology-only @@ -713,110 +887,84 @@ output: .. code:: shell - === test session starts === - platform linux2 -- Python 2.7.12, pytest-3.1.2, py-1.4.34, pluggy-0.4.0 - rootdir: /media/sf_src/topotests, inifile: pytest.ini - collected 3 items - - ospf-topo1/test_ospf_topo1.py *** Starting controller - - *** Starting 6 switches - switch1 switch2 switch3 switch4 switch5 switch6 ... - r2: frr zebra started - r2: frr ospfd started - r3: frr zebra started - r3: frr ospfd started - r1: frr zebra started - r1: frr ospfd started - r4: frr zebra started - r4: frr ospfd started - *** Starting CLI: - mininet> - -The last line shows us that we are now using the Mininet CLI (Command Line -Interface), from here you can call your router ``vtysh`` or even bash. + frr/tests/topotests# sudo -E pytest -s --topology-only ospf_topo1/test_ospf_topo1.py + ============================= test session starts ============================== + platform linux -- Python 3.9.2, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 + rootdir: /home/chopps/w/frr/tests/topotests, configfile: pytest.ini + plugins: forked-1.3.0, xdist-2.3.0 + collected 11 items -Here are some commands example: - -.. code:: shell - - mininet> r1 ping 10.0.3.1 - PING 10.0.3.1 (10.0.3.1) 56(84) bytes of data. - 64 bytes from 10.0.3.1: icmp_seq=1 ttl=64 time=0.576 ms - 64 bytes from 10.0.3.1: icmp_seq=2 ttl=64 time=0.083 ms - 64 bytes from 10.0.3.1: icmp_seq=3 ttl=64 time=0.088 ms - ^C - --- 10.0.3.1 ping statistics --- - 3 packets transmitted, 3 received, 0% packet loss, time 1998ms - rtt min/avg/max/mdev = 0.083/0.249/0.576/0.231 ms + [...] + unet> +The last line shows us that we are now using the CLI (Command Line +Interface), from here you can call your router ``vtysh`` or even bash. +Here's the help text: - mininet> r1 ping 10.0.3.3 - PING 10.0.3.3 (10.0.3.3) 56(84) bytes of data. - 64 bytes from 10.0.3.3: icmp_seq=1 ttl=64 time=2.87 ms - 64 bytes from 10.0.3.3: icmp_seq=2 ttl=64 time=0.080 ms - 64 bytes from 10.0.3.3: icmp_seq=3 ttl=64 time=0.091 ms - ^C - --- 10.0.3.3 ping statistics --- - 3 packets transmitted, 3 received, 0% packet loss, time 2003ms - rtt min/avg/max/mdev = 0.080/1.014/2.872/1.313 ms +.. code:: shell + unet> help + Commands: + help :: this help + sh [hosts] <shell-command> :: execute <shell-command> on <host> + term [hosts] :: open shell terminals for hosts + vtysh [hosts] :: open vtysh terminals for hosts + [hosts] <vtysh-command> :: execute vtysh-command on hosts +.. code:: shell - mininet> r3 vtysh +Here are some commands example: - Hello, this is FRRouting (version 3.1-devrzalamena-build). - Copyright 1996-2005 Kunihiro Ishiguro, et al. +.. code:: shell - frr-1# show running-config - Building configuration... - - Current configuration: - ! - frr version 3.1-devrzalamena-build - frr defaults traditional - hostname r3 - no service integrated-vtysh-config - ! - log file zebra.log - ! - log file ospfd.log - ! - interface r3-eth0 - ip address 10.0.3.1/24 - ! - interface r3-eth1 - ip address 10.0.10.1/24 - ! - interface r3-eth2 - ip address 172.16.0.2/24 - ! - router ospf - ospf router-id 10.0.255.3 - redistribute kernel - redistribute connected - redistribute static - network 10.0.3.0/24 area 0 - network 10.0.10.0/24 area 0 - network 172.16.0.0/24 area 1 - ! - line vty - ! - end - frr-1# + unet> sh r1 ping 10.0.3.1 + PING 10.0.3.1 (10.0.3.1) 56(84) bytes of data. + 64 bytes from 10.0.3.1: icmp_seq=1 ttl=64 time=0.576 ms + 64 bytes from 10.0.3.1: icmp_seq=2 ttl=64 time=0.083 ms + 64 bytes from 10.0.3.1: icmp_seq=3 ttl=64 time=0.088 ms + ^C + --- 10.0.3.1 ping statistics --- + 3 packets transmitted, 3 received, 0% packet loss, time 1998ms + rtt min/avg/max/mdev = 0.083/0.249/0.576/0.231 ms + + unet> r1 show run + Building configuration... + + Current configuration: + ! + frr version 8.1-dev-my-manual-build + frr defaults traditional + hostname r1 + log file /tmp/topotests/ospf_topo1.test_ospf_topo1/r1/zebra.log + [...] + end + + unet> show daemons + ------ Host: r1 ------ + zebra ospfd ospf6d staticd + ------- End: r1 ------ + ------ Host: r2 ------ + zebra ospfd ospf6d staticd + ------- End: r2 ------ + ------ Host: r3 ------ + zebra ospfd ospf6d staticd + ------- End: r3 ------ + ------ Host: r4 ------ + zebra ospfd ospf6d staticd + ------- End: r4 ------ After you successfully configured your topology, you can obtain the configuration files (per-daemon) using the following commands: .. code:: shell - mininet> r3 vtysh -d ospfd + unet> sh r3 vtysh -d ospfd Hello, this is FRRouting (version 3.1-devrzalamena-build). Copyright 1996-2005 Kunihiro Ishiguro, et al. - frr-1# show running-config + r1# show running-config Building configuration... Current configuration: @@ -839,59 +987,91 @@ configuration files (per-daemon) using the following commands: line vty ! end - frr-1# + r1# + +You can also login to the node specified by nsenter using bash, etc. +A pid file for each node will be created in the relevant test dir. +You can run scripts inside the node, or use vtysh's <tab> or <?> feature. + +.. code:: shell + + [unet shell] + # cd tests/topotests/srv6_locator + # ./test_srv6_locator.py --topology-only + unet> r1 show segment-routing srv6 locator + Locator: + Name ID Prefix Status + -------------------- ------- ------------------------ ------- + loc1 1 2001:db8:1:1::/64 Up + loc2 2 2001:db8:2:2::/64 Up + + [Another shell] + # nsenter -a -t $(cat /tmp/topotests/srv6_locator.test_srv6_locator/r1.pid) bash --norc + # vtysh + r1# r1 show segment-routing srv6 locator + Locator: + Name ID Prefix Status + -------------------- ------- ------------------------ ------- + loc1 1 2001:db8:1:1::/64 Up + loc2 2 2001:db8:2:2::/64 Up Writing Tests """"""""""""" Test topologies should always be bootstrapped from -:file:`tests/topotests/example-test/test_template.py` because it contains +:file:`tests/topotests/example_test/test_template.py` because it contains important boilerplate code that can't be avoided, like: -- imports: os, sys, pytest, topotest/topogen and mininet topology class -- The global variable CWD (Current Working directory): which is most likely - going to be used to reference the routers configuration file location - Example: .. code:: py - # For all registered routers, load the zebra configuration file - for rname, router in router_list.items(): - router.load_config( - TopoRouter.RD_ZEBRA, - os.path.join(CWD, '{}/zebra.conf'.format(rname)) - ) - # os.path.join() joins the CWD string with arguments adding the necessary - # slashes ('/'). Arguments must not begin with '/'. + # For all routers arrange for: + # - starting zebra using config file from <rtrname>/zebra.conf + # - starting ospfd using an empty config file. + for rname, router in router_list.items(): + router.load_config(TopoRouter.RD_ZEBRA, "zebra.conf") + router.load_config(TopoRouter.RD_OSPF) + -- The topology class that inherits from Mininet Topo class: +- The topology definition or build function .. code:: py - class TemplateTopo(Topo): - def build(self, *_args, **_opts): - tgen = get_topogen(self) + topodef = { + "s1": ("r1", "r2"), + "s2": ("r2", "r3") + } + + def build_topo(tgen): # topology build code + ... -- pytest ``setup_module()`` and ``teardown_module()`` to start the topology +- pytest setup/teardown fixture to start the topology and supply ``tgen`` + argument to tests. .. code:: py - def setup_module(_m): - tgen = Topogen(TemplateTopo) - tgen.start_topology('debug') - def teardown_module(_m): - tgen = get_topogen() - tgen.stop_topology() + @pytest.fixture(scope="module") + def tgen(request): + "Setup/Teardown the environment and provide tgen argument to tests" -- ``__main__`` initialization code (to support running the script directly) + tgen = Topogen(topodef, module.__name__) + # or + tgen = Topogen(build_topo, module.__name__) -.. code:: py + ... + + # Start and configure the router daemons + tgen.start_router() + + # Provide tgen as argument to each test function + yield tgen + + # Teardown after last test runs + tgen.stop_topology() - if __name__ == '__main__': - sys.exit(pytest.main(["-s"])) Requirements: @@ -1042,11 +1222,10 @@ Example of pdb usage: (Pdb) router1 = tgen.gears[router] (Pdb) router1.vtysh_cmd('show ip ospf route') '============ OSPF network routing table ============\r\nN 10.0.1.0/24 [10] area: 0.0.0.0\r\n directly attached to r1-eth0\r\nN 10.0.2.0/24 [20] area: 0.0.0.0\r\n via 10.0.3.3, r1-eth1\r\nN 10.0.3.0/24 [10] area: 0.0.0.0\r\n directly attached to r1-eth1\r\nN 10.0.10.0/24 [20] area: 0.0.0.0\r\n via 10.0.3.1, r1-eth1\r\nN IA 172.16.0.0/24 [20] area: 0.0.0.0\r\n via 10.0.3.1, r1-eth1\r\nN IA 172.16.1.0/24 [30] area: 0.0.0.0\r\n via 10.0.3.1, r1-eth1\r\n\r\n============ OSPF router routing table =============\r\nR 10.0.255.2 [10] area: 0.0.0.0, ASBR\r\n via 10.0.3.3, r1-eth1\r\nR 10.0.255.3 [10] area: 0.0.0.0, ABR, ASBR\r\n via 10.0.3.1, r1-eth1\r\nR 10.0.255.4 IA [20] area: 0.0.0.0, ASBR\r\n via 10.0.3.1, r1-eth1\r\n\r\n============ OSPF external routing table ===========\r\n\r\n\r\n' - (Pdb) tgen.mininet_cli() - *** Starting CLI: - mininet> + (Pdb) tgen.cli() + unet> -To enable more debug messages in other Topogen subsystems (like Mininet), more +To enable more debug messages in other Topogen subsystems, more logging messages can be displayed by modifying the test configuration file ``pytest.ini``: diff --git a/doc/developer/workflow.rst b/doc/developer/workflow.rst index b6fde2b283..2ce5f5d1c8 100644 --- a/doc/developer/workflow.rst +++ b/doc/developer/workflow.rst @@ -637,6 +637,39 @@ well as CERT or MISRA C guidelines may provide useful input on safe C code. However, these rules are not applied as-is; some of them expressly collide with established practice. + +Container implementations +^^^^^^^^^^^^^^^^^^^^^^^^^ + +In particular to gain defensive coding benefits from better compiler type +checks, there is a set of replacement container data structures to be found +in :file:`lib/typesafe.h`. They're documented under :ref:`lists`. + +Unfortunately, the FRR codebase is quite large, and migrating existing code to +use these new structures is a tedious and far-reaching process (even if it +can be automated with coccinelle, the patches would touch whole swaths of code +and create tons of merge conflicts for ongoing work.) Therefore, little +existing code has been migrated. + +However, both **new code and refactors of existing code should use the new +containers**. If there are any reasons this can't be done, please work to +remove these reasons (e.g. by adding necessary features to the new containers) +rather than falling back to the old code. + +In order of likelyhood of removal, these are the old containers: + +- :file:`nhrpd/list.*`, ``hlist_*`` ⇒ ``DECLARE_LIST`` +- :file:`nhrpd/list.*`, ``list_*`` ⇒ ``DECLARE_DLIST`` +- :file:`lib/skiplist.*`, ``skiplist_*`` ⇒ ``DECLARE_SKIPLIST`` +- :file:`lib/*_queue.h` (BSD), ``SLIST_*`` ⇒ ``DECLARE_LIST`` +- :file:`lib/*_queue.h` (BSD), ``LIST_*`` ⇒ ``DECLARE_DLIST`` +- :file:`lib/*_queue.h` (BSD), ``STAILQ_*`` ⇒ ``DECLARE_LIST`` +- :file:`lib/*_queue.h` (BSD), ``TAILQ_*`` ⇒ ``DECLARE_DLIST`` +- :file:`lib/hash.*`, ``hash_*`` ⇒ ``DECLARE_HASH`` +- :file:`lib/linklist.*`, ``list_*`` ⇒ ``DECLARE_DLIST`` +- open-coded linked lists ⇒ ``DECLARE_LIST``/``DECLARE_DLIST`` + + Code Formatting --------------- @@ -1217,6 +1250,20 @@ it possible to use your apis in paths that involve ``const`` objects. If you encounter existing apis that *could* be ``const``, consider including changes in your own pull-request. +Help with specific warnings +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +FRR's configure script enables a whole batch of extra warnings, some of which +may not be obvious in how to fix. Here are some notes on specific warnings: + +* ``-Wstrict-prototypes``: you probably just forgot the ``void`` in a function + declaration with no parameters, i.e. ``static void foo() {...}`` rather than + ``static void foo(void) {...}``. + + Without the ``void``, in C, it's a function with *unspecified* parameters + (and varargs calling convention.) This is a notable difference to C++, where + the ``void`` is optional and an empty parameter list means no parameters. + .. _documentation: |