]>
Commit | Line | Data |
---|---|---|
9de103f0 QY |
1 | .. _process-and-workflow: |
2 | ||
3 | ******************* | |
b22ba015 | 4 | Process & Workflow |
9de103f0 | 5 | ******************* |
d1890d04 | 6 | |
b6820993 QY |
7 | .. highlight:: none |
8 | ||
b22ba015 QY |
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. | |
d1890d04 | 12 | |
9de103f0 QY |
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 | |
b22ba015 QY |
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. | |
d1890d04 | 18 | |
9de103f0 QY |
19 | Mailing Lists |
20 | ============= | |
d1890d04 | 21 | |
b22ba015 QY |
22 | The FRR development group maintains multiple mailing lists for use by the |
23 | community. Italicized lists are private. | |
d1890d04 QY |
24 | |
25 | +----------------------------------+--------------------------------+ | |
26 | | Topic | List | | |
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 | +----------------------------------+--------------------------------+ | |
38 | ||
9de103f0 | 39 | The Development list is used to discuss and document general issues related to |
b6820993 QY |
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/. | |
47 | ||
48 | Development & Release Cycle | |
49 | =========================== | |
50 | ||
51 | Development | |
52 | ----------- | |
53 | ||
54 | .. figure:: ../figures/git_branches.png | |
55 | :align: center | |
56 | :scale: 55% | |
57 | :alt: Merging Git branches into a central trunk | |
58 | ||
59 | Rough outline of FRR development workflow | |
60 | ||
61 | The master Git for FRR resides on `GitHub`_. | |
62 | ||
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 | |
67 | branches. | |
8ce7861f | 68 | |
b6820993 | 69 | Subsequent point releases based on a major branch are handled with git tags. |
c804874a | 70 | |
b6820993 QY |
71 | Releases |
72 | -------- | |
73 | FRR employs a ``<MAJOR>.<MINOR>.<BUGFIX>`` versioning scheme. | |
c804874a | 74 | |
b6820993 | 75 | ``MAJOR`` |
ac97970d DL |
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.) | |
c804874a | 81 | |
b6820993 | 82 | ``MINOR`` |
ac97970d DL |
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. | |
c804874a | 87 | |
b6820993 | 88 | ``BUGFIX`` |
ac97970d | 89 | Fixes for actual bugs and/or security issues. Fully compatible. |
c804874a QY |
90 | |
91 | We will pull a new development branch for the next release every 4 months. The | |
b6820993 QY |
92 | current schedule is Feb/June/October 1. The decision for a ``MAJOR/MINOR`` |
93 | release is made at the time of branch pull based on what has been received the | |
94 | previous 4 months. The branch name will be ``dev/MAJOR.MINOR``. At this point | |
95 | in time the master branch and this new branch, :file:`configure.ac`, | |
96 | documentation and packaging systems will be updated to reflect the next | |
97 | possible release name to allow for easy distinguishing. | |
98 | ||
99 | After one month the development branch will be renamed to | |
bd2b4fc3 PG |
100 | ``stable/MAJOR.MINOR``. The branch is a stable branch. This process is not |
101 | held up unless a crash or security issue has been found and needs to | |
102 | be addressed. Issues being fixed will not cause a delay. | |
c804874a QY |
103 | |
104 | Bugfix releases are made as needed at 1 month intervals until the next | |
bd2b4fc3 | 105 | ``MAJOR.MINOR`` release branch is pulled. Depending on the severity of the bugs, |
c804874a QY |
106 | bugfix releases may occur sooner. |
107 | ||
07ff01d2 PG |
108 | Bugfixes are applied to the two most recent releases. However, backporting of bug |
109 | fixes to older than the two most recent releases will not be prevented, if acked | |
110 | under the classical development workflow applying for a pull request. | |
111 | ||
112 | Security fixes are backported to all releases less than or equal to at least one | |
113 | year old. Security fixes may also be backported to older releases depending on | |
114 | severity. | |
115 | ||
f4bcc72f QY |
116 | For detailed instructions on how to produce an FRR release, refer to |
117 | :ref:`frr-release-procedure`. | |
118 | ||
bd2b4fc3 PG |
119 | |
120 | Long term support branches ( LTS ) | |
121 | ----------------------------------------- | |
122 | ||
123 | This kind of branch is not yet officially supported, and need experimentation | |
124 | before being effective. | |
125 | ||
126 | Previous definition of releases prevents long term support of previous releases. | |
127 | For instance, bug and security fixes are not applied if the stable branch is too | |
128 | old. | |
129 | ||
130 | Because the FRR users have a need to backport bug and security fixes after the | |
131 | stable branch becomes too old, there is a need to provide support on a long term | |
132 | basis on that stable branch. If that support is applied on that stable branch, | |
133 | then that branch is a long term support branch. | |
134 | ||
135 | Having a LTS branch requires extra-work and requires one person to be in charge | |
136 | of that maintenance branch for a certain amount of time. The amount of time will | |
137 | be by default set to 4 months, and can be increased. 4 months stands for the time | |
138 | between two releases, this time can be applied to the decision to continue with a | |
139 | LTS release or not. In all cases, that time period will be well-defined and | |
140 | published. Also, a self nomination from a person that proposes to handle the LTS | |
141 | branch is required. The work can be shared by multiple people. In all cases, there | |
142 | must be at least one person that is in charge of the maintenance branch. The person | |
143 | on people responsible for a maintenance branch must be a FRR maintainer. Note that | |
144 | they may choose to abandon support for the maintenance branch at any time. If | |
56f0bea7 | 145 | no one takes over the responsibility of the LTS branch, then the support will be |
bd2b4fc3 PG |
146 | discontinued. |
147 | ||
148 | The LTS branch duties are the following ones: | |
149 | ||
150 | - organise meetings on a (bi-)weekly or monthly basis, the handling of issues | |
151 | and pull requested relative to that branch. When time permits, this may be done | |
152 | during the regularly scheduled FRR meeting. | |
153 | ||
154 | - ensure the stability of the branch, by using and eventually adapting the | |
155 | checking the CI tools of FRR ( indeed, maintaining may lead to create | |
156 | maintenance branches for topotests or for CI). | |
157 | ||
158 | It will not be possible to backport feature requests to LTS branches. Actually, it | |
159 | is a false good idea to use LTS for that need. Introducing feature requests may | |
160 | break the paradigm where all more recent releases should also include the feature | |
161 | request. This would require the LTS maintainer to ensure that all more recent | |
162 | releases have support for this feature request. Moreover, introducing features | |
163 | requests may result in breaking the stability of the branch. LTS branches are first | |
164 | done to bring long term support for stability. | |
8ce7861f | 165 | |
16318c5c DS |
166 | Development Branches |
167 | -------------------- | |
168 | ||
169 | Occassionally the community will desire the ability to work together | |
170 | on a feature that is considered useful to FRR. In this case the | |
171 | parties may ask the Maintainers for the creation of a development | |
172 | branch in the main FRR repository. Requirements for this to happen | |
173 | are: | |
174 | ||
175 | - A one paragraph description of the feature being implemented to | |
176 | allow for the facilitation of discussion about the feature. This | |
177 | might include pointers to relevant RFC's or presentations that | |
178 | explain what is planned. This is intended to set a somewhat | |
179 | low bar for organization. | |
180 | - A branch maintainer must be named. This person is responsible for | |
181 | keeping the branch up to date, and general communication about the | |
182 | project with the other FRR Maintainers. Additionally this person | |
183 | must already be a FRR Maintainer. | |
184 | - Commits to this branch must follow the normal PR and commit process | |
185 | as outlined in other areas of this document. The goal of this is | |
186 | to prevent the current state where large features are submitted | |
187 | and are so large they are difficult to review. | |
188 | ||
189 | After a development branch has completed the work together, a final | |
190 | review can be made and the branch merged into master. If a development | |
191 | branch is becomes un-maintained or not being actively worked on after | |
192 | three months then the Maintainers can decide to remove the branch. | |
193 | ||
d1890d04 | 194 | Changelog |
b6820993 | 195 | --------- |
b22ba015 QY |
196 | The changelog will be the base for the release notes. A changelog entry for |
197 | your changes is usually not required and will be added based on your commit | |
198 | messages by the maintainers. However, you are free to include an update to the | |
199 | changelog with some better description. | |
d1890d04 QY |
200 | |
201 | Submitting Patches and Enhancements | |
9de103f0 | 202 | =================================== |
d1890d04 | 203 | |
b22ba015 QY |
204 | FRR accepts patches from two sources: |
205 | ||
206 | - Email (git format-patch) | |
b6820993 | 207 | - GitHub pull request |
b22ba015 | 208 | |
b6820993 QY |
209 | Contributors are highly encouraged to use GitHub's fork-and-PR workflow. It is |
210 | easier for us to review it, test it, try it and discuss it on GitHub than it is | |
211 | via email, thus your patch will get more attention more quickly on GitHub. | |
b22ba015 QY |
212 | |
213 | The base branch for new contributions and non-critical bug fixes should be | |
214 | ``master``. Please ensure your pull request is based on this branch when you | |
215 | submit it. | |
216 | ||
b6820993 QY |
217 | GitHub Pull Requests |
218 | -------------------- | |
d1890d04 | 219 | |
b6820993 QY |
220 | The preferred method of submitting changes is a GitHub pull request. Code |
221 | submitted by pull request will be automatically tested by one or more CI | |
222 | systems. Once the automated tests succeed, other developers will review your | |
223 | code for quality and correctness. After any concerns are resolved, your code | |
224 | will be merged into the branch it was submitted against. | |
d1890d04 | 225 | |
01bf2ccb LB |
226 | The title of the pull request should provide a high level technical |
227 | summary of the included patches. The description should provide | |
228 | additional details that will help the reviewer to understand the context | |
229 | of the included patches. | |
230 | ||
b6820993 QY |
231 | Patch Submission via Mailing List |
232 | --------------------------------- | |
d1890d04 | 233 | |
b6820993 QY |
234 | As an alternative submission method, a patch can be mailed to the |
235 | development mailing list. Patches received on the mailing list will be | |
236 | picked up by Patchwork and tested against the latest development branch. | |
d1890d04 | 237 | |
b6820993 QY |
238 | The recommended way to send the patch (or series of NN patches) to the |
239 | list is by using ``git send-email`` as follows (assuming they are the N | |
240 | most recent commit(s) in your git history):: | |
d1890d04 | 241 | |
b6820993 | 242 | git send-email -NN --annotate --to=dev@lists.frrouting.org |
d1890d04 | 243 | |
b6820993 QY |
244 | If your commits do not already contain a ``Signed-off-by`` line, then |
245 | use the following command to add it (after making sure you agree to the | |
246 | Developer Certificate of Origin as outlined above):: | |
d1890d04 | 247 | |
b6820993 | 248 | git send-email -NN --annotate --signoff --to=dev@lists.frrouting.org |
d1890d04 | 249 | |
b6820993 QY |
250 | Submitting multi-commit patches as a GitHub pull request is **strongly |
251 | encouraged** and increases the probability of your patch getting reviewed and | |
252 | merged in a timely manner. | |
d1890d04 | 253 | |
b6820993 | 254 | .. _license-for-contributions: |
d1890d04 | 255 | |
b6820993 QY |
256 | License for Contributions |
257 | ------------------------- | |
258 | FRR is under a “GPLv2 or later” license. Any code submitted must be released | |
259 | under the same license (preferred) or any license which allows redistribution | |
260 | under this GPLv2 license (eg MIT License). | |
e2abcff8 PG |
261 | It is forbidden to push any code that prevents from using GPLv3 license. This |
262 | becomes a community rule, as FRR produces binaries that links with Apache 2.0 | |
263 | libraries. Apache 2.0 and GPLv2 license are incompatible, if put together. | |
264 | Please see `<http://www.apache.org/licenses/GPL-compatibility.html>`_ for | |
265 | more information. This rule guarantees the user to distribute FRR binary code | |
266 | without any licensing issues. | |
b22ba015 | 267 | |
b6820993 QY |
268 | Pre-submission Checklist |
269 | ------------------------ | |
270 | - Format code (see `Code Formatting <#code-formatting>`__) | |
271 | - Verify and acknowledge license (see :ref:`license-for-contributions`) | |
272 | - Ensure you have properly signed off (see :ref:`signing-off`) | |
273 | - Test building with various configurations: | |
d1890d04 | 274 | |
b6820993 | 275 | - ``buildtest.sh`` |
d1890d04 | 276 | |
b6820993 | 277 | - Verify building source distribution: |
d1890d04 | 278 | |
b6820993 | 279 | - ``make dist`` (and try rebuilding from the resulting tar file) |
d1890d04 | 280 | |
b6820993 | 281 | - Run unit tests: |
d1890d04 | 282 | |
b6820993 | 283 | - ``make test`` |
d1890d04 | 284 | |
b6820993 QY |
285 | - In the case of a major new feature or other significant change, document |
286 | plans for continued maintenance of the feature | |
d1890d04 | 287 | |
b6820993 | 288 | .. _signing-off: |
d1890d04 | 289 | |
b6820993 QY |
290 | Signing Off |
291 | ----------- | |
292 | Code submitted to FRR must be signed off. We have the same requirements for | |
293 | using the signed-off-by process as the Linux kernel. In short, you must include | |
294 | a ``Signed-off-by`` tag in every patch. | |
d1890d04 | 295 | |
118cf7ed SW |
296 | An easy way to do this is to use ``git commit -s`` where ``-s`` will automatically |
297 | append a signed-off line to the end of your commit message. Also, if you commit | |
298 | and forgot to add the line you can use ``git commit --amend -s`` to add the | |
299 | signed-off line to the last commit. | |
300 | ||
b6820993 QY |
301 | ``Signed-off-by`` is a developer's certification that they have the right to |
302 | submit the patch for inclusion into the project. It is an agreement to the | |
303 | :ref:`Developer's Certificate of Origin <developers-certificate-of-origin>`. | |
304 | Code without a proper ``Signed-off-by`` line cannot and will not be merged. | |
d1890d04 | 305 | |
b6820993 QY |
306 | If you are unfamiliar with this process, you should read the |
307 | `official policy at kernel.org <https://www.kernel.org/doc/html/latest/process/submitting-patches.html>`_. | |
308 | You might also find | |
309 | `this article <http://www.linuxfoundation.org/content/how-participate-linux-community-0>`_ | |
310 | about participating in the Linux community on the Linux Foundation website to | |
311 | be a helpful resource. | |
d1890d04 | 312 | |
b6820993 | 313 | .. _developers-certificate-of-origin: |
d1890d04 | 314 | |
b6820993 QY |
315 | In short, when you sign off on a commit, you assert your agreement to all of |
316 | the following:: | |
d1890d04 | 317 | |
b6820993 | 318 | Developer's Certificate of Origin 1.1 |
d1890d04 | 319 | |
b6820993 | 320 | By making a contribution to this project, I certify that: |
d1890d04 | 321 | |
b6820993 QY |
322 | (a) The contribution was created in whole or in part by me and I |
323 | have the right to submit it under the open source license | |
324 | indicated in the file; or | |
d1890d04 | 325 | |
b6820993 QY |
326 | (b) The contribution is based upon previous work that, to the best |
327 | of my knowledge, is covered under an appropriate open source | |
328 | license and I have the right under that license to submit that | |
329 | work with modifications, whether created in whole or in part by | |
330 | me, under the same open source license (unless I am permitted to | |
331 | submit under a different license), as indicated in the file; or | |
d1890d04 | 332 | |
b6820993 QY |
333 | (c) The contribution was provided directly to me by some other |
334 | person who certified (a), (b) or (c) and I have not modified it. | |
d1890d04 | 335 | |
b6820993 QY |
336 | (d) I understand and agree that this project and the contribution |
337 | are public and that a record of the contribution (including all | |
338 | personal information I submit with it, including my sign-off) is | |
339 | maintained indefinitely and may be redistributed consistent with | |
340 | this project or the open source license(s) involved. | |
d1890d04 | 341 | |
b6820993 | 342 | After Submitting Your Changes |
d1890d04 QY |
343 | ----------------------------- |
344 | ||
b6820993 | 345 | - Watch for Continuous Integration (CI) test results |
d1890d04 QY |
346 | |
347 | - You should automatically receive an email with the test results | |
348 | within less than 2 hrs of the submission. If you don’t get the | |
b6820993 | 349 | email, then check status on the GitHub pull request. |
d1890d04 | 350 | - Please notify the development mailing list if you think something |
b22ba015 | 351 | doesn't work. |
d1890d04 QY |
352 | |
353 | - If the tests failed: | |
354 | ||
355 | - In general, expect the community to ignore the submission until | |
356 | the tests pass. | |
357 | - It is up to you to fix and resubmit. | |
358 | ||
359 | - This includes fixing existing unit (“make test”) tests if your | |
360 | changes broke or changed them. | |
361 | - It also includes fixing distribution packages for the failing | |
362 | platforms (ie if new libraries are required). | |
363 | - Feel free to ask for help on the development list. | |
364 | ||
365 | - Go back to the submission process and repeat until the tests pass. | |
366 | ||
367 | - If the tests pass: | |
368 | ||
369 | - Wait for reviewers. Someone will review your code or be assigned | |
370 | to review your code. | |
493e3eed LB |
371 | - Respond to any comments or concerns the reviewer has. Use e-mail or |
372 | add a comment via github to respond or to let the reviewer know how | |
373 | their comment or concern is addressed. | |
374 | - An author must never delete or manually dismiss someone else's comments | |
375 | or review. (A review may be overridden by agreement in the weekly | |
376 | technical meeting.) | |
377 | - Automatically generated comments, e.g., those generated by CI systems, | |
378 | may be deleted by authors and others when such comments are not the most | |
22265b35 | 379 | recent results from that automated comment source. |
d1890d04 QY |
380 | - After all comments and concerns are addressed, expect your patch |
381 | to be merged. | |
382 | ||
383 | - Watch out for questions on the mailing list. At this time there will | |
384 | be a manual code review and further (longer) tests by various | |
385 | community members. | |
386 | - Your submission is done once it is merged to the master branch. | |
387 | ||
9de103f0 QY |
388 | Programming Languages, Tools and Libraries |
389 | ========================================== | |
390 | ||
391 | The core of FRR is written in C (gcc or clang supported) and makes | |
392 | use of GNU compiler extensions. A few non-essential scripts are | |
393 | implemented in Perl and Python. FRR requires the following tools | |
394 | to build distribution packages: automake, autoconf, texinfo, libtool and | |
395 | gawk and various libraries (i.e. libpam and libjson-c). | |
396 | ||
397 | If your contribution requires a new library or other tool, then please | |
398 | highlight this in your description of the change. Also make sure it’s | |
399 | supported by all FRR platform OSes or provide a way to build | |
400 | without the library (potentially without the new feature) on the other | |
401 | platforms. | |
402 | ||
403 | Documentation should be written in reStructuredText. Sphinx extensions may be | |
404 | utilized but pure ReST is preferred where possible. See | |
405 | :ref:`documentation`. | |
406 | ||
ca9dfee0 DL |
407 | Use of C++ |
408 | ---------- | |
409 | ||
410 | While C++ is not accepted for core components of FRR, extensions, modules or | |
411 | other distinct components may want to use C++ and include FRR header files. | |
412 | There is no requirement on contributors to work to retain C++ compatibility, | |
413 | but fixes for C++ compatibility are welcome. | |
414 | ||
415 | This implies that the burden of work to keep C++ compatibility is placed with | |
416 | the people who need it, and they may provide it at their leisure to the extent | |
417 | it is useful to them. So, if only a subset of header files, or even parts of | |
418 | a header file are made available to C++, this is perfectly fine. | |
419 | ||
590a7368 QY |
420 | Code Reviews |
421 | ============ | |
422 | ||
423 | Code quality is paramount for any large program. Consequently we require | |
424 | reviews of all submitted patches by at least one person other than the | |
425 | submitter before the patch is merged. | |
426 | ||
427 | Because of the nature of the software, FRR's maintainer list (i.e. those with | |
428 | commit permissions) tends to contain employees / members of various | |
429 | organizations. In order to prevent conflicts of interest, we use an honor | |
430 | system in which submissions from an individual representing one company should | |
431 | be merged by someone unaffiliated with that company. | |
432 | ||
433 | Guidelines for code review | |
924947e4 | 434 | -------------------------- |
590a7368 QY |
435 | |
436 | - As a rule of thumb, the depth of the review should be proportional to the | |
437 | scope and / or impact of the patch. | |
438 | ||
439 | - Anyone may review a patch. | |
440 | ||
441 | - When using GitHub reviews, marking "Approve" on a code review indicates | |
442 | willingness to merge the PR. | |
443 | ||
444 | - For individuals with merge rights, marking "Changes requested" is equivalent | |
445 | to a NAK. | |
446 | ||
447 | - For a PR you marked with "Changes requested", please respond to updates in a | |
448 | timely manner to avoid impeding the flow of development. | |
449 | ||
7e678379 LB |
450 | - Rejected or obsolete PRs are generally closed by the submitter based |
451 | on requests and/or agreement captured in a PR comment. The comment | |
452 | may originate with a reviewer or document agreement reached on Slack, | |
453 | the Development mailing list, or the weekly technical meeting. | |
454 | ||
590a7368 | 455 | |
b22ba015 | 456 | Coding Practices & Style |
9de103f0 | 457 | ======================== |
d1890d04 QY |
458 | |
459 | Commit messages | |
9de103f0 | 460 | --------------- |
d1890d04 QY |
461 | |
462 | Commit messages should be formatted in the same way as Linux kernel | |
b6820993 | 463 | commit messages. The format is roughly:: |
d1890d04 QY |
464 | |
465 | dir: short summary | |
466 | ||
467 | extended summary | |
468 | ||
b6820993 QY |
469 | ``dir`` should be the top level source directory under which the change was |
470 | made. For example, a change in :file:`bgpd/rfapi` would be formatted as:: | |
d1890d04 | 471 | |
9de103f0 | 472 | bgpd: short summary |
d1890d04 | 473 | |
b6820993 QY |
474 | ... |
475 | ||
476 | The first line should be no longer than 50 characters. Subsequent lines should | |
477 | be wrapped to 72 characters. | |
d1890d04 | 478 | |
b6820993 QY |
479 | You must also sign off on your commit. |
480 | ||
481 | .. seealso:: :ref:`signing-off` | |
482 | ||
483 | Source File Header | |
9de103f0 | 484 | ------------------ |
d1890d04 | 485 | |
b6820993 QY |
486 | New files must have a copyright header (see :ref:`license-for-contributions` |
487 | above) added to the file. The header should be: | |
d1890d04 | 488 | |
b6820993 | 489 | .. code-block:: c |
d1890d04 QY |
490 | |
491 | /* | |
492 | * Title/Function of file | |
493 | * Copyright (C) YEAR Author’s Name | |
494 | * | |
495 | * This program is free software; you can redistribute it and/or modify it | |
496 | * under the terms of the GNU General Public License as published by the Free | |
497 | * Software Foundation; either version 2 of the License, or (at your option) | |
498 | * any later version. | |
499 | * | |
500 | * This program is distributed in the hope that it will be useful, but WITHOUT | |
501 | * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | |
502 | * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for | |
503 | * more details. | |
504 | * | |
505 | * You should have received a copy of the GNU General Public License along | |
506 | * with this program; see the file COPYING; if not, write to the Free Software | |
507 | * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA | |
508 | */ | |
509 | ||
510 | #include <zebra.h> | |
511 | ||
b6820993 QY |
512 | Please copy-paste this header verbatim. In particular: |
513 | ||
514 | - Do not replace "This program" with "FRR" | |
515 | - Do not change the address of the FSF | |
516 | ||
517 | Adding Copyright Claims to Existing Files | |
9de103f0 | 518 | ----------------------------------------- |
d1890d04 | 519 | |
b6820993 QY |
520 | When adding copyright claims for modifications to an existing file, please |
521 | add a ``Portions:`` section as shown below. If this section already exists, add | |
522 | your new claim at the end of the list. | |
d1890d04 | 523 | |
b6820993 | 524 | .. code-block:: c |
d1890d04 | 525 | |
b6820993 QY |
526 | /* |
527 | * Title/Function of file | |
528 | * Copyright (C) YEAR Author’s Name | |
529 | * Portions: | |
530 | * Copyright (C) 2010 Entity A .... | |
531 | * Copyright (C) 2016 Your name [optional brief change description] | |
532 | * ... | |
533 | */ | |
d1890d04 | 534 | |
c964e511 | 535 | Code Formatting |
9de103f0 | 536 | --------------- |
d1890d04 | 537 | |
6f7a9254 QY |
538 | C Code |
539 | ^^^^^^ | |
540 | ||
541 | For C code, FRR uses Linux kernel style except where noted below. Code which | |
542 | does not comply with these style guidelines will not be accepted. | |
d1890d04 | 543 | |
281ba953 QY |
544 | The project provides multiple tools to allow you to correctly style your code |
545 | as painlessly as possible, primarily built around ``clang-format``. | |
546 | ||
547 | clang-format | |
548 | In the project root there is a :file:`.clang-format` configuration file | |
549 | which can be used with the ``clang-format`` source formatter tool from the | |
550 | LLVM project. Most of the time, this is the easiest and smartest tool to | |
551 | use. It can be run in a variety of ways. If you point it at a C source file | |
552 | or directory of source files, it will format all of them. In the LLVM source | |
553 | tree there are scripts that allow you to integrate it with ``git``, ``vim`` | |
554 | and ``emacs``, and there are third-party plugins for other editors. The | |
555 | ``git`` integration is particularly useful; suppose you have some changes in | |
556 | your git index. Then, with the integration installed, you can do the | |
557 | following: | |
558 | ||
559 | :: | |
560 | ||
561 | git clang-format | |
562 | ||
563 | This will format *only* the changes present in your index. If you have just | |
564 | made a few commits and would like to correctly style only the changes made | |
565 | in those commits, you can use the following syntax: | |
566 | ||
567 | :: | |
568 | ||
569 | git clang-format HEAD~X | |
570 | ||
571 | Where X is one more than the number of commits back from the tip of your | |
572 | branch you would like ``clang-format`` to look at (similar to specifying the | |
573 | target for a rebase). | |
574 | ||
575 | The ``vim`` plugin is particularly useful. It allows you to select lines in | |
576 | visual line mode and press a key binding to invoke ``clang-format`` on only | |
577 | those lines. | |
578 | ||
579 | When using ``clang-format``, it is recommended to use the latest version. | |
580 | Each consecutive version generally has better handling of various edge | |
581 | cases. You may notice on occasion that two consecutive runs of | |
582 | ``clang-format`` over the same code may result in changes being made on the | |
583 | second run. This is an unfortunate artifact of the tool. Please check with | |
584 | the kernel style guide if in doubt. | |
585 | ||
586 | One stylistic problem with the FRR codebase is the use of ``DEFUN`` macros | |
587 | for defining CLI commands. ``clang-format`` will happily format these macro | |
588 | invocations, but the result is often unsightly and difficult to read. | |
589 | Consequently, FRR takes a more relaxed position with how these are | |
590 | formatted. In general you should lean towards using the style exemplified in | |
591 | the section on :ref:`command-line-interface`. Because ``clang-format`` | |
592 | mangles this style, there is a Python script named ``tools/indent.py`` that | |
593 | wraps ``clang-format`` and handles ``DEFUN`` macros as well as some other | |
594 | edge cases specific to FRR. If you are submitting a new file, it is | |
595 | recommended to run that script over the new file, preferably after ensuring | |
596 | that the latest stable release of ``clang-format`` is in your ``PATH``. | |
597 | ||
598 | Documentation on ``clang-format`` and its various integrations is maintained | |
599 | on the LLVM website. | |
600 | ||
601 | https://clang.llvm.org/docs/ClangFormat.html | |
602 | ||
603 | checkpatch.sh | |
604 | In the Linux kernel source tree there is a Perl script used to check | |
605 | incoming patches for style errors. FRR uses an adapted version of this | |
606 | script for the same purpose. It can be found at | |
2780ae0c | 607 | :file:`tools/checkpatch.sh`. This script takes a git-formatted diff or |
281ba953 QY |
608 | patch file, applies it to a clean FRR tree, and inspects the result to catch |
609 | potential style errors. Running this script on your patches before | |
610 | submission is highly recommended. The CI system runs this script as well and | |
611 | will comment on the PR with the results if style errors are found. | |
612 | ||
b6820993 | 613 | It is run like this:: |
281ba953 | 614 | |
b6820993 | 615 | ./checkpatch.sh <patch> <tree> |
281ba953 QY |
616 | |
617 | Reports are generated on ``stderr`` and the exit code indicates whether | |
618 | issues were found (2, 1) or not (0). | |
619 | ||
620 | Where ``<patch>`` is the path to the diff or patch file and ``<tree>`` is | |
621 | the path to your FRR source tree. The tree should be on the branch that you | |
622 | intend to submit the patch against. The script will make a best-effort | |
623 | attempt to save the state of your working tree and index before applying the | |
624 | patch, and to restore it when it is done, but it is still recommended that | |
625 | you have a clean working tree as the script does perform a hard reset on | |
626 | your tree during its run. | |
627 | ||
628 | The script reports two classes of issues, namely WARNINGs and ERRORs. Please | |
629 | pay attention to both of them. The script will generally report WARNINGs | |
630 | where it cannot be 100% sure that a particular issue is real. In most cases | |
631 | WARNINGs indicate an issue that needs to be fixed. Sometimes the script will | |
632 | report false positives; these will be handled in code review on a | |
633 | case-by-case basis. Since the script only looks at changed lines, | |
634 | occasionally changing one part of a line can cause the script to report a | |
635 | style issue already present on that line that is unrelated to the change. | |
636 | When convenient it is preferred that these be cleaned up inline, but this is | |
637 | not required. | |
638 | ||
115e70a1 PZ |
639 | In general, a developer should heed the information reported by checkpatch. |
640 | However, some flexibility is needed for cases where human judgement yields | |
641 | better clarity than the script. Accordingly, it may be appropriate to | |
642 | ignore some checkpatch.sh warnings per discussion among the submitter(s) | |
643 | and reviewer(s) of a change. Misreporting of errors by the script is | |
d3c2e316 QY |
644 | possible. When this occurs, the exception should be handled either by |
645 | patching checkpatch to correct the false error report, or by documenting the | |
646 | exception in this document under :ref:`style-exceptions`. If the incorrect | |
647 | report is likely to appear again, a checkpatch update is preferred. | |
115e70a1 | 648 | |
281ba953 QY |
649 | If the script finds one or more WARNINGs it will exit with 1. If it finds |
650 | one or more ERRORs it will exit with 2. | |
651 | ||
652 | ||
653 | Please remember that while FRR provides these tools for your convenience, | |
654 | responsibility for properly formatting your code ultimately lies on the | |
655 | shoulders of the submitter. As such, it is recommended to double-check the | |
656 | results of these tools to avoid delays in merging your submission. | |
d1890d04 | 657 | |
115e70a1 PZ |
658 | In some cases, these tools modify or flag the format in ways that go beyond or |
659 | even conflict [#tool_style_conflicts]_ with the canonical documented Linux | |
660 | kernel style. In these cases, the Linux kernel style takes priority; | |
661 | non-canonical issues flagged by the tools are not compulsory but rather are | |
662 | opportunities for discussion among the submitter(s) and reviewer(s) of a change. | |
663 | ||
d1890d04 QY |
664 | **Whitespace changes in untouched parts of the code are not acceptable |
665 | in patches that change actual code.** To change/fix formatting issues, | |
666 | please create a separate patch that only does formatting changes and | |
667 | nothing else. | |
668 | ||
d1890d04 QY |
669 | Kernel and BSD styles are documented externally: |
670 | ||
671 | - https://www.kernel.org/doc/html/latest/process/coding-style.html | |
672 | - http://man.openbsd.org/style | |
673 | ||
674 | For GNU coding style, use ``indent`` with the following invocation: | |
675 | ||
676 | :: | |
677 | ||
678 | indent -nut -nfc1 file_for_submission.c | |
679 | ||
28ac5a03 QY |
680 | |
681 | Historically, FRR used fixed-width integral types that do not exist in any | |
682 | standard but were defined by most platforms at some point. Officially these | |
683 | types are not guaranteed to exist. Therefore, please use the fixed-width | |
684 | integral types introduced in the C99 standard when contributing new code to | |
685 | FRR. If you need to convert a large amount of code to use the correct types, | |
686 | there is a shell script in :file:`tools/convert-fixedwidth.sh` that will do the | |
687 | necessary replacements. | |
688 | ||
689 | +-----------+--------------------------+ | |
690 | | Incorrect | Correct | | |
691 | +===========+==========================+ | |
692 | | u_int8_t | uint8_t | | |
693 | +-----------+--------------------------+ | |
694 | | u_int16_t | uint16_t | | |
695 | +-----------+--------------------------+ | |
696 | | u_int32_t | uint32_t | | |
697 | +-----------+--------------------------+ | |
698 | | u_int64_t | uint64_t | | |
699 | +-----------+--------------------------+ | |
700 | | u_char | uint8_t or unsigned char | | |
701 | +-----------+--------------------------+ | |
702 | | u_short | unsigned short | | |
703 | +-----------+--------------------------+ | |
704 | | u_int | unsigned int | | |
705 | +-----------+--------------------------+ | |
706 | | u_long | unsigned long | | |
707 | +-----------+--------------------------+ | |
708 | ||
d3c2e316 QY |
709 | .. _style-exceptions: |
710 | ||
d1890d04 QY |
711 | Exceptions |
712 | ^^^^^^^^^^ | |
713 | ||
714 | FRR project code comes from a variety of sources, so there are some | |
715 | stylistic exceptions in place. They are organized here by branch. | |
716 | ||
9de103f0 QY |
717 | For ``master`` |
718 | """""""""""""" | |
d1890d04 QY |
719 | |
720 | BSD coding style applies to: | |
721 | ||
722 | - ``ldpd/`` | |
723 | ||
724 | ``babeld`` uses, approximately, the following style: | |
725 | ||
726 | - K&R style braces | |
727 | - Indents are 4 spaces | |
728 | - Function return types are on their own line | |
729 | ||
9de103f0 QY |
730 | For ``stable/3.0`` and ``stable/2.0`` |
731 | """"""""""""""""""""""""""""""""""""" | |
d1890d04 QY |
732 | |
733 | GNU coding style apply to the following parts: | |
734 | ||
735 | - ``lib/`` | |
736 | - ``zebra/`` | |
737 | - ``bgpd/`` | |
738 | - ``ospfd/`` | |
739 | - ``ospf6d/`` | |
740 | - ``isisd/`` | |
741 | - ``ripd/`` | |
742 | - ``ripngd/`` | |
743 | - ``vtysh/`` | |
744 | ||
745 | BSD coding style applies to: | |
746 | ||
747 | - ``ldpd/`` | |
748 | ||
6f7a9254 QY |
749 | YANG |
750 | ^^^^ | |
751 | ||
752 | FRR uses YANG to define data models for its northbound interface. YANG models | |
753 | should follow conventions used by the IETF standard models. From a practical | |
754 | standpoint, this corresponds to the output produced by the ``yanglint`` tool | |
755 | included in the ``libyang`` project, which is used by FRR to parse and validate | |
756 | YANG models. You should run the following command on all YANG documents you | |
757 | write: | |
758 | ||
759 | .. code-block:: console | |
760 | ||
761 | yanglint -f yang <model> | |
762 | ||
763 | The output of this command should be identical to the input file. The sole | |
764 | exception to this is comments. ``yanglint`` does not support comments and will | |
765 | strip them from its output. You may include comments in your YANG documents, | |
766 | but they should be indented appropriately (use spaces). Where possible, | |
767 | comments should be eschewed in favor of a suitable ``description`` statement. | |
768 | ||
769 | In short, a diff between your input file and the output of ``yanglint`` should | |
770 | either be empty or contain only comments. | |
d3c2e316 QY |
771 | |
772 | Specific Exceptions | |
773 | ^^^^^^^^^^^^^^^^^^^ | |
774 | ||
775 | Most of the time checkpatch errors should be corrected. Occasionally as a group | |
776 | maintainers will decide to ignore certain stylistic issues. Usually this is | |
777 | because correcting the issue is not possible without large unrelated code | |
778 | changes. When an exception is made, if it is unlikely to show up again and | |
779 | doesn't warrant an update to checkpatch, it is documented here. | |
780 | ||
781 | +------------------------------------------+---------------------------------------------------------------+ | |
782 | | Issue | Ignore Reason | | |
783 | +==========================================+===============================================================+ | |
784 | | DEFPY_HIDDEN, DEFPY_ATTR: complex macros | DEF* macros cannot be wrapped in parentheses without updating | | |
785 | | should be wrapped in parentheses | all usages of the macro, which would be highly disruptive. | | |
786 | +------------------------------------------+---------------------------------------------------------------+ | |
787 | ||
d1890d04 | 788 | Compile-time conditional code |
9de103f0 | 789 | ----------------------------- |
d1890d04 QY |
790 | |
791 | Many users access FRR via binary packages from 3rd party sources; | |
792 | compile-time code puts inclusion/exclusion in the hands of the package | |
793 | maintainer. Please think very carefully before making code conditional | |
794 | at compile time, as it increases regression testing, maintenance | |
795 | burdens, and user confusion. In particular, please avoid gratuitous | |
796 | ``--enable-…`` switches to the configure script - in general, code | |
797 | should be of high quality and in working condition, or it shouldn’t be | |
798 | in FRR at all. | |
799 | ||
800 | When code must be compile-time conditional, try have the compiler make | |
801 | it conditional rather than the C pre-processor so that it will still be | |
802 | checked by the compiler, even if disabled. For example, | |
803 | ||
804 | :: | |
805 | ||
806 | if (SOME_SYMBOL) | |
807 | frobnicate(); | |
808 | ||
809 | is preferred to | |
810 | ||
811 | :: | |
812 | ||
813 | #ifdef SOME_SYMBOL | |
814 | frobnicate (); | |
815 | #endif /* SOME_SYMBOL */ | |
816 | ||
b6820993 QY |
817 | Note that the former approach requires ensuring that ``SOME_SYMBOL`` will be |
818 | defined (watch your ``AC_DEFINE``\ s). | |
d1890d04 QY |
819 | |
820 | Debug-guards in code | |
9de103f0 | 821 | -------------------- |
d1890d04 | 822 | |
b6820993 QY |
823 | Debugging statements are an important methodology to allow developers to fix |
824 | issues found in the code after it has been released. The caveat here is that | |
825 | the developer must remember that people will be using the code at scale and in | |
826 | ways that can be unexpected for the original implementor. As such debugs | |
827 | **MUST** be guarded in such a way that they can be turned off. FRR has the | |
828 | ability to turn on/off debugs from the CLI and it is expected that the | |
829 | developer will use this convention to allow control of their debugs. | |
d1890d04 | 830 | |
81047bc5 DL |
831 | Custom syntax-like block macros |
832 | ------------------------------- | |
833 | ||
834 | FRR uses some macros that behave like the ``for`` or ``if`` C keywords. These | |
835 | macros follow these patterns: | |
836 | ||
837 | - loop-style macros are named ``frr_each_*`` (and ``frr_each``) | |
838 | - single run macros are named ``frr_with_*`` | |
839 | - to avoid confusion, ``frr_with_*`` macros must always use a ``{ ... }`` | |
840 | block even if the block only contains one statement. The ``frr_each`` | |
841 | constructs are assumed to be well-known enough to use normal ``for`` rules. | |
842 | - ``break``, ``return`` and ``goto`` all work correctly. For loop-style | |
843 | macros, ``continue`` works correctly too. | |
844 | ||
845 | Both the ``each`` and ``with`` keywords are inspired by other (more | |
846 | higher-level) programming languages that provide these constructs. | |
847 | ||
848 | There are also some older iteration macros, e.g. ``ALL_LIST_ELEMENTS`` and | |
849 | ``FOREACH_AFI_SAFI``. These macros in some cases do **not** fulfill the above | |
850 | pattern (e.g. ``break`` does not work in ``FOREACH_AFI_SAFI`` because it | |
851 | expands to 2 nested loops.) | |
852 | ||
9e001286 QY |
853 | Static Analysis and Sanitizers |
854 | ------------------------------ | |
81af0317 DL |
855 | Clang/LLVM and GCC come with a variety of tools that can be used to help find |
856 | bugs in FRR. | |
9e001286 QY |
857 | |
858 | clang-analyze | |
859 | This is a static analyzer that scans the source code looking for patterns | |
860 | that are likely to be bugs. The tool is run automatically on pull requests | |
861 | as part of CI and new static analysis warnings will be placed in the CI | |
862 | results. FRR aims for absolutely zero static analysis errors. While the | |
863 | project is not quite there, code that introduces new static analysis errors | |
864 | is very unlikely to be merged. | |
865 | ||
866 | AddressSanitizer | |
867 | This is an excellent tool that provides runtime instrumentation for | |
868 | detecting memory errors. As part of CI FRR is built with this | |
869 | instrumentation and run through a series of tests to look for any results. | |
870 | Testing your own code with this tool before submission is encouraged. You | |
871 | can enable it by passing:: | |
d5403d4f | 872 | |
9e001286 QY |
873 | --enable-address-sanitizer |
874 | ||
875 | to ``configure``. | |
876 | ||
877 | ThreadSanitizer | |
878 | Similar to AddressSanitizer, this tool provides runtime instrumentation for | |
879 | detecting data races. If you are working on or around multithreaded code, | |
880 | extensive testing with this instrumtation enabled is *highly* recommended. | |
881 | You can enable it by passing:: | |
d5403d4f | 882 | |
9e001286 QY |
883 | --enable-thread-sanitizer |
884 | ||
885 | to ``configure``. | |
886 | ||
887 | MemorySanitizer | |
888 | Similar to AddressSanitizer, this tool provides runtime instrumentation for | |
889 | detecting use of uninitialized heap memory. Testing your own code with this | |
890 | tool before submission is encouraged. You can enable it by passing:: | |
d5403d4f | 891 | |
9e001286 QY |
892 | --enable-memory-sanitizer |
893 | ||
894 | to ``configure``. | |
895 | ||
896 | All of the above tools are available in the Clang/LLVM toolchain since 3.4. | |
897 | AddressSanitizer and ThreadSanitizer are available in recent versions of GCC, | |
898 | but are no longer actively maintained. MemorySanitizer is not available in GCC. | |
899 | ||
81af0317 DL |
900 | .. note:: |
901 | ||
902 | The different Sanitizers are mostly incompatible with each other. Please | |
903 | refer to GCC/LLVM documentation for details. | |
904 | ||
9e001286 QY |
905 | Additionally, the FRR codebase is regularly scanned with Coverity. |
906 | Unfortunately Coverity does not have the ability to handle scanning pull | |
907 | requests, but after code is merged it will send an email notifying project | |
908 | members with Coverity access of newly introduced defects. | |
909 | ||
81af0317 DL |
910 | Executing non-installed dynamic binaries |
911 | ---------------------------------------- | |
912 | ||
913 | Since FRR uses the GNU autotools build system, it inherits its shortcomings. | |
914 | To execute a binary directly from the build tree under a wrapper like | |
915 | `valgrind`, `gdb` or `strace`, use:: | |
916 | ||
917 | ./libtool --mode=execute valgrind [--valgrind-opts] zebra/zebra [--zebra-opts] | |
918 | ||
919 | While replacing valgrind/zebra as needed. The `libtool` script is found in | |
920 | the root of the build directory after `./configure` has completed. Its purpose | |
921 | is to correctly set up `LD_LIBRARY_PATH` so that libraries from the build tree | |
922 | are used. (On some systems, `libtool` is also available from PATH, but this is | |
923 | not always the case.) | |
924 | ||
d1890d04 | 925 | CLI changes |
9de103f0 | 926 | ----------- |
d1890d04 | 927 | |
b6820993 QY |
928 | CLI's are a complicated ugly beast. Additions or changes to the CLI should use |
929 | a DEFUN to encapsulate one setting as much as is possible. Additionally as new | |
930 | DEFUN's are added to the system, documentation should be provided for the new | |
931 | commands. | |
d1890d04 QY |
932 | |
933 | Backwards Compatibility | |
9de103f0 | 934 | ----------------------- |
d1890d04 | 935 | |
b6820993 QY |
936 | As a general principle, changes to CLI and code in the lib/ directory should be |
937 | made in a backwards compatible fashion. This means that changes that are purely | |
938 | stylistic in nature should be avoided, e.g., renaming an existing macro or | |
939 | library function name without any functional change. When adding new parameters | |
940 | to common functions, it is also good to consider if this too should be done in | |
941 | a backward compatible fashion, e.g., by preserving the old form in addition to | |
d1890d04 QY |
942 | adding the new form. |
943 | ||
b6820993 QY |
944 | This is not to say that minor or even major functional changes to CLI and |
945 | common code should be avoided, but rather that the benefit gained from a change | |
946 | should be weighed against the added cost/complexity to existing code. Also, | |
947 | that when making such changes, it is good to preserve compatibility when | |
948 | possible to do so without introducing maintenance overhead/cost. It is also | |
949 | important to keep in mind, existing code includes code that may reside in | |
950 | private repositories (and is yet to be submitted) or code that has yet to be | |
951 | migrated from Quagga to FRR. | |
110bb121 | 952 | |
b6820993 QY |
953 | That said, compatibility measures can (and should) be removed when either: |
954 | ||
955 | - they become a significant burden, e.g. when data structures change and the | |
956 | compatibility measure would need a complex adaptation layer or becomes | |
957 | flat-out impossible | |
958 | - some measure of time (dependent on the specific case) has passed, so that | |
959 | the compatibility grace period is considered expired. | |
960 | ||
e12ea4bb QY |
961 | For CLI commands, the deprecation period is 1 year. |
962 | ||
b6820993 QY |
963 | In all cases, compatibility pieces should be marked with compiler/preprocessor |
964 | annotations to print warnings at compile time, pointing to the appropriate | |
965 | update path. A ``-Werror`` build should fail if compatibility bits are used. To | |
966 | avoid compilation issues in released code, such compiler/preprocessor | |
967 | annotations must be ignored non-development branches. For example: | |
968 | ||
969 | .. code-block:: c | |
970 | ||
e60dd6ca | 971 | #if CONFDATE > 20180403 |
b6820993 QY |
972 | CPP_NOTICE("Use of <XYZ> is deprecated, please use <ABC>") |
973 | #endif | |
d1890d04 | 974 | |
cab3f811 LB |
975 | Preferably, the shell script :file:`tools/fixup-deprecated.py` will be |
976 | updated along with making non-backwards compatible code changes, or an | |
977 | alternate script should be introduced, to update the code to match the | |
978 | change. When the script is updated, there is no need to preserve the | |
979 | deprecated code. Note that this does not apply to user interface | |
980 | changes, just internal code, macros and libraries. | |
981 | ||
d1890d04 | 982 | Miscellaneous |
9de103f0 | 983 | ------------- |
d1890d04 | 984 | |
b6820993 QY |
985 | When in doubt, follow the guidelines in the Linux kernel style guide, or ask on |
986 | the development mailing list / public Slack instance. | |
9de103f0 QY |
987 | |
988 | ||
989 | .. _documentation: | |
990 | ||
991 | Documentation | |
992 | ============= | |
993 | ||
994 | FRR uses Sphinx+RST as its documentation system. The document you are currently | |
995 | reading was generated by Sphinx from RST source in | |
996 | :file:`doc/developer/workflow.rst`. The documentation is structured as follows: | |
997 | ||
d5403d4f QY |
998 | +-----------------------+-------------------------------------------+ |
999 | | Directory | Contents | | |
1000 | +=======================+===========================================+ | |
1001 | | :file:`doc/user` | User documentation; configuration guides; | | |
1002 | | | protocol overviews | | |
1003 | +-----------------------+-------------------------------------------+ | |
1004 | | :file:`doc/developer` | Developer's documentation; API specs; | | |
1005 | | | datastructures; architecture overviews; | | |
1006 | | | project management procedure | | |
1007 | +-----------------------+-------------------------------------------+ | |
1008 | | :file:`doc/manpages` | Source for manpages | | |
1009 | +-----------------------+-------------------------------------------+ | |
1010 | | :file:`doc/figures` | Images and diagrams | | |
1011 | +-----------------------+-------------------------------------------+ | |
1012 | | :file:`doc/extra` | Miscellaneous Sphinx extensions, scripts, | | |
1013 | | | customizations, etc. | | |
1014 | +-----------------------+-------------------------------------------+ | |
1015 | ||
1016 | Each of these directories, with the exception of :file:`doc/figures` and | |
1017 | :file:`doc/extra`, contains a Sphinx-generated Makefile and configuration | |
1018 | script :file:`conf.py` used to set various document parameters. The makefile | |
1019 | can be used for a variety of targets; invoke `make help` in any of these | |
1020 | directories for a listing of available output formats. For convenience, there | |
1021 | is a top-level :file:`Makefile.am` that has targets for PDF and HTML | |
1022 | documentation for both developer and user documentation, respectively. That | |
1023 | makefile is also responsible for building manual pages packed with distribution | |
1024 | builds. | |
9de103f0 QY |
1025 | |
1026 | Indent and styling should follow existing conventions: | |
1027 | ||
1028 | - 3 spaces for indents under directives | |
1029 | - Cross references may contain only lowercase alphanumeric characters and | |
1030 | hyphens ('-') | |
1031 | - Lines wrapped to 80 characters where possible | |
1032 | ||
1033 | Characters for header levels should follow Python documentation guide: | |
1034 | ||
1035 | - ``#`` with overline, for parts | |
1036 | - ``*`` with overline, for chapters | |
1037 | - ``=``, for sections | |
1038 | - ``-``, for subsections | |
1039 | - ``^``, for subsubsections | |
1040 | - ``"``, for paragraphs | |
1041 | ||
1042 | After you have made your changes, please make sure that you can invoke | |
1043 | ``make latexpdf`` and ``make html`` with no warnings. | |
1044 | ||
1045 | The documentation is currently incomplete and needs love. If you find a broken | |
1046 | cross-reference, figure, dead hyperlink, style issue or any other nastiness we | |
1047 | gladly accept documentation patches. | |
1048 | ||
c91e9b8f QY |
1049 | To build the docs, please ensure you have installed a recent version of |
1050 | `Sphinx <http://www.sphinx-doc.org/en/stable/install.html>`_. If you want to | |
1051 | build LaTeX or PDF docs, you will also need a full LaTeX distribution | |
1052 | installed. | |
1053 | ||
9de103f0 QY |
1054 | Code |
1055 | ---- | |
1056 | ||
1057 | FRR is a large and complex software project developed by many different people | |
1058 | over a long period of time. Without adequate documentation, it can be | |
1059 | exceedingly difficult to understand code segments, APIs and other interfaces. | |
1060 | In the interest of keeping the project healthy and maintainable, you should | |
1061 | make every effort to document your code so that other people can understand | |
1062 | what it does without needing to closely read the code itself. | |
1063 | ||
1064 | Some specific guidelines that contributors should follow are: | |
1065 | ||
1066 | - Functions exposed in header files should have descriptive comments above | |
1067 | their signatures in the header file. At a minimum, a function comment should | |
1068 | contain information about the return value, parameters, and a general summary | |
1069 | of the function's purpose. Documentation on parameter values can be omitted | |
1070 | if it is (very) obvious what they are used for. | |
1071 | ||
1072 | Function comments must follow the style for multiline comments laid out in | |
1073 | the kernel style guide. | |
1074 | ||
1075 | Example: | |
1076 | ||
1077 | .. code-block:: c | |
1078 | ||
1079 | /* | |
1080 | * Determines whether or not a string is cool. | |
1081 | * | |
b6820993 QY |
1082 | * text |
1083 | * the string to check for coolness | |
1084 | * | |
1085 | * is_clccfc | |
1086 | * whether capslock is cruise control for cool | |
1087 | * | |
1088 | * Returns: | |
1089 | * 7 if the text is cool, 0 otherwise | |
9de103f0 QY |
1090 | */ |
1091 | int check_coolness(const char *text, bool is_clccfc); | |
1092 | ||
b6820993 QY |
1093 | Function comments should make it clear what parameters and return values are |
1094 | used for. | |
9de103f0 QY |
1095 | |
1096 | - Static functions should have descriptive comments in the same form as above | |
1097 | if what they do is not immediately obvious. Use good engineering judgement | |
1098 | when deciding whether a comment is necessary. If you are unsure, document | |
1099 | your code. | |
1100 | - Global variables, static or not, should have a comment describing their use. | |
1101 | - **For new code in lib/, these guidelines are hard requirements.** | |
1102 | ||
1103 | If you make significant changes to portions of the codebase covered in the | |
1104 | Developer's Manual, add a major subsystem or feature, or gain arcane mastery of | |
1105 | some undocumented or poorly documented part of the codebase, please document | |
1106 | your work so others can benefit. If you add a major feature or introduce a new | |
1107 | API, please document the architecture and API to the best of your abilities in | |
1108 | the Developer's Manual, using good judgement when choosing where to place it. | |
1109 | ||
1110 | Finally, if you come across some code that is undocumented and feel like | |
1111 | going above and beyond, document it! We absolutely appreciate and accept | |
1112 | patches that document previously undocumented code. | |
1113 | ||
1114 | User | |
1115 | ---- | |
1116 | ||
1117 | If you are contributing code that adds significant user-visible functionality | |
1118 | please document how to use it in :file:`doc/user`. Use good judgement when | |
1119 | choosing where to place documentation. For example, instructions on how to use | |
1120 | your implementation of a new BGP draft should go in the BGP chapter instead of | |
1121 | being its own chapter. If you are adding a new protocol daemon, please create a | |
1122 | new chapter. | |
1123 | ||
d5403d4f QY |
1124 | FRR Specific Markup |
1125 | ------------------- | |
1126 | ||
1127 | FRR has some customizations applied to the Sphinx markup that go a long way | |
1128 | towards making documentation easier to use, write and maintain. | |
1129 | ||
1130 | CLI Commands | |
1131 | ^^^^^^^^^^^^ | |
1132 | ||
9de103f0 QY |
1133 | When documenting CLI please use a combination of the ``.. index::`` and |
1134 | ``.. clicmd::`` directives. For example, the command :clicmd:`show pony` would | |
1135 | be documented as follows: | |
1136 | ||
1137 | .. code-block:: rest | |
1138 | ||
1139 | .. index:: show pony | |
1140 | .. clicmd:: show pony | |
1141 | ||
1142 | Prints an ASCII pony. Example output::: | |
1143 | ||
1144 | >>\. | |
1145 | /_ )`. | |
1146 | / _)`^)`. _.---. _ | |
1147 | (_,' \ `^-)"" `.\ | |
1148 | | | \ | |
1149 | \ / | | |
1150 | / \ /.___.'\ (\ (_ | |
1151 | < ,"|| \ |`. \`-' | |
1152 | \\ () )| )/ | |
1153 | hjw |_>|> /_] // | |
1154 | /_] /_] | |
1155 | ||
1156 | When documented this way, CLI commands can be cross referenced with the | |
1157 | ``:clicmd:`` inline markup like so: | |
1158 | ||
1159 | .. code-block:: rest | |
1160 | ||
1161 | :clicmd:`show pony` | |
1162 | ||
1163 | This is very helpful for users who want to quickly remind themselves what a | |
1164 | particular command does. | |
1165 | ||
d5403d4f QY |
1166 | Configuration Snippets |
1167 | ^^^^^^^^^^^^^^^^^^^^^^ | |
1168 | ||
1169 | When putting blocks of example configuration please use the | |
1170 | ``.. code-block::`` directive and specify ``frr`` as the highlighting language, | |
1171 | as in the following example. This will tell Sphinx to use a custom Pygments | |
1172 | lexer to highlight FRR configuration syntax. | |
1173 | ||
1174 | .. code-block:: rest | |
1175 | ||
1176 | .. code-block:: frr | |
1177 | ||
1178 | ! | |
1179 | ! Example configuration file. | |
1180 | ! | |
1181 | log file /tmp/log.log | |
1182 | service integrated-vtysh-config | |
1183 | ! | |
1184 | ip route 1.2.3.0/24 reject | |
1185 | ipv6 route de:ea:db:ee:ff::/64 reject | |
1186 | ! | |
1187 | ||
1188 | ||
9de103f0 QY |
1189 | .. _GitHub: https://github.com/frrouting/frr |
1190 | .. _GitHub issues: https://github.com/frrouting/frr/issues | |
115e70a1 PZ |
1191 | |
1192 | .. rubric:: Footnotes | |
1193 | ||
1194 | .. [#tool_style_conflicts] For example, lines over 80 characters are allowed | |
1195 | for text strings to make it possible to search the code for them: please | |
1196 | 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>`_ | |
1197 | and `Issue #1794 <https://github.com/FRRouting/frr/issues/1794>`_. |