maintainer-netdev.rst 23 KB

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