Portal:DeveloperDocs/Patch submission guidelines

Submitting patches is one of the main ways to contributing to the Netfilter project. We try to follow common patterns in order to put some order in our work.

Sometimes reviewing patches and maintaining software written collaboratively can be difficult. We would like to keep the review process simple. Also, maintainers needs to understand what your change is about. In some cases, in the future, the patch may be reverted or reworked. Or simply, someone would need to understand it out of context. Take into account that there are people following the development only by reading commit messages and not the code itself. This is the reason that commit messages should be accurate.

That's why we have some rules we would like to highlight.

And in a nutshell, we use git and we follow Linux Kernel patch submission process and coding style.

Rules

Please, stick to these rules:

1. Every patch should be well formatted (see Commit style below).
2. Don't submit untested patches. You should test your patches and prove they work before sending them out. If you are sharing just an idea, untested, state that clearly in the commit message.
3. Put related patches in a patch series. This way maintainers know that they all go together.
4. Don't patch-bomb, i.e. don't submit 50 patches in a row. Split patch series in small chunks if possible.
5. Use one commit/patch for each logical change. Don't mix different things in the same patch. This eases the review process.
6. The repository should be keep consistent between commits, i.e. try to don't introduce changes that breaks the code for no reason.
7. Include documentation updates within the patch-series.
8. Include test-suites updates within the patch-series.
9. If you have doubts about formatting a patch, look at others commit/patches. There are plenty of examples. If you still have doubts, ask in the mailing list. But ask after doing your homework, i.e. trying yourself.
10. Use an email address which is reachable for replies, i.e. don't submit patches from root@myserver.example.com.
11. Be patient after submitting a patch. Don't expect same day replies. Ping after a week or so.
12. Always submit fixes first in a patch series. This way they could be applied independently of the rest of the series.

Commit style

A patch contains some metadata which is really important for us. This metadata is mainly 3 elements: title, description and Signed-off-by: lines.

Example:

src: fix use after free bug

Due to this code branch never being tested before, there was a hidden user-after-free bug.
Fix it now and include some test cases to execute this code path.

Signed-off-by: One Developer <onedeveloper@example.com>

Title

Should give context of what component is being updated. Just by reading the title, the reader should be able to know what you are touching.

A good idea is to follow this pattern:

component: subcomponent: message

If the patch is a fix, include the fix word as the first one in the message:

component: subcomponent: fix [...]

If you are touching the whole project code, use the project name as component:

nftables: switch all source code from C to Java

Other examples:

expr: drop unused code

evaluate: introduce extra step to verify whatever

main: config: cleanup warning messages

Message

The message of the patch should contain a complete explanation of what are you doing, since this may not be obvious from reading the code. You should justify the change as well. Include results of tests if applicable.

Some examples:

The code was not fast enough causing the system to block.
Solve this by implementing the algorithm foo.

Previous to this patch, an execution takes 100ms
After this patch, the same execution takes 10ms

Again, be explicit in this description. People unfamiliar with the codebase could be reading.

Footer lines

All commits should contain at least one 'Signed-off-by:' line. This comes directly from the Linux Kernel submission guidelines and indicates that you are responsible of the code you are submitting, among other things.

Other footer lines you should include, if applicable:

• Fixing a previous commit, you would likely need:

Fixes: be441e1ffdc24 ("src: add debugging mask to context structure")

• Fixing a bugzilla bug, you would use:

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1145

• If someone give additional testing to your patch:

Tested-by: Someone <someone@example.com>

• If someone ACK your change:

Acked-by: Someone <someone@example.com>

• If the change was suggested by other person

Suggested-by: Other Person <other@example.com>

• If you fix a bug thanks to the report of someone else, give credits to that person!

Reported-by: Someone Else <else@example.com>

Examples

These are great real-life examples of commit/patch:

http://git.netfilter.org/nftables/commit/?id=bada2f9c182dddf72a6d3b7b00c9eace7eb596c3

evaluate: merge nested set flags

A set may contain a nested set element definition, merge the nested set
flags so we don't hit:

 BUG: invalid data expression type range
 nft: netlink.c:400: netlink_gen_data: Assertion `0' failed.
 Aborted

With the following example ruleset:

 define dnat_ports      = { 1234-1567 }
 define port_allow      = {
        53,             # dns
        $dnat_ports,    # dnat
 }

 add rule x y tcp dport $port_allow accept

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1145
Fixes: a6b75b837f5e ("evaluate: set: Allow for set elems to be sets")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

http://git.netfilter.org/nftables/commit/?id=4ae0b6dc90d16b4d93a4e8b6703f23dcf2467b85

statement: fix print of ip dnat address

the change causes non-ipv6 addresses to not be printed at all in case
a nfproto was given.

Also add a test case to catch this.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1117
Fixes: 5ab0e10fc6e2c22363a ("src: support for RFC2732 IPv6 address format with brackets")
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Patch series

If you submit several changes in a row which are related, consider using a patch series. A cover letter is usually recommended, but not always required. You can skip the cover letter if the patch series is small (2-5 patches) and each individual patch is small and well explained.

Writing a cover letter is mandatory if your patch series introduces a really big change to the codebase.

Email submission

Git can email things directly if well configured. Please, read some docs on the topic (see External links).

Take into account that the email subject should contain a tag like this:

[PATCH] commit title

You should include the repository name in this tag:

[nft PATCH] commit title
[libnftnl PATCH] commit title
[conntrack-tools PATCH] commit title

Also, if you are in a series, the tag will include some additional numbers:

[nft PATCH 1/3] commit title
[nft PATCH 2/3] commit title
[libnftnl PATCH 3/3] commit title

If your patch is a RFC (just an idea, request for comments), include that as well:

[RFC nft PATCH 1/2] commit title
[RFC nft PATCH 2/2] commit title

If you version your patchset (i.e. changes are requested or whatever), state that as well:

[nft PATCH 1/2 v2] commit title
[nft PATCH 2/2 v2] commit title

In this last case, some changelog should be included before the diffstat included in the patch.

See also

Please, read this useful information:

• Bug reporting
• Git main website
• Linux Kernel docs - coding style
• Linux Kernel docs - submitting patches
• Development environment
• Contributing