RE: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended statistics

-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Thursday, January 19, 2017 9:11 PM
To: Roopa Prabhu <redacted>
Cc: Nogah Frankel <redacted>; netdev@vger.kernel.org;
roszenrami@gmail.com; Jiri Pirko [off-list ref]; Ido Schimmel
[off-list ref]; Elad Raz [off-list ref]; Yotam Gigi
[off-list ref]; Or Gerlitz [off-list ref]
Subject: Re: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended statistics

On Thu, 19 Jan 2017 08:06:21 -0800
Roopa Prabhu [off-list ref] wrote:

quoted

On 1/19/17, 7:21 AM, Nogah Frankel wrote:

quoted

quoted

-----Original Message-----
From: Nogah Frankel
Sent: Sunday, January 15, 2017 3:55 PM
To: 'Stephen Hemminger' <stephen@networkplumber.org>
Cc: netdev@vger.kernel.org; roszenrami@gmail.com;

roopa@cumulusnetworks.com; Jiri

quoted

quoted

quoted

Pirko [off-list ref]; Ido Schimmel [off-list ref]; Elad Raz
[off-list ref]; Yotam Gigi [off-list ref]; Or Gerlitz
[off-list ref]
Subject: RE: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended

statistics

quoted

quoted

quoted

quoted

-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Friday, January 13, 2017 3:44 AM
To: Nogah Frankel <redacted>
Cc: netdev@vger.kernel.org; roszenrami@gmail.com;

roopa@cumulusnetworks.com;

quoted

quoted

quoted

Jiri

quoted

Pirko [off-list ref]; Ido Schimmel [off-list ref]; Elad Raz
[off-list ref]; Yotam Gigi [off-list ref]; Or Gerlitz
[off-list ref]
Subject: Re: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended

statistics

quoted

quoted

quoted

quoted

On Thu, 12 Jan 2017 15:49:50 +0200
Nogah Frankel [off-list ref] wrote:

quoted

The default stats for ifstat are 32 bits based.
The kernel supports 64 bits based stats. (They are returned in struct
rtnl_link_stats64 which is an exact copy of struct rtnl_link_stats, in
which the "normal" stats are returned, but with fields of u64 instead of
u32). This patch adds them as an extended stats.

It is read with filter type IFLA_STATS_LINK_64 and no sub type.

It is under the name 64bits
(or any shorten of it as "64")

For example:
ifstat -x 64bit

Signed-off-by: Nogah Frankel <redacted>
Reviewed-by: Jiri Pirko <redacted>

Other commands (like ip link) always use the 64 bit statistics if available
from the device. I see no reason that ifstat needs to be different.

Do you mean to change the default ifstat results to be 64 bits based?
I tried it in the first version, but Roopa commented that it was not a good idea.
She said they tried it in the past and it caused backward compatibilities problems.
(Or maybe I didn't understand correctly)

So, can I leave the default ifstat results to be 32 bits based, for the time being?

From past discussions: Moving the default to 64bit has compat issues with the old

history file.

quoted

There is a way to make it work by using a new file header (to indicate that it is 64 bit) in
a freshly created history file and also check this header before dumping stats into the

history file.

quoted

ie maintain backward compat without introducing a new option. It is doable.

One approach is, you can drop the 64bit option from this series and
try updating the default to 64 bit (with compat handling code) in a later series.

I think I will take your suggestion to drop the 64 bits from this series.
Hopefully, I'll return to it in some later series in the future.
Thanks

The ifstat code could do conversion based on file size.

  if (history_file_is_32bit()) {
	printf("converting to 64 bit format\n");
        ...
  }