Skip to content

Conversation

@ethercflow
Copy link
Contributor

Add -H option to display latency as histograms grouped by PID. Supports periodic printing with interval argument and works with existing filters like -p, -t, -l, -r.

Trace latency between TCP received pkt and picked up by userspace thread.

USAGE: tcppktlat [--help] [-T] [-p PID] [-t TID] [-l LPORT] [-r RPORT] [-v]
USAGE: tcppktlat [--help] [-T] [-H] [-p PID] [-t TID] [-l LPORT] [-r RPORT] [-w] [-v]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The USAGE description in tcppktlat_example.txt is inconsistent with the actual implementation in tcppktlat. c

env.histogram = true;
if (arg) {
errno = 0;
env.interval = strtoul(arg, NULL, 10);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -H option allows specifying interval both as an optional argument and as a positional argument, which creates ambiguity.

fprintf(stderr, "Invalid delay (in us): %s\n", arg);
argp_usage(state);
if (env.histogram) {
/* For histogram mode, positional args are interval and count */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Positional argument behavior depends on -H, but this is unclear in the help message.

hkey = pid;

histp = bpf_map_lookup_or_try_init(&hists, &hkey, &zero);
if (histp) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit error handling would be preferable, similar to the else case. For example:

if (!histp)
    goto cleanup;

Add -H option to display latency as histograms grouped by PID.
Supports periodic printing with interval argument and works with
existing filters like -p, -t, -l, -r.

Signed-off-by: Wenbo Zhang <[email protected]>
@ethercflow
Copy link
Contributor Author

@ekyooo addressed, thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants