Re: [PATCH] iproute2: fix type incompatibility in ifstat.c
From: Stephen Gallagher <hidden>
Date: 2024-02-08 18:18:13
On Tue, Feb 6, 2024 at 10:12 PM Stephen Hemminger [off-list ref] wrote:
On Tue, 6 Feb 2024 09:22:06 -0500 Stephen Gallagher [off-list ref] wrote:quoted
Throughout ifstat.c, ifstat_ent.val is accessed as a long long unsigned type, however it is defined as __u64. This works by coincidence on many systems, however on ppc64le, __u64 is a long unsigned. This patch makes the type definition consistent with all of the places where it is accessed. Signed-off-by: Stephen Gallagher <redacted> ---Why not fix the use of unsigned long long to be __u64 instead? That would make more sense.
FWIW, that's the first path I tried, but it's accessed in dozens of places, every one of which treats it as a 'long long unsigned'. That led me to the belief that the specified type was just wrong (or at minimum, in contravention of the consumers' expectations).
Looking at ifstat it has other problems. It doesn't check the sizes in the netlink response. It really should be using 64 bit stat counters, not the legacy 32 bit values. (ie IFLA_STATS64). Anyone want to take this on?X
I definitely don't have the subject-matter expertise to do this, sorry. Given that this issue is blocking our builds in Fedora/RHEL, I was hoping we could try to solve the acute problem and leave wider-ranging fixes to a separate effort.
Also, the cpu_hits offload code is a wart. If it is of interest, it should be more than one stat. One last issue, is that change the size of this structure will break old history files.
I'm not sure I understand this; it looks to me like this is a private structure. If it has API impact (as in, it's written directly to output data meant to be re-read), shouldn't it be in a public header and/or otherwise denoted as API?