CoDel, PIE, FQ-CoDel and FQ-PIE AQM for Dummynet included in FreeBSD 11 and back ported to FreeBSD 10.3. However, their usage syntax and options / parameters have not been documented in ipfw man page.
This patch adds description, parameters, options, sysctl and examples of using these AQMs to ipfw man page.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Thank you for working on updating the man page.
Can you run "mandoc -Tlint" over the man page with your changes and fix the warnings ("new sentence, new line" and such)?
Running textproc/igor over the man page can also find and report issues.
The FDP primer's man page section on Markup Guidelines has some additional info that can help.
Let us know if you need help or have questions.
Comment Actions
Thank you for reviewing the patch. I have revised that patch and checked it using mandoc -Tlint and igor. Hopefully, it is OK now.
Comment Actions
Looks good documentation-wise, can't comment much on the actual content, just the formatting.
Comment Actions
Overall, quite good: thank you. Above are mostly nits. I note, though that when I tried applying the patch to src/sbin/ipfw/ipfw.8 @338406, I got one rejection:
g1-215(11.2-S)[18] patch -C -p 2 < D12507.diff Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |Index: sbin/ipfw/ipfw.8 |=================================================================== |--- sbin/ipfw/ipfw.8 |+++ sbin/ipfw/ipfw.8 -------------------------- Patching file ipfw.8 using Plan A... Hunk #1 succeeded at 2711 (offset 179 lines). Hunk #2 succeeded at 2738 (offset 179 lines). Hunk #3 succeeded at 2929 (offset 179 lines). Hunk #4 succeeded at 3471 (offset 228 lines). Hunk #5 succeeded at 3552 (offset 179 lines). Hunk #6 failed at 4077. Hunk #7 succeeded at 4299 (offset 271 lines). Hunk #8 succeeded at 4243 (offset 179 lines). 1 out of 8 hunks failed while patching ipfw.8 done
| sbin/ipfw/ipfw.8 | ||
|---|---|---|
| 2565 ↗ | (On Diff #44948) | Specification of the nature of the "two lists" is good, but I don't see what the "1024" refers to. Maximum (default) queue depth? |
| 2582 ↗ | (On Diff #44948) | Would the wording: "fq_codel uses the same AQM parameters and options as codel (see blow), and fq_pie uses the same AQM parameters and options as pie (see below)." be clearer? Or perhaps use the phrasing "inherits ... from"? |
| 2595 ↗ | (On Diff #44948) | typo, I think: s/1514 byte,/1514 bytes,/ |
| 2596 ↗ | (On Diff #44948) | Similar: "byte" should be plural. |
| 2599 ↗ | (On Diff #44948) | I think "packet" should also be plural. |
| 2603 ↗ | (On Diff #44948) | And plural "packets" again (both instances). |
| 2607 ↗ | (On Diff #44948) | creates and manages. [Rationale: Structure is "specifies the ... number ... that fq_* creates ...."] |
| 2807 ↗ | (On Diff #44948) | I'd insert "of" at the end of the above line. |
| 2821 ↗ | (On Diff #44948) | Make that last bit "as follows:" |
| 2825 ↗ | (On Diff #44948) | "... between 0 and 7 which specifies ..." |
| 2830 ↗ | (On Diff #44948) | As above, "...between 0 and 7 which specifies ...". |
| 2851 ↗ | (On Diff #44948) | Need a bit of white space between "too far." and "De-randomisation". |
| 2854 ↗ | (On Diff #44948) | s/ turing / turning /, yes? |
| 2856 ↗ | (On Diff #44948) | Should that read "PIE turns on when ...."? |
| 2871 ↗ | (On Diff #44948) | The comma isn't needed here, I think. |
| 2872 ↗ | (On Diff #44948) | Append " the" to the above line. |
This comment was removed by ralsaadi_swin.edu.au.
| sbin/ipfw/ipfw.8 | ||
|---|---|---|
| 2565 ↗ | (On Diff #44948) | corrected and not it reads "(old sub-queues and new sub-queues) for providing brief periods of priority to |
| 2582 ↗ | (On Diff #44948) | corrected and now it reads "fq_codel inherits AQM parameters and options from codel (see below), and fq_pie inherits AQM parameters and options from pie (see below)." |
| 2595 ↗ | (On Diff #44948) | corrected |
| 2599 ↗ | (On Diff #44948) | corrected |
| 2603 ↗ | (On Diff #44948) | corrected |
| 2607 ↗ | (On Diff #44948) | Corrected and not it reads "specifies the total number of flow queues (sub-queues) that fq_* creates and manages." |
| 2807 ↗ | (On Diff #44948) | corrected |
| 2821 ↗ | (On Diff #44948) | corrected |
| 2825 ↗ | (On Diff #44948) | corrected |
| 2851 ↗ | (On Diff #44948) | corrected |
| 2854 ↗ | (On Diff #44948) | corrected |
| 2856 ↗ | (On Diff #44948) | corrected |
| 2871 ↗ | (On Diff #44948) | corrected |
| 2872 ↗ | (On Diff #44948) | corrected |
Comment Actions
Thanks a lot for reviewing ipfw AQM man page patch. I have corrected all the mistakes that you mentioned and created a new patch against the current ipfw.8 man page.
| sbin/ipfw/ipfw.8 | ||
|---|---|---|
| 2607 ↗ | (On Diff #44948) | Sorry, ** now it reads ... |