xref: /linux/Documentation/process/maintainer-netdev.rst (revision 1d227fcc72223cbdd34d0ce13541cbaab5e0d72f)
1.. SPDX-License-Identifier: GPL-2.0
2
3.. _netdev-FAQ:
4
5=============================
6Networking subsystem (netdev)
7=============================
8
9tl;dr
10-----
11
12 - designate your patch to a tree - ``[PATCH net]`` or ``[PATCH net-next]``
13 - for fixes the ``Fixes:`` tag is required, regardless of the tree
14 - don't post large series (> 15 patches), break them up
15 - don't repost your patches within one 24h period
16 - reverse xmas tree
17
18netdev
19------
20
21netdev is a mailing list for all network-related Linux stuff.  This
22includes anything found under net/ (i.e. core code like IPv6) and
23drivers/net (i.e. hardware specific drivers) in the Linux source tree.
24
25Note that some subsystems (e.g. wireless drivers) which have a high
26volume of traffic have their own specific mailing lists and trees.
27
28Like many other Linux mailing lists, the netdev list is hosted at
29kernel.org with archives available at https://lore.kernel.org/netdev/.
30
31Aside from subsystems like those mentioned above, all network-related
32Linux development (i.e. RFC, review, comments, etc.) takes place on
33netdev.
34
35Development cycle
36-----------------
37
38Here is a bit of background information on
39the cadence of Linux development.  Each new release starts off with a
40two week "merge window" where the main maintainers feed their new stuff
41to Linus for merging into the mainline tree.  After the two weeks, the
42merge window is closed, and it is called/tagged ``-rc1``.  No new
43features get mainlined after this -- only fixes to the rc1 content are
44expected.  After roughly a week of collecting fixes to the rc1 content,
45rc2 is released.  This repeats on a roughly weekly basis until rc7
46(typically; sometimes rc6 if things are quiet, or rc8 if things are in a
47state of churn), and a week after the last vX.Y-rcN was done, the
48official vX.Y is released.
49
50To find out where we are now in the cycle - load the mainline (Linus)
51page here:
52
53  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
54
55and note the top of the "tags" section.  If it is rc1, it is early in
56the dev cycle.  If it was tagged rc7 a week ago, then a release is
57probably imminent. If the most recent tag is a final release tag
58(without an ``-rcN`` suffix) - we are most likely in a merge window
59and ``net-next`` is closed.
60
61git trees and patch flow
62------------------------
63
64There are two networking trees (git repositories) in play.  Both are
65driven by David Miller, the main network maintainer.  There is the
66``net`` tree, and the ``net-next`` tree.  As you can probably guess from
67the names, the ``net`` tree is for fixes to existing code already in the
68mainline tree from Linus, and ``net-next`` is where the new code goes
69for the future release.  You can find the trees here:
70
71- https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
72- https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
73
74Relating that to kernel development: At the beginning of the 2-week
75merge window, the ``net-next`` tree will be closed - no new changes/features.
76The accumulated new content of the past ~10 weeks will be passed onto
77mainline/Linus via a pull request for vX.Y -- at the same time, the
78``net`` tree will start accumulating fixes for this pulled content
79relating to vX.Y
80
81An announcement indicating when ``net-next`` has been closed is usually
82sent to netdev, but knowing the above, you can predict that in advance.
83
84.. warning::
85  Do not send new ``net-next`` content to netdev during the
86  period during which ``net-next`` tree is closed.
87
88RFC patches sent for review only are obviously welcome at any time
89(use ``--subject-prefix='RFC net-next'`` with ``git format-patch``).
90
91Shortly after the two weeks have passed (and vX.Y-rc1 is released), the
92tree for ``net-next`` reopens to collect content for the next (vX.Y+1)
93release.
94
95If you aren't subscribed to netdev and/or are simply unsure if
96``net-next`` has re-opened yet, simply check the ``net-next`` git
97repository link above for any new networking-related commits.  You may
98also check the following website for the current status:
99
100  https://netdev.bots.linux.dev/net-next.html
101
102The ``net`` tree continues to collect fixes for the vX.Y content, and is
103fed back to Linus at regular (~weekly) intervals.  Meaning that the
104focus for ``net`` is on stabilization and bug fixes.
105
106Finally, the vX.Y gets released, and the whole cycle starts over.
107
108netdev patch review
109-------------------
110
111.. _patch_status:
112
113Patch status
114~~~~~~~~~~~~
115
116Status of a patch can be checked by looking at the main patchwork
117queue for netdev:
118
119  https://patchwork.kernel.org/project/netdevbpf/list/
120
121The "State" field will tell you exactly where things are at with your
122patch:
123
124================== =============================================================
125Patch state        Description
126================== =============================================================
127New, Under review  pending review, patch is in the maintainer’s queue for
128                   review; the two states are used interchangeably (depending on
129                   the exact co-maintainer handling patchwork at the time)
130Accepted           patch was applied to the appropriate networking tree, this is
131                   usually set automatically by the pw-bot
132Needs ACK          waiting for an ack from an area expert or testing
133Changes requested  patch has not passed the review, new revision is expected
134                   with appropriate code and commit message changes
135Rejected           patch has been rejected and new revision is not expected
136Not applicable     patch is expected to be applied outside of the networking
137                   subsystem
138Awaiting upstream  patch should be reviewed and handled by appropriate
139                   sub-maintainer, who will send it on to the networking trees;
140                   patches set to ``Awaiting upstream`` in netdev's patchwork
141                   will usually remain in this state, whether the sub-maintainer
142                   requested changes, accepted or rejected the patch
143Deferred           patch needs to be reposted later, usually due to dependency
144                   or because it was posted for a closed tree
145Superseded         new version of the patch was posted, usually set by the
146                   pw-bot
147RFC                not to be applied, usually not in maintainer’s review queue,
148                   pw-bot can automatically set patches to this state based
149                   on subject tags
150================== =============================================================
151
152Patches are indexed by the ``Message-ID`` header of the emails
153which carried them so if you have trouble finding your patch append
154the value of ``Message-ID`` to the URL above.
155
156Updating patch status
157~~~~~~~~~~~~~~~~~~~~~
158
159Contributors and reviewers do not have the permissions to update patch
160state directly in patchwork. Patchwork doesn't expose much information
161about the history of the state of patches, therefore having multiple
162people update the state leads to confusion.
163
164Instead of delegating patchwork permissions netdev uses a simple mail
165bot which looks for special commands/lines within the emails sent to
166the mailing list. For example to mark a series as Changes Requested
167one needs to send the following line anywhere in the email thread::
168
169  pw-bot: changes-requested
170
171As a result the bot will set the entire series to Changes Requested.
172This may be useful when author discovers a bug in their own series
173and wants to prevent it from getting applied.
174
175The use of the bot is entirely optional, if in doubt ignore its existence
176completely. Maintainers will classify and update the state of the patches
177themselves. No email should ever be sent to the list with the main purpose
178of communicating with the bot, the bot commands should be seen as metadata.
179
180The use of the bot is restricted to authors of the patches (the ``From:``
181header on patch submission and command must match!), maintainers of
182the modified code according to the MAINTAINERS file (again, ``From:``
183must match the MAINTAINERS entry) and a handful of senior reviewers.
184
185Bot records its activity here:
186
187  https://netdev.bots.linux.dev/pw-bot.html
188
189Review timelines
190~~~~~~~~~~~~~~~~
191
192Generally speaking, the patches get triaged quickly (in less than
19348h). But be patient, if your patch is active in patchwork (i.e. it's
194listed on the project's patch list) the chances it was missed are close to zero.
195
196The high volume of development on netdev makes reviewers move on
197from discussions relatively quickly. New comments and replies
198are very unlikely to arrive after a week of silence. If a patch
199is no longer active in patchwork and the thread went idle for more
200than a week - clarify the next steps and/or post the next version.
201
202For RFC postings specifically, if nobody responded in a week - reviewers
203either missed the posting or have no strong opinions. If the code is ready,
204repost as a PATCH.
205
206Emails saying just "ping" or "bump" are considered rude. If you can't figure
207out the status of the patch from patchwork or where the discussion has
208landed - describe your best guess and ask if it's correct. For example::
209
210  I don't understand what the next steps are. Person X seems to be unhappy
211  with A, should I do B and repost the patches?
212
213.. _Changes requested:
214
215Changes requested
216~~~~~~~~~~~~~~~~~
217
218Patches :ref:`marked<patch_status>` as ``Changes Requested`` need
219to be revised. The new version should come with a change log,
220preferably including links to previous postings, for example::
221
222  [PATCH net-next v3] net: make cows go moo
223
224  Even users who don't drink milk appreciate hearing the cows go "moo".
225
226  The amount of mooing will depend on packet rate so should match
227  the diurnal cycle quite well.
228
229  Signed-off-by: Joe Defarmer <joe@barn.org>
230  ---
231  v3:
232    - add a note about time-of-day mooing fluctuation to the commit message
233  v2: https://lore.kernel.org/netdev/123themessageid@barn.org/
234    - fix missing argument in kernel doc for netif_is_bovine()
235    - fix memory leak in netdev_register_cow()
236  v1: https://lore.kernel.org/netdev/456getstheclicks@barn.org/
237
238The commit message should be revised to answer any questions reviewers
239had to ask in previous discussions. Occasionally the update of
240the commit message will be the only change in the new version.
241
242Partial resends
243~~~~~~~~~~~~~~~
244
245Please always resend the entire patch series and make sure you do number your
246patches such that it is clear this is the latest and greatest set of patches
247that can be applied. Do not try to resend just the patches which changed.
248
249Handling misapplied patches
250~~~~~~~~~~~~~~~~~~~~~~~~~~~
251
252Occasionally a patch series gets applied before receiving critical feedback,
253or the wrong version of a series gets applied.
254
255Making the patch disappear once it is pushed out is not possible, the commit
256history in netdev trees is immutable.
257Please send incremental versions on top of what has been merged in order to fix
258the patches the way they would look like if your latest patch series was to be
259merged.
260
261In cases where full revert is needed the revert has to be submitted
262as a patch to the list with a commit message explaining the technical
263problems with the reverted commit. Reverts should be used as a last resort,
264when original change is completely wrong; incremental fixes are preferred.
265
266Stable tree
267~~~~~~~~~~~
268
269While it used to be the case that netdev submissions were not supposed
270to carry explicit ``CC: stable@vger.kernel.org`` tags that is no longer
271the case today. Please follow the standard stable rules in
272:ref:`Documentation/process/stable-kernel-rules.rst <stable_kernel_rules>`,
273and make sure you include appropriate Fixes tags!
274
275Security fixes
276~~~~~~~~~~~~~~
277
278Do not email netdev maintainers directly if you think you discovered
279a bug that might have possible security implications.
280The current netdev maintainer has consistently requested that
281people use the mailing lists and not reach out directly.  If you aren't
282OK with that, then perhaps consider mailing security@kernel.org or
283reading about http://oss-security.openwall.org/wiki/mailing-lists/distros
284as possible alternative mechanisms.
285
286
287Co-posting changes to user space components
288~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
289
290User space code exercising kernel features should be posted
291alongside kernel patches. This gives reviewers a chance to see
292how any new interface is used and how well it works.
293
294When user space tools reside in the kernel repo itself all changes
295should generally come as one series. If series becomes too large
296or the user space project is not reviewed on netdev include a link
297to a public repo where user space patches can be seen.
298
299In case user space tooling lives in a separate repository but is
300reviewed on netdev  (e.g. patches to ``iproute2`` tools) kernel and
301user space patches should form separate series (threads) when posted
302to the mailing list, e.g.::
303
304  [PATCH net-next 0/3] net: some feature cover letter
305   └─ [PATCH net-next 1/3] net: some feature prep
306   └─ [PATCH net-next 2/3] net: some feature do it
307   └─ [PATCH net-next 3/3] selftest: net: some feature
308
309  [PATCH iproute2-next] ip: add support for some feature
310
311Posting as one thread is discouraged because it confuses patchwork
312(as of patchwork 2.2.2).
313
314Preparing changes
315-----------------
316
317Attention to detail is important.  Re-read your own work as if you were the
318reviewer.  You can start with using ``checkpatch.pl``, perhaps even with
319the ``--strict`` flag.  But do not be mindlessly robotic in doing so.
320If your change is a bug fix, make sure your commit log indicates the
321end-user visible symptom, the underlying reason as to why it happens,
322and then if necessary, explain why the fix proposed is the best way to
323get things done.  Don't mangle whitespace, and as is common, don't
324mis-indent function arguments that span multiple lines.  If it is your
325first patch, mail it to yourself so you can test apply it to an
326unpatched tree to confirm infrastructure didn't mangle it.
327
328Finally, go back and read
329:ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
330to be sure you are not repeating some common mistake documented there.
331
332Indicating target tree
333~~~~~~~~~~~~~~~~~~~~~~
334
335To help maintainers and CI bots you should explicitly mark which tree
336your patch is targeting. Assuming that you use git, use the prefix
337flag::
338
339  git format-patch --subject-prefix='PATCH net-next' start..finish
340
341Use ``net`` instead of ``net-next`` (always lower case) in the above for
342bug-fix ``net`` content.
343
344Dividing work into patches
345~~~~~~~~~~~~~~~~~~~~~~~~~~
346
347Put yourself in the shoes of the reviewer. Each patch is read separately
348and therefore should constitute a comprehensible step towards your stated
349goal.
350
351Avoid sending series longer than 15 patches. Larger series takes longer
352to review as reviewers will defer looking at it until they find a large
353chunk of time. A small series can be reviewed in a short time, so Maintainers
354just do it. As a result, a sequence of smaller series gets merged quicker and
355with better review coverage. Re-posting large series also increases the mailing
356list traffic.
357
358.. _rcs:
359
360Local variable ordering ("reverse xmas tree", "RCS")
361~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
362
363Netdev has a convention for ordering local variables in functions.
364Order the variable declaration lines longest to shortest, e.g.::
365
366  struct scatterlist *sg;
367  struct sk_buff *skb;
368  int err, i;
369
370If there are dependencies between the variables preventing the ordering
371move the initialization out of line.
372
373Format precedence
374~~~~~~~~~~~~~~~~~
375
376When working in existing code which uses nonstandard formatting make
377your code follow the most recent guidelines, so that eventually all code
378in the domain of netdev is in the preferred format.
379
380Using device-managed and cleanup.h constructs
381~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
382
383Netdev remains skeptical about promises of all "auto-cleanup" APIs,
384including even ``devm_`` helpers, historically. They are not the preferred
385style of implementation, merely an acceptable one.
386
387Use of ``guard()`` is discouraged within any function longer than 20 lines,
388``scoped_guard()`` is considered more readable. Using normal lock/unlock is
389still (weakly) preferred.
390
391Low level cleanup constructs (such as ``__free()``) can be used when building
392APIs and helpers, especially scoped iterators. However, direct use of
393``__free()`` within networking core and drivers is discouraged.
394Similar guidance applies to declaring variables mid-function.
395
396Clean-up patches
397~~~~~~~~~~~~~~~~
398
399Netdev discourages patches which perform simple clean-ups, which are not in
400the context of other work. For example:
401
402* Addressing ``checkpatch.pl`` warnings
403* Addressing :ref:`Local variable ordering<rcs>` issues
404* Conversions to device-managed APIs (``devm_`` helpers)
405
406This is because it is felt that the churn that such changes produce comes
407at a greater cost than the value of such clean-ups.
408
409Conversely, spelling and grammar fixes are not discouraged.
410
411Resending after review
412~~~~~~~~~~~~~~~~~~~~~~
413
414Allow at least 24 hours to pass between postings. This will ensure reviewers
415from all geographical locations have a chance to chime in. Do not wait
416too long (weeks) between postings either as it will make it harder for reviewers
417to recall all the context.
418
419Make sure you address all the feedback in your new posting. Do not post a new
420version of the code if the discussion about the previous version is still
421ongoing, unless directly instructed by a reviewer.
422
423The new version of patches should be posted as a separate thread,
424not as a reply to the previous posting. Change log should include a link
425to the previous posting (see :ref:`Changes requested`).
426
427Testing
428-------
429
430Expected level of testing
431~~~~~~~~~~~~~~~~~~~~~~~~~
432
433At the very minimum your changes must survive an ``allyesconfig`` and an
434``allmodconfig`` build with ``W=1`` set without new warnings or failures.
435
436Ideally you will have done run-time testing specific to your change,
437and the patch series contains a set of kernel selftest for
438``tools/testing/selftests/net`` or using the KUnit framework.
439
440You are expected to test your changes on top of the relevant networking
441tree (``net`` or ``net-next``) and not e.g. a stable tree or ``linux-next``.
442
443patchwork checks
444~~~~~~~~~~~~~~~~
445
446Checks in patchwork are mostly simple wrappers around existing kernel
447scripts, the sources are available at:
448
449https://github.com/linux-netdev/nipa/tree/master/tests
450
451**Do not** post your patches just to run them through the checks.
452You must ensure that your patches are ready by testing them locally
453before posting to the mailing list. The patchwork build bot instance
454gets overloaded very easily and netdev@vger really doesn't need more
455traffic if we can help it.
456
457netdevsim
458~~~~~~~~~
459
460``netdevsim`` is a test driver which can be used to exercise driver
461configuration APIs without requiring capable hardware.
462Mock-ups and tests based on ``netdevsim`` are strongly encouraged when
463adding new APIs, but ``netdevsim`` in itself is **not** considered
464a use case/user. You must also implement the new APIs in a real driver.
465
466We give no guarantees that ``netdevsim`` won't change in the future
467in a way which would break what would normally be considered uAPI.
468
469``netdevsim`` is reserved for use by upstream tests only, so any
470new ``netdevsim`` features must be accompanied by selftests under
471``tools/testing/selftests/``.
472
473Reviewer guidance
474-----------------
475
476Reviewing other people's patches on the list is highly encouraged,
477regardless of the level of expertise. For general guidance and
478helpful tips please see :ref:`development_advancedtopics_reviews`.
479
480It's safe to assume that netdev maintainers know the community and the level
481of expertise of the reviewers. The reviewers should not be concerned about
482their comments impeding or derailing the patch flow.
483
484Less experienced reviewers are highly encouraged to do more in-depth
485review of submissions and not focus exclusively on trivial or subjective
486matters like code formatting, tags etc.
487
488Testimonials / feedback
489-----------------------
490
491Some companies use peer feedback in employee performance reviews.
492Please feel free to request feedback from netdev maintainers,
493especially if you spend significant amount of time reviewing code
494and go out of your way to improve shared infrastructure.
495
496The feedback must be requested by you, the contributor, and will always
497be shared with you (even if you request for it to be submitted to your
498manager).
499