Thread (7 messages) 7 messages, 4 authors, 2024-02-15

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?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help