X-Git-Url: http://git.droids-corp.org/?a=blobdiff_plain;f=doc%2Fguides%2Fcontributing%2Fcoding_style.rst;h=7601162c4f6750cf98c86b441237b1fd4f14e943;hb=a8f0df6bf98d295668b429f65884af887c2c5b77;hp=4163960c96c7f8d07533d06c566034785fd08111;hpb=d76a592782c1291b19f74a8ec29bcbd40b5170cd;p=dpdk.git diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst index 4163960c96..7601162c4f 100644 --- a/doc/guides/contributing/coding_style.rst +++ b/doc/guides/contributing/coding_style.rst @@ -1,3 +1,6 @@ +.. SPDX-License-Identifier: BSD-3-Clause + Copyright 2018 The DPDK contributors + .. _coding_style: DPDK Coding Style @@ -51,8 +54,13 @@ To document a public API, a doxygen-like format must be used: refer to :ref:`dox License Header ~~~~~~~~~~~~~~ -Each file should begin with a special comment containing the appropriate copyright and license for the file. -Generally this is the BSD License, except for code for Linux Kernel modules. +Each file must begin with a special comment containing the +`Software Package Data Exchange (SPDX) License Identfier `_. + +Generally this is the BSD License, except for code granted special exceptions. +The SPDX licences identifier is sufficient, a file should not contain +an additional text version of the license (boilerplate). + After any copyright header, a blank line should be left before any other contents, e.g. include statements in a C file. C Preprocessor Directives @@ -244,6 +252,15 @@ Structure Declarations * Use of the structures should be by separate variable declarations and those declarations must be extern if they are declared in a header file. * Externally visible structure definitions should have the structure name prefixed by ``rte_`` to avoid namespace collisions. +.. note:: + + Uses of ``bool`` in structures are not preferred as is wastes space and + it's also not clear as to what type size the bool is. A preferred use of + ``bool`` is mainly as a return type from functions that return true/false, + and maybe local variable functions. + + Ref: `LKML `_ + Queues ~~~~~~ @@ -266,6 +283,29 @@ Thus, the previous example would be better written: DPDK also provides an optimized way to store elements in lockless rings. This should be used in all data-path code, when there are several consumer and/or producers to avoid locking for concurrent access. +Naming +------ + +For symbol names and documentation, new usage of +'master / slave' (or 'slave' independent of 'master') and 'blacklist / +whitelist' is not allowed. + +Recommended replacements for 'master / slave' are: + '{primary,main} / {secondary,replica,subordinate}' + '{initiator,requester} / {target,responder}' + '{controller,host} / {device,worker,proxy}' + 'leader / follower' + 'director / performer' + +Recommended replacements for 'blacklist/whitelist' are: + 'denylist / allowlist' + 'blocklist / passlist' + +Exceptions for introducing new usage is to maintain compatibility +with an existing (as of 2020) hardware or protocol +specification that mandates those terms. + + Typedefs ~~~~~~~~ @@ -322,7 +362,7 @@ For example: typedef int (lcore_function_t)(void *); /* launch a function of lcore_function_t type */ - int rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned slave_id); + int rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned worker_id); C Indentation @@ -603,22 +643,30 @@ In the DPDK environment, use the logging interface provided: .. code-block:: c - #define RTE_LOGTYPE_TESTAPP1 RTE_LOGTYPE_USER1 - #define RTE_LOGTYPE_TESTAPP2 RTE_LOGTYPE_USER2 + /* register log types for this application */ + int my_logtype1 = rte_log_register("myapp.log1"); + int my_logtype2 = rte_log_register("myapp.log2"); + + /* set global log level to INFO */ + rte_log_set_global_level(RTE_LOG_INFO); + + /* only display messages higher than NOTICE for log2 (default + * is DEBUG) */ + rte_log_set_level(my_logtype2, RTE_LOG_NOTICE); - /* enable these logs type */ - rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1); - rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1); + /* enable all PMD logs (whose identifier string starts with "pmd.") */ + rte_log_set_level_pattern("pmd.*", RTE_LOG_DEBUG); /* log in debug level */ - rte_set_log_level(RTE_LOG_DEBUG); - RTE_LOG(DEBUG, TESTAPP1, "this is is a debug level message\n"); - RTE_LOG(INFO, TESTAPP1, "this is is a info level message\n"); - RTE_LOG(WARNING, TESTAPP1, "this is is a warning level message\n"); + rte_log_set_global_level(RTE_LOG_DEBUG); + RTE_LOG(DEBUG, my_logtype1, "this is a debug level message\n"); + RTE_LOG(INFO, my_logtype1, "this is a info level message\n"); + RTE_LOG(WARNING, my_logtype1, "this is a warning level message\n"); + RTE_LOG(WARNING, my_logtype2, "this is a debug level message (not displayed)\n"); /* log in info level */ - rte_set_log_level(RTE_LOG_INFO); - RTE_LOG(DEBUG, TESTAPP2, "debug level message (not displayed)\n"); + rte_log_set_global_level(RTE_LOG_INFO); + RTE_LOG(DEBUG, my_logtype1, "debug level message (not displayed)\n"); Branch Prediction ~~~~~~~~~~~~~~~~~ @@ -686,11 +734,327 @@ Control Statements /* NOTREACHED */ } +Dynamic Logging +--------------- + +DPDK provides infrastructure to perform logging during runtime. This is very +useful for enabling debug output without recompilation. To enable or disable +logging of a particular topic, the ``--log-level`` parameter can be provided +to EAL, which will change the log level. DPDK code can register topics, +which allows the user to adjust the log verbosity for that specific topic. + +In general, the naming scheme is as follows: ``type.section.name`` + + * Type is the type of component, where ``lib``, ``pmd``, ``bus`` and ``user`` + are the common options. + * Section refers to a specific area, for example a poll-mode-driver for an + ethernet device would use ``pmd.net``, while an eventdev PMD uses + ``pmd.event``. + * The name identifies the individual item that the log applies to. + The name section must align with + the directory that the PMD code resides. See examples below for clarity. + +Examples: + + * The virtio network PMD in ``drivers/net/virtio`` uses ``pmd.net.virtio`` + * The eventdev software poll mode driver in ``drivers/event/sw`` uses ``pmd.event.sw`` + * The octeontx mempool driver in ``drivers/mempool/octeontx`` uses ``pmd.mempool.octeontx`` + * The DPDK hash library in ``lib/hash`` uses ``lib.hash`` + +Specializations +~~~~~~~~~~~~~~~ + +In addition to the above logging topic, any PMD or library can further split +logging output by using "specializations". A specialization could be the +difference between initialization code, and logs of events that occur at runtime. + +An example could be the initialization log messages getting one +specialization, while another specialization handles mailbox command logging. +Each PMD, library or component can create as many specializations as required. + +A specialization looks like this: + + * Initialization output: ``type.section.name.init`` + * PF/VF mailbox output: ``type.section.name.mbox`` + +A real world example is the i40e poll mode driver which exposes two +specializations, one for initialization ``pmd.net.i40e.init`` and the other for +the remaining driver logs ``pmd.net.i40e.driver``. + +Note that specializations have no formatting rules, but please follow +a precedent if one exists. In order to see all current log topics and +specializations, run the ``app/test`` binary, and use the ``dump_log_types`` Python Code ----------- -All Python code should work with Python 2.7+ and 3.2+ and be compliant with +All Python code should be compliant with `PEP8 (Style Guide for Python Code) `_. The ``pep8`` tool can be used for testing compliance with the guidelines. + +Integrating with the Build System +--------------------------------- + +DPDK is built using the tools ``meson`` and ``ninja``. + +.. note:: + + In order to catch possible issues as soon as possible, + it is recommended that developers build DPDK in "developer mode" to enable additional checks. + By default, this mode is enabled if the build is being done from a git checkout, + but the mode can be manually enabled/disabled using the + ``developer_mode`` meson configuration option. + +Therefore all new component additions should include a ``meson.build`` file, +and should be added to the component lists in the ``meson.build`` files in the +relevant top-level directory: +either ``lib`` directory or a ``driver`` subdirectory. + +Meson Build File Contents - Libraries +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``meson.build`` file for a new DPDK library should be of the following basic +format. + +.. code-block:: python + + sources = files('file1.c', ...) + headers = files('file1.h', ...) + + +This will build based on a number of conventions and assumptions within the DPDK +itself, for example, that the library name is the same as the directory name in +which the files are stored. + +For a library ``meson.build`` file, there are number of variables which can be +set, some mandatory, others optional. The mandatory fields are: + +sources + **Default Value = []**. + This variable should list out the files to be compiled up to create the + library. Files must be specified using the meson ``files()`` function. + + +The optional fields are: + +build + **Default Value = true** + Used to optionally compile a library, based on its dependencies or + environment. When set to "false" the ``reason`` value, explained below, should + also be set to explain to the user why the component is not being built. + A simple example of use would be: + +.. code-block:: python + + if not is_linux + build = false + reason = 'only supported on Linux' + endif + + +cflags + **Default Value = [<-march/-mcpu flags>]**. + Used to specify any additional cflags that need to be passed to compile + the sources in the library. + +deps + **Default Value = ['eal']**. + Used to list the internal library dependencies of the library. It should + be assigned to using ``+=`` rather than overwriting using ``=``. The + dependencies should be specified as strings, each one giving the name of + a DPDK library, without the ``librte_`` prefix. Dependencies are handled + recursively, so specifying e.g. ``mempool``, will automatically also + make the library depend upon the mempool library's dependencies too - + ``ring`` and ``eal``. For libraries that only depend upon EAL, this + variable may be omitted from the ``meson.build`` file. For example: + +.. code-block:: python + + deps += ['ethdev'] + + +ext_deps + **Default Value = []**. + Used to specify external dependencies of this library. They should be + returned as dependency objects, as returned from the meson + ``dependency()`` or ``find_library()`` functions. Before returning + these, they should be checked to ensure the dependencies have been + found, and, if not, the ``build`` variable should be set to ``false``. + For example: + +.. code-block:: python + + my_dep = dependency('libX', required: 'false') + if my_dep.found() + ext_deps += my_dep + else + build = false + endif + + +headers + **Default Value = []**. + Used to return the list of header files for the library that should be + installed to $PREFIX/include when ``ninja install`` is run. As with + source files, these should be specified using the meson ``files()`` + function. + When ``check_includes`` build option is set to ``true``, each header file + has additional checks performed on it, for example to ensure that it is + not missing any include statements for dependent headers. + For header files which are public, but only included indirectly in + applications, these checks can be skipped by using the ``indirect_headers`` + variable rather than ``headers``. + +indirect_headers + **Default Value = []**. + As with ``headers`` option above, except that the files are not checked + for all needed include files as part of a DPDK build when + ``check_includes`` is set to ``true``. + +includes: + **Default Value = []**. + Used to indicate any additional header file paths which should be + added to the header search path for other libs depending on this + library. EAL uses this so that other libraries building against it + can find the headers in subdirectories of the main EAL directory. The + base directory of each library is always given in the include path, + it does not need to be specified here. + +name + **Default Value = library name derived from the directory name**. + If a library's .so or .a file differs from that given in the directory + name, the name should be specified using this variable. In practice, + since the convention is that for a library called ``librte_xyz.so``, the + sources are stored in a directory ``lib/xyz``, this value should + never be needed for new libraries. + +.. note:: + + The name value also provides the name used to find the function version + map file, as part of the build process, so if the directory name and + library names differ, the ``version.map`` file should be named + consistently with the library, not the directory + +objs + **Default Value = []**. + This variable can be used to pass to the library build some pre-built + objects that were compiled up as part of another target given in the + included library ``meson.build`` file. + +reason + **Default Value = ''**. + This variable should be used when a library is not to be built i.e. when + ``build`` is set to "false", to specify the reason why a library will not be + built. For missing dependencies this should be of the form + ``'missing dependency, "libname"'``. + +use_function_versioning + **Default Value = false**. + Specifies if the library in question has ABI versioned functions. If it + has, this value should be set to ensure that the C files are compiled + twice with suitable parameters for each of shared or static library + builds. + +Meson Build File Contents - Drivers +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +For drivers, the values are largely the same as for libraries. The variables +supported are: + +build + As above. + +cflags + As above. + +deps + As above. + +ext_deps + As above. + +includes + **Default Value = ** Some drivers include a base + directory for additional source files and headers, so we have this + variable to allow the headers from that base directory to be found when + compiling driver sources. Should be appended to using ``+=`` rather than + overwritten using ``=``. The values appended should be meson include + objects got using the ``include_directories()`` function. For example: + +.. code-block:: python + + includes += include_directories('base') + +name + As above, though note that each driver class can define it's own naming + scheme for the resulting ``.so`` files. + +objs + As above, generally used for the contents of the ``base`` directory. + +pkgconfig_extra_libs + **Default Value = []** + This variable is used to pass additional library link flags through to + the DPDK pkgconfig file generated, for example, to track any additional + libraries that may need to be linked into the build - especially when + using static libraries. Anything added here will be appended to the end + of the ``pkgconfig --libs`` output. + +reason + As above. + +sources [mandatory] + As above + +headers + As above + +version + As above + + +Meson Coding Style +------------------ + +The following guidelines apply to the build system code in meson.build files in DPDK. + +* Indentation should be using 4 spaces, no hard tabs. + +* Line continuations should be doubly-indented to ensure visible difference from normal indentation. + Any line continuations beyond the first may be singly indented to avoid large amounts of indentation. + +* Lists of files or components must be alphabetical unless doing so would cause errors. + +* Two formats are supported for lists of files or list of components: + + * For a small number of list entries, generally 3 or fewer, all elements may be put on a single line. + In this case, the opening and closing braces of the list must be on the same line as the list items. + No trailing comma is put on the final list entry. + * For lists with more than 3 items, + it is recommended that the lists be put in the files with a *single* entry per line. + In this case, the opening brace, or ``files`` function call must be on a line on its own, + and the closing brace must similarly be on a line on its own at the end. + To help with readability of nested sublists, the closing brace should be dedented to appear + at the same level as the opening braced statement. + The final list entry must have a trailing comma, + so that adding a new entry to the list never modifies any other line in the list. + +Examples:: + + sources = files('file1.c', 'file2.c') + + subdirs = ['dir1', 'dir2'] + + headers = files( + 'header1.c', + 'header2.c', + 'header3.c', # always include trailing comma + ) # closing brace at indent level of opening brace + + components = [ + 'comp1', + 'comp2', + ... + 'compN', + ]