1 .. _process-and-workflow:
9 FRR is a large project developed by many different groups. This section
10 documents standards for code style & quality, commit messages, pull requests
11 and best practices that all contributors are asked to follow.
13 This chapter is "descriptive/post-factual" in that it documents pratices that
14 are in use; it is not "definitive/pre-factual" in prescribing practices. This
15 means that when a procedure changes, it is agreed upon, then put into practice,
16 and then documented here. If this document doesn't match reality, it's the
17 document that needs to be updated, not reality.
22 The FRR development group maintains multiple mailing lists for use by the
23 community. Italicized lists are private.
25 +----------------------------------+--------------------------------+
27 +==================================+================================+
28 | Development | dev@lists.frrouting.org |
29 +----------------------------------+--------------------------------+
30 | Users & Operators | frog@lists.frrouting.org |
31 +----------------------------------+--------------------------------+
32 | Announcements | announce@lists.frrouting.org |
33 +----------------------------------+--------------------------------+
34 | *Security* | security@lists.frrouting.org |
35 +----------------------------------+--------------------------------+
36 | *Technical Steering Committee* | tsc@lists.frrouting.org |
37 +----------------------------------+--------------------------------+
39 The Development list is used to discuss and document general issues related to
40 project development and governance. The public
41 `Slack instance <https://frrouting.slack.com>`_ and weekly technical meetings
42 provide a higher bandwidth channel for discussions. The results of such
43 discussions must be reflected in updates, as appropriate, to code (i.e.,
44 merges), `GitHub issues`_, and for governance or process changes, updates to
45 the Development list and either this file or information posted at
46 https://frrouting.org/.
48 Development & Release Cycle
49 ===========================
54 .. figure:: ../figures/git_branches.png
57 :alt: Merging Git branches into a central trunk
59 Rough outline of FRR development workflow
61 The master Git for FRR resides on `GitHub`_.
63 There is one main branch for development, ``master``. For each major release
64 (2.0, 3.0 etc) a new release branch is created based on the master. Significant
65 bugfixes should be backported to upcoming and existing release branches no more
66 than 1 year old. As a general rule new features are not backported to release
69 Subsequent point releases based on a major branch are handled with git tags.
73 FRR employs a ``<MAJOR>.<MINOR>.<BUGFIX>`` versioning scheme.
76 Significant new features or multiple minor features. This should mostly
77 cover any kind of disruptive change that is visible or "risky" to operators.
78 New features or protocols do not necessarily trigger this. (This was changed
79 for FRR 7.x after feedback from users that the pace of major version number
80 increments was too high.)
83 General incremental development releases, excluding "major" changes
84 mentioned above. Not necessarily fully backwards compatible, as smaller
85 (but still visible) changes or deprecated feature removals may still happen.
86 However, there shouldn't be any huge "surprises" between minor releases.
89 Fixes for actual bugs and/or security issues. Fully compatible.
91 Releases are scheduled in a 4-month cycle on the first Tuesday each
92 March/July/November. Walking backwards from this date:
94 - 6 weeks earlier, ``master`` is frozen for new features, and feature PRs
95 are considered lowest priority (regardless of when they were opened.)
97 - 4 weeks earlier, the stable branch separates from master (named
98 ``dev/MAJOR.MINOR`` at this point) and tagged as ``base_X.Y``.
99 Master is unfrozen and new features may again proceed.
101 Part of unfreezing master is editing the ``AC_INIT`` statement in
102 :file:`configure.ac` to reflect the new development version that master
103 now refers to. This is accompanied by a ``frr-X.Y-dev`` tag on master,
104 which should always be on the first commit on master *after* the stable
105 branch was forked (even if that is not the edit to ``AC_INIT``; it's more
106 important to have it on the very first commit on master after the fork.)
108 (The :file:`configure.ac` edit and tag push are considered git housekeeping
109 and are pushed directly to ``master``, not through a PR.)
111 Below is the snippet of the commands to use in this step.
113 .. code-block:: console
115 % git remote --verbose
116 upstream git@github.com:frrouting/frr (fetch)
117 upstream git@github.com:frrouting/frr (push)
119 % git checkout master
120 % git pull upstream master
121 % git checkout -b dev/8.2
123 % git push upstream base_8.2
124 % git push upstream dev/8.2
125 % git checkout master
126 % sed -i 's/8.2-dev/8.3-dev/' configure.ac
127 % git add configure.ac
128 % git commit -s -m "build: FRR 8.3 development version"
129 % git tag -a frr-8.3-dev -m "frr-8.3-dev"
130 % git push upstream master
131 % git push upstream frr-8.3-dev
133 In this step, we also have to update package versions to reflect
134 the development version. Versions need to be updated using
135 a standard way of development (Pull Requests) based on master branch.
137 Only change the version number with no other changes. This will produce
138 packages with the a version number that is higher than any previous
139 version. Once the release is done, whatever updates we make to changelog
140 files on the release branch need to be cherry-picked to the master branch.
142 Update essential dates in advance for reference table (below) when
143 the next freeze, dev/X.Y, RC, and release phases are scheduled. This should
144 go in the ``master`` branch.
146 - 2 weeks earlier, a ``frr-X.Y-rc`` release candidate is tagged.
148 .. code-block:: console
150 % git remote --verbose
151 upstream git@github.com:frrouting/frr (fetch)
152 upstream git@github.com:frrouting/frr (push)
154 % git checkout dev/8.2
156 % git push upstream frr-8.2-rc
158 - on release date, the branch is renamed to ``stable/MAJOR.MINOR``.
160 The 2 week window between each of these events should be used to run any and
161 all testing possible for the release in progress. However, the current
162 intention is to stick to the schedule even if known issues remain. This would
163 hopefully occur only after all avenues of fixing issues are exhausted, but to
164 achieve this, an as exhaustive as possible list of issues needs to be available
165 as early as possible, i.e. the first 2-week window.
167 For reference, the expected release schedule according to the above is:
169 +---------+------------+------------+------------+------------+------------+
170 | Release | 2023-03-07 | 2023-07-04 | 2023-10-31 | 2024-02-27 | 2024-06-25 |
171 +---------+------------+------------+------------+------------+------------+
172 | RC | 2023-02-21 | 2023-06-20 | 2023-10-17 | 2024-02-13 | 2024-06-11 |
173 +---------+------------+------------+------------+------------+------------+
174 | dev/X.Y | 2023-02-07 | 2023-06-06 | 2023-10-03 | 2024-01-30 | 2024-05-28 |
175 +---------+------------+------------+------------+------------+------------+
176 | freeze | 2023-01-24 | 2023-05-23 | 2023-09-19 | 2024-01-16 | 2024-05-14 |
177 +---------+------------+------------+------------+------------+------------+
179 Here is the hint on how to get the dates easily:
181 .. code-block:: console
183 ~$ # Last freeze date was 2023-09-19
184 ~$ date +%F --date='2023-09-19 +119 days' # Next freeze date
186 ~$ date +%F --date='2024-01-16 +14 days' # Next dev/X.Y date
188 ~$ date +%F --date='2024-01-30 +14 days' # Next RC date
190 ~$ date +%F --date='2024-02-13 +14 days' # Next Release date
193 Each release is managed by one or more volunteer release managers from the FRR
194 community. These release managers are expected to handle the branch for a period
195 of one year. To spread and distribute this workload, this should be rotated for
196 subsequent releases. The release managers are currently assumed/expected to
197 run a release management meeting during the weeks listed above. Barring other
198 constraints, this would be scheduled before the regular weekly FRR community
199 call such that important items can be carried over into that call.
201 Bugfixes are applied to the two most recent releases. It is expected that
202 each bugfix backported should include some reasoning for its inclusion
203 as well as receiving approval by the release managers for that release before
204 accepted into the release branch. This does not necessarily preclude backporting of
205 bug fixes to older than the two most recent releases.
207 Security fixes are backported to all releases less than or equal to at least one
208 year old. Security fixes may also be backported to older releases depending on
211 For detailed instructions on how to produce an FRR release, refer to
212 :ref:`frr-release-procedure`.
215 Long term support branches ( LTS )
216 -----------------------------------------
218 This kind of branch is not yet officially supported, and need experimentation
219 before being effective.
221 Previous definition of releases prevents long term support of previous releases.
222 For instance, bug and security fixes are not applied if the stable branch is too
225 Because the FRR users have a need to backport bug and security fixes after the
226 stable branch becomes too old, there is a need to provide support on a long term
227 basis on that stable branch. If that support is applied on that stable branch,
228 then that branch is a long term support branch.
230 Having a LTS branch requires extra-work and requires one person to be in charge
231 of that maintenance branch for a certain amount of time. The amount of time will
232 be by default set to 4 months, and can be increased. 4 months stands for the time
233 between two releases, this time can be applied to the decision to continue with a
234 LTS release or not. In all cases, that time period will be well-defined and
235 published. Also, a self nomination from a person that proposes to handle the LTS
236 branch is required. The work can be shared by multiple people. In all cases, there
237 must be at least one person that is in charge of the maintenance branch. The person
238 on people responsible for a maintenance branch must be a FRR maintainer. Note that
239 they may choose to abandon support for the maintenance branch at any time. If
240 no one takes over the responsibility of the LTS branch, then the support will be
243 The LTS branch duties are the following ones:
245 - organise meetings on a (bi-)weekly or monthly basis, the handling of issues
246 and pull requested relative to that branch. When time permits, this may be done
247 during the regularly scheduled FRR meeting.
249 - ensure the stability of the branch, by using and eventually adapting the
250 checking the CI tools of FRR ( indeed, maintaining may lead to create
251 maintenance branches for topotests or for CI).
253 It will not be possible to backport feature requests to LTS branches. Actually, it
254 is a false good idea to use LTS for that need. Introducing feature requests may
255 break the paradigm where all more recent releases should also include the feature
256 request. This would require the LTS maintainer to ensure that all more recent
257 releases have support for this feature request. Moreover, introducing features
258 requests may result in breaking the stability of the branch. LTS branches are first
259 done to bring long term support for stability.
264 Occassionally the community will desire the ability to work together
265 on a feature that is considered useful to FRR. In this case the
266 parties may ask the Maintainers for the creation of a development
267 branch in the main FRR repository. Requirements for this to happen
270 - A one paragraph description of the feature being implemented to
271 allow for the facilitation of discussion about the feature. This
272 might include pointers to relevant RFC's or presentations that
273 explain what is planned. This is intended to set a somewhat
274 low bar for organization.
275 - A branch maintainer must be named. This person is responsible for
276 keeping the branch up to date, and general communication about the
277 project with the other FRR Maintainers. Additionally this person
278 must already be a FRR Maintainer.
279 - Commits to this branch must follow the normal PR and commit process
280 as outlined in other areas of this document. The goal of this is
281 to prevent the current state where large features are submitted
282 and are so large they are difficult to review.
284 After a development branch has completed the work together, a final
285 review can be made and the branch merged into master. If a development
286 branch is becomes un-maintained or not being actively worked on after
287 three months then the Maintainers can decide to remove the branch.
292 The Debian project contains "official" packages for FRR. While FRR
293 Maintainers may participate in creating these, it is entirely the Debian
294 project's decision what to ship and how to work on this.
296 As a courtesy and for FRR's benefit, this packaging work is currently visible
297 in git branches named ``debian/*`` on the main FRR git repository. These
298 branches are for the exclusive use by people involved in Debian packaging work
299 for FRR. Direct commit access may be handed out and FRR git rules (review,
300 testing, etc.) do not apply. Do not push to these branches without talking
301 to the people noted under ``Maintainer:`` and ``Uploaders:`` in
302 ``debian/control`` on the target branch -- even if you are a FRR Maintainer.
306 The changelog will be the base for the release notes. A changelog entry for
307 your changes is usually not required and will be added based on your commit
308 messages by the maintainers. However, you are free to include an update to the
309 changelog with some better description.
311 Accords: non-code community consensus
312 =====================================
314 The FRR repository has a place for "accords" - these are items of
315 consideration for FRR that influence how we work as a community, but either
316 haven't resulted in code *yet*, or may *never* result in code being written.
317 They are placed in the ``doc/accords/`` directory.
319 The general idea is to simply pass small blurbs of text through our normal PR
320 procedures, giving them the same visibility, comment and review mechanisms as
321 code PRs - and changing them later is another PR. Please refer to the README
322 file in ``doc/accords/`` for further details. The file names of items in that
323 directory are hopefully helpful in determining whether some of them might be
324 relevant to your work.
326 Submitting Patches and Enhancements
327 ===================================
329 FRR accepts patches using GitHub pull requests.
331 The base branch for new contributions and non-critical bug fixes should be
332 ``master``. Please ensure your pull request is based on this branch when you
335 Code submitted by pull request will be automatically tested by one or more CI
336 systems. Once the automated tests succeed, other developers will review your
337 code for quality and correctness. After any concerns are resolved, your code
338 will be merged into the branch it was submitted against.
340 The title of the pull request should provide a high level technical
341 summary of the included patches. The description should provide
342 additional details that will help the reviewer to understand the context
343 of the included patches.
348 Before merging make sure a PR has squashed the following kinds of commits:
350 - Fixes/review feedback
355 This helps to automatically generate human-readable changelog messages.
360 There is a built-in commit linter. Basic rules:
362 - Commit messages must be prefixed with the name of the changed subsystem, followed
363 by a colon and a space and start with an imperative verb.
365 `Check <https://github.com/FRRouting/frr/tree/master/.github/commitlint.config.js>`_ all
366 the supported subsystems.
368 - Commit messages must not end with a period ``.``
370 Why was my pull request closed?
371 -------------------------------
373 Pull requests older than 180 days will be closed. Exceptions can be made for
374 pull requests that have active review comments, or that are awaiting other
375 dependent pull requests. Closed pull requests are easy to recreate, and little
376 work is lost by closing a pull request that subsequently needs to be reopened.
378 We want to limit the total number of pull requests in flight to:
380 - Maintain a clean project
381 - Remove old pull requests that would be difficult to rebase as the underlying code has changed over time
382 - Encourage code velocity
384 .. _license-for-contributions:
386 License for Contributions
387 -------------------------
388 FRR is under a “GPLv2 or later” license. Any code submitted must be released
389 under the same license (preferred) or any license which allows redistribution
390 under this GPLv2 license (eg MIT License).
391 It is forbidden to push any code that prevents from using GPLv3 license. This
392 becomes a community rule, as FRR produces binaries that links with Apache 2.0
393 libraries. Apache 2.0 and GPLv2 license are incompatible, if put together.
394 Please see `<http://www.apache.org/licenses/GPL-compatibility.html>`_ for
395 more information. This rule guarantees the user to distribute FRR binary code
396 without any licensing issues.
398 Pre-submission Checklist
399 ------------------------
400 - Format code (see `Code Formatting <#code-formatting>`__)
401 - Verify and acknowledge license (see :ref:`license-for-contributions`)
402 - Ensure you have properly signed off (see :ref:`signing-off`)
403 - Test building with various configurations:
407 - Verify building source distribution:
409 - ``make dist`` (and try rebuilding from the resulting tar file)
415 - In the case of a major new feature or other significant change, document
416 plans for continued maintenance of the feature. In addition it is a
417 requirement that automated testing must be written that exercises
418 the new feature within our existing CI infrastructure. Also the
419 addition of automated testing to cover any pull request is encouraged.
421 - All new code must use the current latest version of acceptable code.
423 - If a daemon is converted to YANG, then new code must use YANG.
424 - DEFPY's must be used for new cli
425 - Typesafe lists must be used
426 - printf formatting changes must be used
432 Code submitted to FRR must be signed off. We have the same requirements for
433 using the signed-off-by process as the Linux kernel. In short, you must include
434 a ``Signed-off-by`` tag in every patch.
436 An easy way to do this is to use ``git commit -s`` where ``-s`` will automatically
437 append a signed-off line to the end of your commit message. Also, if you commit
438 and forgot to add the line you can use ``git commit --amend -s`` to add the
439 signed-off line to the last commit.
441 ``Signed-off-by`` is a developer's certification that they have the right to
442 submit the patch for inclusion into the project. It is an agreement to the
443 :ref:`Developer's Certificate of Origin <developers-certificate-of-origin>`.
444 Code without a proper ``Signed-off-by`` line cannot and will not be merged.
446 If you are unfamiliar with this process, you should read the
447 `official policy at kernel.org <https://www.kernel.org/doc/html/latest/process/submitting-patches.html>`_.
449 `this article <http://www.linuxfoundation.org/content/how-participate-linux-community-0>`_
450 about participating in the Linux community on the Linux Foundation website to
451 be a helpful resource.
453 .. _developers-certificate-of-origin:
455 In short, when you sign off on a commit, you assert your agreement to all of
458 Developer's Certificate of Origin 1.1
460 By making a contribution to this project, I certify that:
462 (a) The contribution was created in whole or in part by me and I
463 have the right to submit it under the open source license
464 indicated in the file; or
466 (b) The contribution is based upon previous work that, to the best
467 of my knowledge, is covered under an appropriate open source
468 license and I have the right under that license to submit that
469 work with modifications, whether created in whole or in part by
470 me, under the same open source license (unless I am permitted to
471 submit under a different license), as indicated in the file; or
473 (c) The contribution was provided directly to me by some other
474 person who certified (a), (b) or (c) and I have not modified it.
476 (d) I understand and agree that this project and the contribution
477 are public and that a record of the contribution (including all
478 personal information I submit with it, including my sign-off) is
479 maintained indefinitely and may be redistributed consistent with
480 this project or the open source license(s) involved.
482 After Submitting Your Changes
483 -----------------------------
485 - Watch for Continuous Integration (CI) test results
487 - You should automatically receive an email with the test results
488 within less than 2 hrs of the submission. If you don’t get the
489 email, then check status on the GitHub pull request.
490 - Please notify the development mailing list if you think something
493 - If the tests failed:
495 - In general, expect the community to ignore the submission until
497 - It is up to you to fix and resubmit.
499 - This includes fixing existing unit (“make test”) tests if your
500 changes broke or changed them.
501 - It also includes fixing distribution packages for the failing
502 platforms (ie if new libraries are required).
503 - Feel free to ask for help on the development list.
505 - Go back to the submission process and repeat until the tests pass.
509 - Wait for reviewers. Someone will review your code or be assigned
511 - Respond to any comments or concerns the reviewer has. Use e-mail or
512 add a comment via github to respond or to let the reviewer know how
513 their comment or concern is addressed.
514 - An author must never delete or manually dismiss someone else's comments
515 or review. (A review may be overridden by agreement in the weekly
517 - When you have addressed someone's review comments, please click the
518 "re-request review" button (in the top-right corner of the PR page, next
519 to the reviewer's name, an icon that looks like "reload")
520 - The responsibility for keeping a PR moving rests with the author at
521 least as long as there are either negative CI results or negative review
522 comments. If you forget to mark a review comment as addressed (by
523 clicking re-request review), the reviewer may very well not notice and
524 won't come back to your PR.
525 - Automatically generated comments, e.g., those generated by CI systems,
526 may be deleted by authors and others when such comments are not the most
527 recent results from that automated comment source.
528 - After all comments and concerns are addressed, expect your patch
531 - Watch out for questions on the mailing list. At this time there will
532 be a manual code review and further (longer) tests by various
534 - Your submission is done once it is merged to the master branch.
536 Programming Languages, Tools and Libraries
537 ==========================================
539 The core of FRR is written in C (gcc or clang supported) and makes
540 use of GNU compiler extensions. A few non-essential scripts are
541 implemented in Perl and Python. FRR requires the following tools
542 to build distribution packages: automake, autoconf, texinfo, libtool and
543 gawk and various libraries (i.e. libpam and libjson-c).
545 If your contribution requires a new library or other tool, then please
546 highlight this in your description of the change. Also make sure it’s
547 supported by all FRR platform OSes or provide a way to build
548 without the library (potentially without the new feature) on the other
551 Documentation should be written in reStructuredText. Sphinx extensions may be
552 utilized but pure ReST is preferred where possible. See
553 :ref:`documentation`.
558 While C++ is not accepted for core components of FRR, extensions, modules or
559 other distinct components may want to use C++ and include FRR header files.
560 There is no requirement on contributors to work to retain C++ compatibility,
561 but fixes for C++ compatibility are welcome.
563 This implies that the burden of work to keep C++ compatibility is placed with
564 the people who need it, and they may provide it at their leisure to the extent
565 it is useful to them. So, if only a subset of header files, or even parts of
566 a header file are made available to C++, this is perfectly fine.
571 Code quality is paramount for any large program. Consequently we require
572 reviews of all submitted patches by at least one person other than the
573 submitter before the patch is merged.
575 Because of the nature of the software, FRR's maintainer list (i.e. those with
576 commit permissions) tends to contain employees / members of various
577 organizations. In order to prevent conflicts of interest, we use an honor
578 system in which submissions from an individual representing one company should
579 be merged by someone unaffiliated with that company.
581 Guidelines for code review
582 --------------------------
584 - As a rule of thumb, the depth of the review should be proportional to the
585 scope and / or impact of the patch.
587 - Anyone may review a patch.
589 - When using GitHub reviews, marking "Approve" on a code review indicates
590 willingness to merge the PR.
592 - For individuals with merge rights, marking "Changes requested" is equivalent
595 - For a PR you marked with "Changes requested", please respond to updates in a
596 timely manner to avoid impeding the flow of development.
598 - Rejected or obsolete PRs are generally closed by the submitter based
599 on requests and/or agreement captured in a PR comment. The comment
600 may originate with a reviewer or document agreement reached on Slack,
601 the Development mailing list, or the weekly technical meeting.
603 - Reviewers may ask for new automated testing if they feel that the
604 code change is large enough/significant enough to warrant such
607 For project members with merge permissions, the following patterns have
610 - a PR with any reviews requesting changes may not be merged.
612 - a PR with any negative CI result may not be merged.
614 - an open "yellow" review mark ("review requested, but not done") should be
615 given some time (a few days up to weeks, depending on the size of the PR),
616 but is not a merge blocker.
618 - a "textbubble" review mark ("review comments, but not positive/negative")
619 should be read through but is not a merge blocker.
621 - non-trivial PRs are generally given some time (again depending on the size)
622 for people to mark an interest in reviewing. Trivial PRs may be merged
623 immediately when CI is green.
626 Coding Practices & Style
627 ========================
632 Commit messages should be formatted in the same way as Linux kernel
633 commit messages. The format is roughly::
639 ``dir`` should be the top level source directory under which the change was
640 made. For example, a change in :file:`bgpd/rfapi` would be formatted as::
646 The first line should be no longer than 50 characters. Subsequent lines should
647 be wrapped to 72 characters.
649 The purpose of commit messages is to briefly summarize what the commit is
650 changing. Therefore, the extended summary portion should be in the form of an
651 English paragraph. Brief examples of program output are acceptable but if
652 present should be short (on the order of 10 lines) and clearly demonstrate what
653 has changed. The goal should be that someone with only passing familiarity with
654 the code in question can understand what is being changed.
656 Commit messages consisting entirely of program output are *unacceptable*. These
657 do not describe the behavior changed. For example, putting VTYSH output or the
658 result of test runs as the sole content of commit messages is unacceptable.
660 You must also sign off on your commit.
662 .. seealso:: :ref:`signing-off`
668 New files must have a copyright header (see :ref:`license-for-contributions`
669 above) added to the file. The header should be:
673 // SPDX-License-Identifier: GPL-2.0-or-later
675 * Title/Function of file
676 * Copyright (C) YEAR Author’s Name
681 A ``SPDX-License-Identifier`` header is required in all source files, i.e.
682 ``.c``, ``.h``, ``.cpp`` and ``.py`` files. The license boilerplate should be
683 removed in these files. Some existing files are missing this header, this is
686 A ``SPDX-License-Identifier`` header *and* the full license boilerplate is
687 required in schema definition files, i.e. ``.yang`` and ``.proto``. The
688 rationale for this is that these files are likely to be individually copied to
689 places outside FRR, and having only the SPDX header would become a "dangling
694 **DO NOT REMOVE A "Copyright" LINE OR AUTHOR NAME, EVER.**
696 **DO NOT APPLY AN SPDX HEADER WHEN THE LICENSE IS UNCLEAR, UNLESS YOU HAVE
697 CHECKED WITH *ALL* SIGNIFICANT AUTHORS.**
699 Please to keep ``#include <zebra.h>``. The absolute first header included in
700 any C file **must** be either ``zebra.h`` or ``config.h`` (with HAVE_CONFIG_H
704 Adding Copyright Claims to Existing Files
705 -----------------------------------------
707 When adding copyright claims for modifications to an existing file, please
708 add a ``Portions:`` section as shown below. If this section already exists, add
709 your new claim at the end of the list.
714 * Title/Function of file
715 * Copyright (C) YEAR Author’s Name
717 * Copyright (C) 2010 Entity A ....
718 * Copyright (C) 2016 Your name [optional brief change description]
722 Defensive coding requirements
723 -----------------------------
725 In general, code submitted into FRR will be rejected if it uses unsafe
726 programming practices. While there is no enforced overall ruleset, the
727 following requirements have achieved consensus:
729 - ``strcpy``, ``strcat`` and ``sprintf`` are unacceptable without exception.
730 Use ``strlcpy``, ``strlcat`` and ``snprintf`` instead. (Rationale: even if
731 you know the operation cannot overflow the buffer, a future code change may
732 inadvertedly introduce an overflow.)
734 - buffer size arguments, particularly to ``strlcpy`` and ``snprintf``, must
735 use ``sizeof()`` whereever possible. Particularly, do not use a size
736 constant in these cases. (Rationale: changing a buffer to another size
737 constant may leave the write operations on a now-incorrect size limit.)
739 - For stack allocated structs and arrays that should be zero initialized,
740 prefer initializer expressions over ``memset()`` wherever possible. This
741 helps prevent ``memset()`` calls being missed in branches, and eliminates the
742 error class of an incorrect ``size`` argument to ``memset()``.
744 For example, instead of:
750 memset(&mystruct, 0x00, sizeof(struct foo));
756 struct foo mystruct = {};
758 - Do not zero initialize stack allocated values that must be initialized with a
759 nonzero value in order to be used. This way the compiler and memory checking
760 tools can catch uninitialized value use that would otherwise be suppressed by
761 the (incorrect) zero initialization.
763 Other than these specific rules, coding practices from the Linux kernel as
764 well as CERT or MISRA C guidelines may provide useful input on safe C code.
765 However, these rules are not applied as-is; some of them expressly collide
766 with established practice.
769 Container implementations
770 ^^^^^^^^^^^^^^^^^^^^^^^^^
772 In particular to gain defensive coding benefits from better compiler type
773 checks, there is a set of replacement container data structures to be found
774 in :file:`lib/typesafe.h`. They're documented under :ref:`lists`.
776 Unfortunately, the FRR codebase is quite large, and migrating existing code to
777 use these new structures is a tedious and far-reaching process (even if it
778 can be automated with coccinelle, the patches would touch whole swaths of code
779 and create tons of merge conflicts for ongoing work.) Therefore, little
780 existing code has been migrated.
782 However, both **new code and refactors of existing code should use the new
783 containers**. If there are any reasons this can't be done, please work to
784 remove these reasons (e.g. by adding necessary features to the new containers)
785 rather than falling back to the old code.
787 In order of likelyhood of removal, these are the old containers:
789 - :file:`nhrpd/list.*`, ``hlist_*`` ⇒ ``DECLARE_LIST``
790 - :file:`nhrpd/list.*`, ``list_*`` ⇒ ``DECLARE_DLIST``
791 - :file:`lib/skiplist.*`, ``skiplist_*`` ⇒ ``DECLARE_SKIPLIST``
792 - :file:`lib/*_queue.h` (BSD), ``SLIST_*`` ⇒ ``DECLARE_LIST``
793 - :file:`lib/*_queue.h` (BSD), ``LIST_*`` ⇒ ``DECLARE_DLIST``
794 - :file:`lib/*_queue.h` (BSD), ``STAILQ_*`` ⇒ ``DECLARE_LIST``
795 - :file:`lib/*_queue.h` (BSD), ``TAILQ_*`` ⇒ ``DECLARE_DLIST``
796 - :file:`lib/hash.*`, ``hash_*`` ⇒ ``DECLARE_HASH``
797 - :file:`lib/linklist.*`, ``list_*`` ⇒ ``DECLARE_DLIST``
798 - open-coded linked lists ⇒ ``DECLARE_LIST``/``DECLARE_DLIST``
807 For C code, FRR uses Linux kernel style except where noted below. Code which
808 does not comply with these style guidelines will not be accepted.
810 The project provides multiple tools to allow you to correctly style your code
811 as painlessly as possible, primarily built around ``clang-format``.
814 In the project root there is a :file:`.clang-format` configuration file
815 which can be used with the ``clang-format`` source formatter tool from the
816 LLVM project. Most of the time, this is the easiest and smartest tool to
817 use. It can be run in a variety of ways. If you point it at a C source file
818 or directory of source files, it will format all of them. In the LLVM source
819 tree there are scripts that allow you to integrate it with ``git``, ``vim``
820 and ``emacs``, and there are third-party plugins for other editors. The
821 ``git`` integration is particularly useful; suppose you have some changes in
822 your git index. Then, with the integration installed, you can do the
829 This will format *only* the changes present in your index. If you have just
830 made a few commits and would like to correctly style only the changes made
831 in those commits, you can use the following syntax:
835 git clang-format HEAD~X
837 Where X is one more than the number of commits back from the tip of your
838 branch you would like ``clang-format`` to look at (similar to specifying the
839 target for a rebase).
841 The ``vim`` plugin is particularly useful. It allows you to select lines in
842 visual line mode and press a key binding to invoke ``clang-format`` on only
845 When using ``clang-format``, it is recommended to use the latest version.
846 Each consecutive version generally has better handling of various edge
847 cases. You may notice on occasion that two consecutive runs of
848 ``clang-format`` over the same code may result in changes being made on the
849 second run. This is an unfortunate artifact of the tool. Please check with
850 the kernel style guide if in doubt.
852 One stylistic problem with the FRR codebase is the use of ``DEFUN`` macros
853 for defining CLI commands. ``clang-format`` will happily format these macro
854 invocations, but the result is often unsightly and difficult to read.
855 Consequently, FRR takes a more relaxed position with how these are
856 formatted. In general you should lean towards using the style exemplified in
857 the section on :ref:`command-line-interface`. Because ``clang-format``
858 mangles this style, there is a Python script named ``tools/indent.py`` that
859 wraps ``clang-format`` and handles ``DEFUN`` macros as well as some other
860 edge cases specific to FRR. If you are submitting a new file, it is
861 recommended to run that script over the new file, preferably after ensuring
862 that the latest stable release of ``clang-format`` is in your ``PATH``.
864 Documentation on ``clang-format`` and its various integrations is maintained
867 https://clang.llvm.org/docs/ClangFormat.html
870 In the Linux kernel source tree there is a Perl script used to check
871 incoming patches for style errors. FRR uses an adapted version of this
872 script for the same purpose. It can be found at
873 :file:`tools/checkpatch.sh`. This script takes a git-formatted diff or
874 patch file, applies it to a clean FRR tree, and inspects the result to catch
875 potential style errors. Running this script on your patches before
876 submission is highly recommended. The CI system runs this script as well and
877 will comment on the PR with the results if style errors are found.
879 It is run like this::
881 ./checkpatch.sh <patch> <tree>
883 Reports are generated on ``stderr`` and the exit code indicates whether
884 issues were found (2, 1) or not (0).
886 Where ``<patch>`` is the path to the diff or patch file and ``<tree>`` is
887 the path to your FRR source tree. The tree should be on the branch that you
888 intend to submit the patch against. The script will make a best-effort
889 attempt to save the state of your working tree and index before applying the
890 patch, and to restore it when it is done, but it is still recommended that
891 you have a clean working tree as the script does perform a hard reset on
892 your tree during its run.
894 The script reports two classes of issues, namely WARNINGs and ERRORs. Please
895 pay attention to both of them. The script will generally report WARNINGs
896 where it cannot be 100% sure that a particular issue is real. In most cases
897 WARNINGs indicate an issue that needs to be fixed. Sometimes the script will
898 report false positives; these will be handled in code review on a
899 case-by-case basis. Since the script only looks at changed lines,
900 occasionally changing one part of a line can cause the script to report a
901 style issue already present on that line that is unrelated to the change.
902 When convenient it is preferred that these be cleaned up inline, but this is
905 In general, a developer should heed the information reported by checkpatch.
906 However, some flexibility is needed for cases where human judgement yields
907 better clarity than the script. Accordingly, it may be appropriate to
908 ignore some checkpatch.sh warnings per discussion among the submitter(s)
909 and reviewer(s) of a change. Misreporting of errors by the script is
910 possible. When this occurs, the exception should be handled either by
911 patching checkpatch to correct the false error report, or by documenting the
912 exception in this document under :ref:`style-exceptions`. If the incorrect
913 report is likely to appear again, a checkpatch update is preferred.
915 If the script finds one or more WARNINGs it will exit with 1. If it finds
916 one or more ERRORs it will exit with 2.
919 Please remember that while FRR provides these tools for your convenience,
920 responsibility for properly formatting your code ultimately lies on the
921 shoulders of the submitter. As such, it is recommended to double-check the
922 results of these tools to avoid delays in merging your submission.
924 In some cases, these tools modify or flag the format in ways that go beyond or
925 even conflict [#tool_style_conflicts]_ with the canonical documented Linux
926 kernel style. In these cases, the Linux kernel style takes priority;
927 non-canonical issues flagged by the tools are not compulsory but rather are
928 opportunities for discussion among the submitter(s) and reviewer(s) of a change.
930 **Whitespace changes in untouched parts of the code are not acceptable
931 in patches that change actual code.** To change/fix formatting issues,
932 please create a separate patch that only does formatting changes and
935 Kernel and BSD styles are documented externally:
937 - https://www.kernel.org/doc/html/latest/process/coding-style.html
938 - http://man.openbsd.org/style
940 For GNU coding style, use ``indent`` with the following invocation:
944 indent -nut -nfc1 file_for_submission.c
947 Historically, FRR used fixed-width integral types that do not exist in any
948 standard but were defined by most platforms at some point. Officially these
949 types are not guaranteed to exist. Therefore, please use the fixed-width
950 integral types introduced in the C99 standard when contributing new code to
951 FRR. If you need to convert a large amount of code to use the correct types,
952 there is a shell script in :file:`tools/convert-fixedwidth.sh` that will do the
953 necessary replacements.
955 +-----------+--------------------------+
956 | Incorrect | Correct |
957 +===========+==========================+
958 | u_int8_t | uint8_t |
959 +-----------+--------------------------+
960 | u_int16_t | uint16_t |
961 +-----------+--------------------------+
962 | u_int32_t | uint32_t |
963 +-----------+--------------------------+
964 | u_int64_t | uint64_t |
965 +-----------+--------------------------+
966 | u_char | uint8_t or unsigned char |
967 +-----------+--------------------------+
968 | u_short | unsigned short |
969 +-----------+--------------------------+
970 | u_int | unsigned int |
971 +-----------+--------------------------+
972 | u_long | unsigned long |
973 +-----------+--------------------------+
975 FRR also uses unnamed struct fields, enabled with ``-fms-extensions`` (cf.
976 https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html). The following two
977 patterns can/should be used where contextually appropriate:
990 struct inner inner_name;
995 .. _style-exceptions:
1000 FRR project code comes from a variety of sources, so there are some
1001 stylistic exceptions in place. They are organized here by branch.
1005 BSD coding style applies to:
1009 ``babeld`` uses, approximately, the following style:
1012 - Indents are 4 spaces
1013 - Function return types are on their own line
1015 For ``stable/3.0`` and ``stable/2.0``:
1017 GNU coding style apply to the following parts:
1029 BSD coding style applies to:
1037 Format all Python code with `black <https://github.com/psf/black>`_.
1041 python3 -m black <file.py>
1043 Run this on any Python files you modify before committing.
1045 FRR's Python code has been formatted with black version 19.10b.
1051 FRR uses YANG to define data models for its northbound interface. YANG models
1052 should follow conventions used by the IETF standard models. From a practical
1053 standpoint, this corresponds to the output produced by the ``yanglint`` tool
1054 included in the ``libyang`` project, which is used by FRR to parse and validate
1055 YANG models. You should run the following command on all YANG documents you
1058 .. code-block:: console
1060 yanglint -f yang <model>
1062 The output of this command should be identical to the input file. The sole
1063 exception to this is comments. ``yanglint`` does not support comments and will
1064 strip them from its output. You may include comments in your YANG documents,
1065 but they should be indented appropriately (use spaces). Where possible,
1066 comments should be eschewed in favor of a suitable ``description`` statement.
1068 In short, a diff between your input file and the output of ``yanglint`` should
1069 either be empty or contain only comments.
1074 Most of the time checkpatch errors should be corrected. Occasionally as a group
1075 maintainers will decide to ignore certain stylistic issues. Usually this is
1076 because correcting the issue is not possible without large unrelated code
1077 changes. When an exception is made, if it is unlikely to show up again and
1078 doesn't warrant an update to checkpatch, it is documented here.
1080 +------------------------------------------+---------------------------------------------------------------+
1081 | Issue | Ignore Reason |
1082 +==========================================+===============================================================+
1083 | DEFPY_HIDDEN, DEFPY_ATTR: complex macros | DEF* macros cannot be wrapped in parentheses without updating |
1084 | should be wrapped in parentheses | all usages of the macro, which would be highly disruptive. |
1085 +------------------------------------------+---------------------------------------------------------------+
1087 Types of configurables
1088 ----------------------
1092 This entire section essentially just argues to not make configuration
1093 unnecessarily involved for the user. Rather than rules, this is more of
1094 a list of conclusions intended to help make FRR usable for operators.
1097 Almost every feature FRR has comes with its own set of switches and options.
1098 There are several stages at which configuration can be applied. In order of
1099 preference, these are:
1101 - at configuration/runtime, through YANG.
1103 This is the preferred way for all FRR knobs. Not all daemons and features
1104 are fully YANGified yet, so in some cases new features cannot rely on a
1105 YANG interface. If a daemon already implements a YANG interface (even
1106 partial), new CLI options must be implemented through a YANG model.
1110 Unlike everything else in this section being guidelines with some slack,
1111 implementing and using a YANG interface for new CLI options in (even
1112 partially!) YANGified daemons is a hard requirement.
1115 - at configuration/runtime, through the CLI.
1117 The "good old" way for all regular configuration. More involved for users
1118 to automate *correctly* than YANG.
1120 - at startup, by loading additional modules.
1122 If a feature introduces a dependency on additional libraries (e.g. libsnmp,
1123 rtrlib, etc.), this is the best way to encapsulate the dependency. Having
1124 a separate module allows the distribution to create a separate package
1125 with the extra dependency, so FRR can still be installed without pulling
1128 A module may also be appropriate if a feature is large and reasonably well
1129 isolated. Reducing the amount of running the code is a security benefit,
1130 so even if there are no new external dependencies, modules can be useful.
1132 While modules cannot currently be loaded at runtime, this is a tradeoff
1133 decision that was made to allow modules to change/extend code that is very
1134 hard to (re)adjust at runtime. If there is a case for runtime (un)loading
1135 of modules, this tradeoff can absolutely be reevaluated.
1137 - at startup, with command line options.
1139 This interface is only appropriate for options that have an effect very
1140 early in FRR startup, i.e. before configuration is loaded. Anything that
1141 affects configuration load itself should be here, as well as options
1142 changing the environment FRR runs in.
1144 If a tunable can be changed at runtime, a command line option is only
1145 acceptable if the configured value has an effect before configuration is
1146 loaded (e.g. zebra reads routes from the kernel before loading config, so
1147 the netlink buffer size is an appropriate command line option.)
1149 - at compile time, with ``./configure`` options.
1151 This is the absolute last preference for tunables, since the distribution
1152 needs to make the decision for the user and/or the user needs to rebuild
1153 FRR in order to change the option.
1155 "Good" configure options do one of three things:
1157 - set distribution-specific parameters, most prominently all the path
1158 options. File system layout is a distribution/packaging choice, so the
1159 user would hopefully never need to adjust these.
1161 - changing toolchain behavior, e.g. instrumentation, warnings,
1162 optimizations and sanitizers.
1164 - enabling/disabling parts of the build, especially if they need
1165 additional dependencies. Being able to build only parts of FRR, or
1166 without some library, is useful. **The only effect these options should
1167 have is adding or removing files from the build result.** If a knob
1168 in this category causes the same binary to exist in different variants,
1169 it is likely implemented incorrectly!
1173 This last guideline is currently ignored by several configure options.
1174 ``vtysh`` in general depends on the entire list of enabled daemons,
1175 and options like ``--enable-bgp-vnc`` and ``--enable-ospfapi`` change
1176 daemons internally. Consider this more of an "ideal" than a "rule".
1179 Whenever adding new knobs, please try reasonably hard to go up as far as
1180 possible on the above list. Especially ``./configure`` flags are often enough
1181 the "easy way out" but should be avoided when at all possible. To a lesser
1182 degree, the same applies to command line options.
1185 Compile-time conditional code
1186 -----------------------------
1188 Many users access FRR via binary packages from 3rd party sources;
1189 compile-time code puts inclusion/exclusion in the hands of the package
1190 maintainer. Please think very carefully before making code conditional
1191 at compile time, as it increases regression testing, maintenance
1192 burdens, and user confusion. In particular, please avoid gratuitous
1193 ``--enable-…`` switches to the configure script - in general, code
1194 should be of high quality and in working condition, or it shouldn’t be
1197 When code must be compile-time conditional, try have the compiler make
1198 it conditional rather than the C pre-processor so that it will still be
1199 checked by the compiler, even if disabled. For example,
1212 #endif /* SOME_SYMBOL */
1214 Note that the former approach requires ensuring that ``SOME_SYMBOL`` will be
1215 defined (watch your ``AC_DEFINE``\ s).
1217 Debug-guards in code
1218 --------------------
1220 Debugging statements are an important methodology to allow developers to fix
1221 issues found in the code after it has been released. The caveat here is that
1222 the developer must remember that people will be using the code at scale and in
1223 ways that can be unexpected for the original implementor. As such debugs
1224 **MUST** be guarded in such a way that they can be turned off. FRR has the
1225 ability to turn on/off debugs from the CLI and it is expected that the
1226 developer will use this convention to allow control of their debugs.
1228 Custom syntax-like block macros
1229 -------------------------------
1231 FRR uses some macros that behave like the ``for`` or ``if`` C keywords. These
1232 macros follow these patterns:
1234 - loop-style macros are named ``frr_each_*`` (and ``frr_each``)
1235 - single run macros are named ``frr_with_*``
1236 - to avoid confusion, ``frr_with_*`` macros must always use a ``{ ... }``
1237 block even if the block only contains one statement. The ``frr_each``
1238 constructs are assumed to be well-known enough to use normal ``for`` rules.
1239 - ``break``, ``return`` and ``goto`` all work correctly. For loop-style
1240 macros, ``continue`` works correctly too.
1242 Both the ``each`` and ``with`` keywords are inspired by other (more
1243 higher-level) programming languages that provide these constructs.
1245 There are also some older iteration macros, e.g. ``ALL_LIST_ELEMENTS`` and
1246 ``FOREACH_AFI_SAFI``. These macros in some cases do **not** fulfill the above
1247 pattern (e.g. ``break`` does not work in ``FOREACH_AFI_SAFI`` because it
1248 expands to 2 nested loops.)
1250 Static Analysis and Sanitizers
1251 ------------------------------
1252 Clang/LLVM and GCC come with a variety of tools that can be used to help find
1256 This is a static analyzer that scans the source code looking for patterns
1257 that are likely to be bugs. The tool is run automatically on pull requests
1258 as part of CI and new static analysis warnings will be placed in the CI
1259 results. FRR aims for absolutely zero static analysis errors. While the
1260 project is not quite there, code that introduces new static analysis errors
1261 is very unlikely to be merged.
1264 This is an excellent tool that provides runtime instrumentation for
1265 detecting memory errors. As part of CI FRR is built with this
1266 instrumentation and run through a series of tests to look for any results.
1267 Testing your own code with this tool before submission is encouraged. You
1268 can enable it by passing::
1270 --enable-address-sanitizer
1275 Similar to AddressSanitizer, this tool provides runtime instrumentation for
1276 detecting data races. If you are working on or around multithreaded code,
1277 extensive testing with this instrumtation enabled is *highly* recommended.
1278 You can enable it by passing::
1280 --enable-thread-sanitizer
1285 Similar to AddressSanitizer, this tool provides runtime instrumentation for
1286 detecting use of uninitialized heap memory. Testing your own code with this
1287 tool before submission is encouraged. You can enable it by passing::
1289 --enable-memory-sanitizer
1293 All of the above tools are available in the Clang/LLVM toolchain since 3.4.
1294 AddressSanitizer and ThreadSanitizer are available in recent versions of GCC,
1295 but are no longer actively maintained. MemorySanitizer is not available in GCC.
1299 The different Sanitizers are mostly incompatible with each other. Please
1300 refer to GCC/LLVM documentation for details.
1303 This is a GCC plugin provided with FRR that does extended type checks for
1304 ``%pFX``-style printfrr extensions. To use this plugin,
1306 1. install GCC plugin development files, e.g.::
1308 apt-get install gcc-10-plugin-dev
1310 2. **before** running ``configure``, compile the plugin with::
1312 make -C tools/gcc-plugins CXX=g++-10
1314 (Edit the GCC version to what you're using, it should work for GCC 9 or
1317 After this, the plugin should be automatically picked up by ``configure``.
1318 The plugin does not change very frequently, so you can keep it around across
1319 work on different FRR branches. After a ``git clean -x``, the ``make`` line
1320 will need to be run again. You can also add ``--with-frr-format`` to the
1321 ``configure`` line to make sure the plugin is used, otherwise if something
1322 is not set up correctly it might be silently ignored.
1326 Do **not** enable this plugin for package/release builds. It is intended
1327 for developer/debug builds only. Since it modifies the compiler, it may
1328 cause silent corruption of the executable files.
1330 Using the plugin also changes the string for ``PRI[udx]64`` from the
1331 system value to ``%L[udx]`` (normally ``%ll[udx]`` or ``%l[udx]``.)
1333 Additionally, the FRR codebase is regularly scanned with Coverity.
1334 Unfortunately Coverity does not have the ability to handle scanning pull
1335 requests, but after code is merged it will send an email notifying project
1336 members with Coverity access of newly introduced defects.
1338 Executing non-installed dynamic binaries
1339 ----------------------------------------
1341 Since FRR uses the GNU autotools build system, it inherits its shortcomings.
1342 To execute a binary directly from the build tree under a wrapper like
1343 `valgrind`, `gdb` or `strace`, use::
1345 ./libtool --mode=execute valgrind [--valgrind-opts] zebra/zebra [--zebra-opts]
1347 While replacing valgrind/zebra as needed. The `libtool` script is found in
1348 the root of the build directory after `./configure` has completed. Its purpose
1349 is to correctly set up `LD_LIBRARY_PATH` so that libraries from the build tree
1350 are used. (On some systems, `libtool` is also available from PATH, but this is
1351 not always the case.)
1358 CLI's are a complicated ugly beast. Additions or changes to the CLI should use
1359 a DEFPY to encapsulate one setting as much as is possible. Additionally as new
1360 DEFPY's are added to the system, documentation should be provided for the new
1363 Backwards Compatibility
1364 -----------------------
1366 As a general principle, changes to CLI and code in the lib/ directory should be
1367 made in a backwards compatible fashion. This means that changes that are purely
1368 stylistic in nature should be avoided, e.g., renaming an existing macro or
1369 library function name without any functional change. When adding new parameters
1370 to common functions, it is also good to consider if this too should be done in
1371 a backward compatible fashion, e.g., by preserving the old form in addition to
1372 adding the new form.
1374 This is not to say that minor or even major functional changes to CLI and
1375 common code should be avoided, but rather that the benefit gained from a change
1376 should be weighed against the added cost/complexity to existing code. Also,
1377 that when making such changes, it is good to preserve compatibility when
1378 possible to do so without introducing maintenance overhead/cost. It is also
1379 important to keep in mind, existing code includes code that may reside in
1380 private repositories (and is yet to be submitted) or code that has yet to be
1381 migrated from Quagga to FRR.
1383 That said, compatibility measures can (and should) be removed when either:
1385 - they become a significant burden, e.g. when data structures change and the
1386 compatibility measure would need a complex adaptation layer or becomes
1388 - some measure of time (dependent on the specific case) has passed, so that
1389 the compatibility grace period is considered expired.
1391 For CLI commands, the deprecation period is 1 year.
1393 In all cases, compatibility pieces should be marked with compiler/preprocessor
1394 annotations to print warnings at compile time, pointing to the appropriate
1395 update path. A ``-Werror`` build should fail if compatibility bits are used. To
1396 avoid compilation issues in released code, such compiler/preprocessor
1397 annotations must be ignored non-development branches. For example:
1401 #if CONFDATE > 20180403
1402 CPP_NOTICE("Use of <XYZ> is deprecated, please use <ABC>")
1405 Preferably, the shell script :file:`tools/fixup-deprecated.py` will be
1406 updated along with making non-backwards compatible code changes, or an
1407 alternate script should be introduced, to update the code to match the
1408 change. When the script is updated, there is no need to preserve the
1409 deprecated code. Note that this does not apply to user interface
1410 changes, just internal code, macros and libraries.
1415 When in doubt, follow the guidelines in the Linux kernel style guide, or ask on
1416 the development mailing list / public Slack instance.
1421 New JSON output in FRR needs to be backed by schema, in particular a YANG model.
1422 When adding new JSON, first search for an existing YANG model, either in FRR or
1423 a standard model (e.g., IETF) and use that model as the basis for any JSON
1424 structure and *especially* for key names and canonical values formats.
1426 If no YANG model exists to support the JSON then an FRR YANG model needs to be
1427 added to or created to support the JSON format.
1429 * All JSON keys are to be ``camelCased``, with no spaces. YANG modules almost
1430 always use ``kebab-case`` (i.e., all lower case with hyphens to separate
1431 words), so these identifiers need to be mapped to ``camelCase`` by removing
1432 the hyphen (or symbol) and capitalizing the following letter, for
1433 example "router-id" becomes "routerId"
1434 * Commands which output JSON should produce ``{}`` if they have nothing to
1436 * In general JSON commands include a ``json`` keyword typically at the end of
1437 the CLI command (e.g., ``show ip ospf json``)
1442 Please consider using ``const`` when possible: it's a useful hint to
1443 callers about the limits to side-effects from your apis, and it makes
1444 it possible to use your apis in paths that involve ``const``
1445 objects. If you encounter existing apis that *could* be ``const``,
1446 consider including changes in your own pull-request.
1448 Help with specific warnings
1449 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
1451 FRR's configure script enables a whole batch of extra warnings, some of which
1452 may not be obvious in how to fix. Here are some notes on specific warnings:
1454 * ``-Wstrict-prototypes``: you probably just forgot the ``void`` in a function
1455 declaration with no parameters, i.e. ``static void foo() {...}`` rather than
1456 ``static void foo(void) {...}``.
1458 Without the ``void``, in C, it's a function with *unspecified* parameters
1459 (and varargs calling convention.) This is a notable difference to C++, where
1460 the ``void`` is optional and an empty parameter list means no parameters.
1462 * ``"strict match required"`` from the frr-format plugin: check if you are
1463 using a cast in a printf parameter list. The frr-format plugin cannot
1464 access correct full type information for casts like
1465 ``printfrr(..., (uint64_t)something, ...)`` and will print incorrect
1466 warnings particularly if ``uint64_t``, ``size_t`` or ``ptrdiff_t`` are
1467 involved. The problem is *not* triggered with a variable or function return
1468 value of the exact same type (without a cast).
1470 Since these cases are very rare, community consensus is to just work around
1471 the warning even though the code might be correct. If you are running into
1472 this, your options are:
1474 1. try to avoid the cast altogether, maybe using a different printf format
1475 specifier (e.g. ``%lu`` instead of ``%zu`` or ``PRIu64``).
1476 2. fix the type(s) of the function/variable/struct member being printed
1477 3. create a temporary variable with the value and print that without a cast
1478 (this is the last resort and was not necessary anywhere so far.)
1486 FRR uses Sphinx+RST as its documentation system. The document you are currently
1487 reading was generated by Sphinx from RST source in
1488 :file:`doc/developer/workflow.rst`. The documentation is structured as follows:
1490 +-----------------------+-------------------------------------------+
1491 | Directory | Contents |
1492 +=======================+===========================================+
1493 | :file:`doc/user` | User documentation; configuration guides; |
1494 | | protocol overviews |
1495 +-----------------------+-------------------------------------------+
1496 | :file:`doc/developer` | Developer's documentation; API specs; |
1497 | | datastructures; architecture overviews; |
1498 | | project management procedure |
1499 +-----------------------+-------------------------------------------+
1500 | :file:`doc/manpages` | Source for manpages |
1501 +-----------------------+-------------------------------------------+
1502 | :file:`doc/figures` | Images and diagrams |
1503 +-----------------------+-------------------------------------------+
1504 | :file:`doc/extra` | Miscellaneous Sphinx extensions, scripts, |
1505 | | customizations, etc. |
1506 +-----------------------+-------------------------------------------+
1508 Each of these directories, with the exception of :file:`doc/figures` and
1509 :file:`doc/extra`, contains a Sphinx-generated Makefile and configuration
1510 script :file:`conf.py` used to set various document parameters. The makefile
1511 can be used for a variety of targets; invoke `make help` in any of these
1512 directories for a listing of available output formats. For convenience, there
1513 is a top-level :file:`Makefile.am` that has targets for PDF and HTML
1514 documentation for both developer and user documentation, respectively. That
1515 makefile is also responsible for building manual pages packed with distribution
1518 Indent and styling should follow existing conventions:
1520 - 3 spaces for indents under directives
1521 - Cross references may contain only lowercase alphanumeric characters and
1523 - Lines wrapped to 80 characters where possible
1525 Characters for header levels should follow Python documentation guide:
1527 - ``#`` with overline, for parts
1528 - ``*`` with overline, for chapters
1529 - ``=``, for sections
1530 - ``-``, for subsections
1531 - ``^``, for subsubsections
1532 - ``"``, for paragraphs
1534 After you have made your changes, please make sure that you can invoke
1535 ``make latexpdf`` and ``make html`` with no warnings.
1537 The documentation is currently incomplete and needs love. If you find a broken
1538 cross-reference, figure, dead hyperlink, style issue or any other nastiness we
1539 gladly accept documentation patches.
1541 To build the docs, please ensure you have installed a recent version of
1542 `Sphinx <http://www.sphinx-doc.org/en/stable/install.html>`_. If you want to
1543 build LaTeX or PDF docs, you will also need a full LaTeX distribution
1549 FRR is a large and complex software project developed by many different people
1550 over a long period of time. Without adequate documentation, it can be
1551 exceedingly difficult to understand code segments, APIs and other interfaces.
1552 In the interest of keeping the project healthy and maintainable, you should
1553 make every effort to document your code so that other people can understand
1554 what it does without needing to closely read the code itself.
1556 Some specific guidelines that contributors should follow are:
1558 - Functions exposed in header files should have descriptive comments above
1559 their signatures in the header file. At a minimum, a function comment should
1560 contain information about the return value, parameters, and a general summary
1561 of the function's purpose. Documentation on parameter values can be omitted
1562 if it is (very) obvious what they are used for.
1564 Function comments must follow the style for multiline comments laid out in
1565 the kernel style guide.
1572 * Determines whether or not a string is cool.
1575 * the string to check for coolness
1578 * whether capslock is cruise control for cool
1581 * 7 if the text is cool, 0 otherwise
1583 int check_coolness(const char *text, bool is_clccfc);
1585 Function comments should make it clear what parameters and return values are
1588 - Static functions should have descriptive comments in the same form as above
1589 if what they do is not immediately obvious. Use good engineering judgement
1590 when deciding whether a comment is necessary. If you are unsure, document
1592 - Global variables, static or not, should have a comment describing their use.
1593 - **For new code in lib/, these guidelines are hard requirements.**
1595 If you make significant changes to portions of the codebase covered in the
1596 Developer's Manual, add a major subsystem or feature, or gain arcane mastery of
1597 some undocumented or poorly documented part of the codebase, please document
1598 your work so others can benefit. If you add a major feature or introduce a new
1599 API, please document the architecture and API to the best of your abilities in
1600 the Developer's Manual, using good judgement when choosing where to place it.
1602 Finally, if you come across some code that is undocumented and feel like
1603 going above and beyond, document it! We absolutely appreciate and accept
1604 patches that document previously undocumented code.
1609 If you are contributing code that adds significant user-visible functionality
1610 please document how to use it in :file:`doc/user`. Use good judgement when
1611 choosing where to place documentation. For example, instructions on how to use
1612 your implementation of a new BGP draft should go in the BGP chapter instead of
1613 being its own chapter. If you are adding a new protocol daemon, please create a
1619 FRR has some customizations applied to the Sphinx markup that go a long way
1620 towards making documentation easier to use, write and maintain.
1625 When documenting CLI please use the ``.. clicmd::`` directive. This directive
1626 will format the command and generate index entries automatically. For example,
1627 the command :clicmd:`show pony` would be documented as follows:
1629 .. code-block:: rest
1631 .. clicmd:: show pony
1633 Prints an ASCII pony. Example output:::
1648 When documented this way, CLI commands can be cross referenced with the
1649 ``:clicmd:`` inline markup like so:
1651 .. code-block:: rest
1655 This is very helpful for users who want to quickly remind themselves what a
1656 particular command does.
1658 When documenting a cli that has a ``no`` form, please do not include the ``no``
1659 form. I.e. ``no show pony`` would not be documented anywhere. Since most
1660 commands have ``no`` forms, users should be able to infer these or get help
1661 from vtysh's completions.
1663 When documenting commands that have lots of possible variants, just document
1664 the single command in summary rather than enumerating each possible variant.
1665 E.g. for ``show pony [foo|bar]``, do not:
1667 .. code-block:: rest
1669 .. clicmd:: show pony
1670 .. clicmd:: show pony foo
1671 .. clicmd:: show pony bar
1675 .. code-block:: rest
1677 .. clicmd:: show pony [foo|bar]
1680 Configuration Snippets
1681 ^^^^^^^^^^^^^^^^^^^^^^
1683 When putting blocks of example configuration please use the
1684 ``.. code-block::`` directive and specify ``frr`` as the highlighting language,
1685 as in the following example. This will tell Sphinx to use a custom Pygments
1686 lexer to highlight FRR configuration syntax.
1688 .. code-block:: rest
1693 ! Example configuration file.
1695 log file /tmp/log.log
1696 service integrated-vtysh-config
1698 ip route 1.2.3.0/24 reject
1699 ipv6 route de:ea:db:ee:ff::/64 reject
1703 .. _GitHub: https://github.com/frrouting/frr
1704 .. _GitHub issues: https://github.com/frrouting/frr/issues
1706 .. rubric:: Footnotes
1708 .. [#tool_style_conflicts] For example, lines over 80 characters are allowed
1709 for text strings to make it possible to search the code for them: please
1710 see `Linux kernel style (breaking long lines and strings) <https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings>`_
1711 and `Issue #1794 <https://github.com/FRRouting/frr/issues/1794>`_.