Re: [PATCH net-next v8 2/3] selftests: tc-testing: add fq_pie per-flow class stats test

[Hemendra M. Naik]

Thank you for the review. Our response to the low-severity comment is inline below.

> [Severity: Low]
> This is a robustness point, not a bug. fq_pie_walk() in
> net/sched/sch_fq_pie.c only dumps flows that have a non-empty flowchain:
>
>     if (list_empty(&q->flows[i].flowchain)) {
>         arg->count++;
>         continue;
>     }
>
> That makes this ping the only thing that gets class 2:1 into the output.
>
> In the standard tdc setup this works. nsPlugin runs the command inside
> the namespace, dummy is NOARP, and the 98-byte frame fits under tbf's
> max_size of 100.
>
> If ping is missing, or doesn't accept -i 0.001 / -W 0.01, the "|| true"
> and the redirect hide the error. The test then fails as a regex mismatch
> in the verify stage, not as a setup error.
>
> Should this step be allowed to fail visibly? The same pattern is already
> used elsewhere in tc-testing (infra/qdiscs.json), so it may be fine as is.

Keeping it as is. tc-tests/infra/qdiscs.json (test ca5e, "Check class
delete notification for ffff:") already uses the identical pattern:

    "ping -c1 -W0.01 -I $DUMMY 10.10.10.1 || true",

as a setup step whose only job is to push a packet through the qdisc,
with the same silent-failure trade-off described above. 83c0 follows
existing practice here rather than introducing a new one.


Regards,
Hemendra