Various improvements to the netmap pkt-gen program:
- indentation fixes
- support for IPV6
- fixes to checksum computation
- support for NS_MOREFRAG
- rate limiting in ping mode
Details
Details
- Reviewers
gnn hrs bcr 0mp - Group Reviewers
manpages - Commits
- rS341457: MFC r340325
rS340619: MFC r340325
rS340325: netmap: pkt-gen: several updates from upstream
Tested locally with many types of interfaces.
Tested by the community for months.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
OK from manpages (don't forget to bump the .Dd, even for such a small content change).
Comment Actions
Please correct me if I'm wrong but you seem to add some functionalities to this software (potentially even some new flags) without documenting those changes in the manual page. Is that right?
Comment Actions
@0mp corrected me that there is no .Dd needed. There was no change in the syntax in the man page itself, just whitespace fixes. In those cases, you can leave the .Dd as it is.
Comment Actions
Addressed reviewer's comments.
pkt-gen man page and usage() cleaned up to reflect the real options.
Some old/unused/obsolete options have been removed.
Comment Actions
Also, igor and mandoc -Tlint still report some errors.
| pkt-gen.8 | ||
|---|---|---|
| 59 ↗ | (On Diff #49615) | tx is not an argument name. It is a fixed string that the user may provide to the utility to change its behavior. Therefor it is a command modifier and it should use Cm instead of Ar. The same applies to rx, ping, ... below. |
| 150 ↗ | (On Diff #49615) | .Ar len |
| 202 ↗ | (On Diff #49615) | You probably want something like this: .It Fl C Ar tx_slots Ns Oo Cm \&, Ns Ar rx_slots Ns Oo Cm \&, Ns Ar tx_rings Ns Oo Cm \&, Ns Ar rx_rings Oc Oc Oc I may fix formatting in a different commit, as there are some similar issues in the synopsis section, if you prefer. |
| 221 ↗ | (On Diff #49615) | I am not sure if I understand this sentence. |
| 235 ↗ | (On Diff #49615) | I'd use Dq here (or even Dq Li) instead of Ar. |
| 244 ↗ | (On Diff #49615) | It would be better to put this example in a code block: .Bd pkg-gen -i cxl0 -f rx .Ed The same applies to the other example. |
Comment Actions
Thanks for the suggestions.
I did not implement two of the suggested changes, see inline.
igor and mandoc -Tlint report zero errors
Comment Actions
Flushing unsubmitted comments
| pkt-gen.8 | ||
|---|---|---|
| 221 ↗ | (On Diff #49615) | I noticed there is an offset between your comments should be displayed and where I actually see them. I'm afraid I don't understand which sentence you don't understand. |
| 244 ↗ | (On Diff #49615) | If I try something like that I get a mandoc -Tlint error: Bd: skipping display without arguments |
Comment Actions
Good job so far. There are couple of issues left:
- igor produces the following errors on my machine (FreeBSD 13.0-CURRENT r339443 GENERIC amd64, igor-1.595):
pkt-gen.8:33:.Sh DESCRIPTION used here:but .Nm has not been defined pkt-gen.8:33:.Sh DESCRIPTION used here:but .Nd has not been defined pkt-gen.8:33:.Sh DESCRIPTION used here:but .Sh SYNOPSIS has not been defined
- mandoc -Tlint produces the following errors:
mandoc: ./pkt-gen.8:10:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:8:2: WARNING: bad NAME section content: Bl mandoc: ./pkt-gen.8:7:2: WARNING: NAME section without Nm before Nd mandoc: ./pkt-gen.8:7:2: WARNING: NAME section without description mandoc: ./pkt-gen.8:34:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:44:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:57:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:69:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:105:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:132:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:141:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:158:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:222:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:230:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:245:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:253:2: ERROR: missing manual name, using "": Nm mandoc: ./pkt-gen.8:257:6: STYLE: referenced manual not found: Xr bridge 8
- I like to render manpage as HTML to see if everything is fine. You may like the idea as well ;)
mandoc -Thtml > /tmp/a.html ./pkg-gen.8 firefox /tmp/a.html
| pkt-gen.8 | ||
|---|---|---|
| 221 ↗ | (On Diff #49615) | I'm referring to: .Nm .Xr netmap 4 or .Xr bpf 4 but which is most often used with .Xr netmap 4 . |
| 235 ↗ | (On Diff #49615) | I think you skipped this one. |
| 244 ↗ | (On Diff #49615) | Oh right, I have not provided you with the full syntax: .Bd -literal -offset indent ... should do the job. |
Comment Actions
- Is a good idea, thanks.
For 1 and 2 I'm sorry , but I only have freefall.freebsd.org and some VMs where I do not see those errors.
Also, it looks like most of those error messages do not make sense...
| pkt-gen.8 | ||
|---|---|---|
| 221 ↗ | (On Diff #49615) | Yes, I also do not really understand that. pkt-gen transmits/receives to/from netmap ports. The tx/rx functions are also able to work on TAP or PCAP interfaces. I think @gnn wrote this sentence ? Maybe he can shed some light on what he meant exactly? |
| 235 ↗ | (On Diff #49615) | I replaced .Ar with .Dq. Maybe we are talking about different lines? I mean line .Dq tx_slots,rx_slots,tx_rings,rx_rings . |