Re: [PATCH net-next v5 3/3] net/sched: pie: correct tc_pie_xstats field documentation

["Hemendra M. Naik" <hemendranaik@gmail.com>]

Hi @Jakub,

Thank you for the review. Replies are inline below.

> Should the vendored copy in tools/include/uapi/linux/pkt_sched.h be
> updated in the same patch? [...] So after this change the two in-tree
> copies of the same structure describe the same field with different
> units. [...] Would a resync of tools/include/uapi/linux/pkt_sched.h,
> or at least of the tc_pie_xstats comments, be appropriate so the stale
> documentation the commit message aims to eliminate is actually gone
> from the tree?

We would prefer to leave this out of v6. The v4 review asked us to drop
that, and we did; adding it back now would undo that. The copy is
stale well beyond this one comment (u32 prob, no dq_rate_estimating, no
FQ-PIE additions at all), so fixing a single unit comment there would
not help much. A proper resync feels like its own patch.

> This isn't a bug introduced by this patch, but does the exported value
> always match the newly documented microsecond unit? [...] a qdelay
> whose nanosecond value exceeds 2^32-1 (roughly 4.295 s) wraps modulo
> 2^32 ns and then gets divided, reporting a small microsecond number
> for a large delay. [...] Would moving the cast after the division in
> both sch_pie.c and sch_fq_pie.c be worth a follow-up, so the code
> matches the microsecond contract this comment now states?

Thank you for catching this; the bug is real, and you already flagged
the same issue for sch_fq_pie.c on patch 1 — we will fix it there with
div_u64(). No code change is needed for this patch itself; it is 
comment-only and correct as posted. We will send the sch_pie.c fix as 
a follow-up after this series.




One more query: I see that the counterpart iproute2 v5 patches are currently
marked as "Awaiting Upstream". In that case, is it necessary to post a v6 of 
the iproute2 series, or can we wait for the v5 patch to be reviewed?



Thanks,
Hemendra