Hi,
I have been using the data counters API and have noticed that the
implementation uses 32 bit data types throughout. For the bytes received and
transmitted this means that the values wrap once 4GB has been transferred. The
other fields are less likely to wrap so soon.
The attached patch changes it to use 64 bit data. Currently it makes no
attempt to maintain compatibility as both the format of the data file and the
DBus API changes. I see little advantage in maintaining compatibility with the
data file. The data type of the counter fields are not documented in the
counter-api.txt file. Depending on how the demarshaller is implemented this
type change is potentially handled automatically. Extending the DBus interface
(instead of just changing the signature) is easy enough.
Any feedback or suggestions would be appreciated.
Cheers,
--
Aaron McCarthy
>From 8e87196bb3317be48f51f65c8e5485d4be3a0571 Mon Sep 17 00:00:00 2001
From: Aaron McCarthy <[email protected]>
Date: Wed, 26 Feb 2014 14:50:52 +1000
Subject: [PATCH] Change data counters to use 64 bit data.
The previous data counters used unsigned 32 bit types. This is
probably adequate for most of the data fields, but for the bytes
received and bytes transmitted it means that the counters wrap after
4GB of data.
Change to use the 64 bit interface to retrieve the data from the kernel
and to use 64 bit types in both the storage and DBus API.
This changes the type signature of the DBus interface and expects
applications to cope.
This changes the binary format of the data files. The record size are
now larger. The magic number is changed which causes the data files to
be reinitialised losing previous data history. But that is ok because it
is likely to have wrapped anyway.
---
src/connman.h | 30 +++++++++++++++---------------
src/ipconfig.c | 26 +++++++++++++-------------
src/rtnl.c | 14 +++++++-------
src/service.c | 32 ++++++++++++++++----------------
src/stats.c | 2 +-
tools/stats-tool.c | 37 +++++++++++++++++++------------------
vpn/vpn-ipconfig.c | 4 ++--
vpn/vpn-rtnl.c | 14 +++++++-------
vpn/vpn.h | 6 +++---
9 files changed, 83 insertions(+), 82 deletions(-)
diff --git a/src/connman.h b/src/connman.h
index f5ed995..00af076 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -315,13 +315,13 @@ void __connman_ipconfig_enable_ipv6(struct connman_ipconfig *ipconfig);
int __connman_ipconfig_init(void);
void __connman_ipconfig_cleanup(void);
-struct rtnl_link_stats;
+struct rtnl_link_stats64;
void __connman_ipconfig_newlink(int index, unsigned short type,
unsigned int flags, const char *address,
unsigned short mtu,
- struct rtnl_link_stats *stats);
-void __connman_ipconfig_dellink(int index, struct rtnl_link_stats *stats);
+ struct rtnl_link_stats64 *stats);
+void __connman_ipconfig_dellink(int index, struct rtnl_link_stats64 *stats);
void __connman_ipconfig_newaddr(int index, int family, const char *label,
unsigned char prefixlen, const char *address);
void __connman_ipconfig_deladdr(int index, int family, const char *label,
@@ -761,10 +761,10 @@ int __connman_service_reset_ipconfig(struct connman_service *service,
enum connman_service_state *new_state);
void __connman_service_notify(struct connman_service *service,
- unsigned int rx_packets, unsigned int tx_packets,
- unsigned int rx_bytes, unsigned int tx_bytes,
- unsigned int rx_error, unsigned int tx_error,
- unsigned int rx_dropped, unsigned int tx_dropped);
+ uint64_t rx_packets, uint64_t tx_packets,
+ uint64_t rx_bytes, uint64_t tx_bytes,
+ uint64_t rx_error, uint64_t tx_error,
+ uint64_t rx_dropped, uint64_t tx_dropped);
int __connman_service_counter_register(const char *counter);
void __connman_service_counter_unregister(const char *counter);
@@ -826,14 +826,14 @@ int __connman_session_init(void);
void __connman_session_cleanup(void);
struct connman_stats_data {
- unsigned int rx_packets;
- unsigned int tx_packets;
- unsigned int rx_bytes;
- unsigned int tx_bytes;
- unsigned int rx_errors;
- unsigned int tx_errors;
- unsigned int rx_dropped;
- unsigned int tx_dropped;
+ uint64_t rx_packets;
+ uint64_t tx_packets;
+ uint64_t rx_bytes;
+ uint64_t tx_bytes;
+ uint64_t rx_errors;
+ uint64_t tx_errors;
+ uint64_t rx_dropped;
+ uint64_t tx_dropped;
unsigned int time;
};
diff --git a/src/ipconfig.c b/src/ipconfig.c
index 9452125..7b88107 100644
--- a/src/ipconfig.c
+++ b/src/ipconfig.c
@@ -66,14 +66,14 @@ struct connman_ipdevice {
unsigned int flags;
char *address;
uint16_t mtu;
- uint32_t rx_packets;
- uint32_t tx_packets;
- uint32_t rx_bytes;
- uint32_t tx_bytes;
- uint32_t rx_errors;
- uint32_t tx_errors;
- uint32_t rx_dropped;
- uint32_t tx_dropped;
+ uint64_t rx_packets;
+ uint64_t tx_packets;
+ uint64_t rx_bytes;
+ uint64_t tx_bytes;
+ uint64_t rx_errors;
+ uint64_t tx_errors;
+ uint64_t rx_dropped;
+ uint64_t tx_dropped;
GSList *address_list;
char *ipv4_gateway;
@@ -430,16 +430,16 @@ static void __connman_ipconfig_lower_down(struct connman_ipdevice *ipdevice)
}
static void update_stats(struct connman_ipdevice *ipdevice,
- const char *ifname, struct rtnl_link_stats *stats)
+ const char *ifname, struct rtnl_link_stats64 *stats)
{
struct connman_service *service;
if (stats->rx_packets == 0 && stats->tx_packets == 0)
return;
- connman_info("%s {RX} %u packets %u bytes", ifname,
+ connman_info("%s {RX} %llu packets %llu bytes", ifname,
stats->rx_packets, stats->rx_bytes);
- connman_info("%s {TX} %u packets %u bytes", ifname,
+ connman_info("%s {TX} %llu packets %llu bytes", ifname,
stats->tx_packets, stats->tx_bytes);
if (!ipdevice->config_ipv4 && !ipdevice->config_ipv6)
@@ -474,7 +474,7 @@ static void update_stats(struct connman_ipdevice *ipdevice,
void __connman_ipconfig_newlink(int index, unsigned short type,
unsigned int flags, const char *address,
unsigned short mtu,
- struct rtnl_link_stats *stats)
+ struct rtnl_link_stats64 *stats)
{
struct connman_ipdevice *ipdevice;
GList *list;
@@ -586,7 +586,7 @@ out:
g_free(ifname);
}
-void __connman_ipconfig_dellink(int index, struct rtnl_link_stats *stats)
+void __connman_ipconfig_dellink(int index, struct rtnl_link_stats64 *stats)
{
struct connman_ipdevice *ipdevice;
GList *list;
diff --git a/src/rtnl.c b/src/rtnl.c
index c8708eb..9dab994 100644
--- a/src/rtnl.c
+++ b/src/rtnl.c
@@ -360,7 +360,7 @@ static const char *operstate2str(unsigned char operstate)
static bool extract_link(struct ifinfomsg *msg, int bytes,
struct ether_addr *address, const char **ifname,
unsigned int *mtu, unsigned char *operstate,
- struct rtnl_link_stats *stats)
+ struct rtnl_link_stats64 *stats)
{
struct rtattr *attr;
@@ -379,10 +379,10 @@ static bool extract_link(struct ifinfomsg *msg, int bytes,
if (mtu)
*mtu = *((unsigned int *) RTA_DATA(attr));
break;
- case IFLA_STATS:
+ case IFLA_STATS64:
if (stats)
memcpy(stats, RTA_DATA(attr),
- sizeof(struct rtnl_link_stats));
+ sizeof(struct rtnl_link_stats64));
break;
case IFLA_OPERSTATE:
if (operstate)
@@ -403,7 +403,7 @@ static void process_newlink(unsigned short type, int index, unsigned flags,
{
struct ether_addr address = {{ 0, 0, 0, 0, 0, 0 }};
struct ether_addr compare = {{ 0, 0, 0, 0, 0, 0 }};
- struct rtnl_link_stats stats;
+ struct rtnl_link_stats64 stats;
unsigned char operstate = 0xff;
struct interface_data *interface;
const char *ifname = NULL;
@@ -496,7 +496,7 @@ static void process_newlink(unsigned short type, int index, unsigned flags,
static void process_dellink(unsigned short type, int index, unsigned flags,
unsigned change, struct ifinfomsg *msg, int bytes)
{
- struct rtnl_link_stats stats;
+ struct rtnl_link_stats64 stats;
unsigned char operstate = 0xff;
const char *ifname = NULL;
GSList *list;
@@ -899,8 +899,8 @@ static void rtnl_link(struct nlmsghdr *hdr)
case IFLA_QDISC:
print_attr(attr, "qdisc");
break;
- case IFLA_STATS:
- print_attr(attr, "stats");
+ case IFLA_STATS64:
+ print_attr(attr, "stats64");
break;
case IFLA_COST:
print_attr(attr, "cost");
diff --git a/src/service.c b/src/service.c
index 9229ba8..ecb8b37 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1990,49 +1990,49 @@ static void stats_append_counters(DBusMessageIter *dict,
if (counters->rx_packets != stats->rx_packets || append_all) {
counters->rx_packets = stats->rx_packets;
connman_dbus_dict_append_basic(dict, "RX.Packets",
- DBUS_TYPE_UINT32, &stats->rx_packets);
+ DBUS_TYPE_UINT64, &stats->rx_packets);
}
if (counters->tx_packets != stats->tx_packets || append_all) {
counters->tx_packets = stats->tx_packets;
connman_dbus_dict_append_basic(dict, "TX.Packets",
- DBUS_TYPE_UINT32, &stats->tx_packets);
+ DBUS_TYPE_UINT64, &stats->tx_packets);
}
if (counters->rx_bytes != stats->rx_bytes || append_all) {
counters->rx_bytes = stats->rx_bytes;
connman_dbus_dict_append_basic(dict, "RX.Bytes",
- DBUS_TYPE_UINT32, &stats->rx_bytes);
+ DBUS_TYPE_UINT64, &stats->rx_bytes);
}
if (counters->tx_bytes != stats->tx_bytes || append_all) {
counters->tx_bytes = stats->tx_bytes;
connman_dbus_dict_append_basic(dict, "TX.Bytes",
- DBUS_TYPE_UINT32, &stats->tx_bytes);
+ DBUS_TYPE_UINT64, &stats->tx_bytes);
}
if (counters->rx_errors != stats->rx_errors || append_all) {
counters->rx_errors = stats->rx_errors;
connman_dbus_dict_append_basic(dict, "RX.Errors",
- DBUS_TYPE_UINT32, &stats->rx_errors);
+ DBUS_TYPE_UINT64, &stats->rx_errors);
}
if (counters->tx_errors != stats->tx_errors || append_all) {
counters->tx_errors = stats->tx_errors;
connman_dbus_dict_append_basic(dict, "TX.Errors",
- DBUS_TYPE_UINT32, &stats->tx_errors);
+ DBUS_TYPE_UINT64, &stats->tx_errors);
}
if (counters->rx_dropped != stats->rx_dropped || append_all) {
counters->rx_dropped = stats->rx_dropped;
connman_dbus_dict_append_basic(dict, "RX.Dropped",
- DBUS_TYPE_UINT32, &stats->rx_dropped);
+ DBUS_TYPE_UINT64, &stats->rx_dropped);
}
if (counters->tx_dropped != stats->tx_dropped || append_all) {
counters->tx_dropped = stats->tx_dropped;
connman_dbus_dict_append_basic(dict, "TX.Dropped",
- DBUS_TYPE_UINT32, &stats->tx_dropped);
+ DBUS_TYPE_UINT64, &stats->tx_dropped);
}
if (counters->time != stats->time || append_all) {
@@ -2081,10 +2081,10 @@ static void stats_append(struct connman_service *service,
}
static void stats_update(struct connman_service *service,
- unsigned int rx_packets, unsigned int tx_packets,
- unsigned int rx_bytes, unsigned int tx_bytes,
- unsigned int rx_errors, unsigned int tx_errors,
- unsigned int rx_dropped, unsigned int tx_dropped)
+ uint64_t rx_packets, uint64_t tx_packets,
+ uint64_t rx_bytes, uint64_t tx_bytes,
+ uint64_t rx_errors, uint64_t tx_errors,
+ uint64_t rx_dropped, uint64_t tx_dropped)
{
struct connman_stats *stats = stats_get(service);
struct connman_stats_data *data_last = &stats->data_last;
@@ -2128,10 +2128,10 @@ static void stats_update(struct connman_service *service,
}
void __connman_service_notify(struct connman_service *service,
- unsigned int rx_packets, unsigned int tx_packets,
- unsigned int rx_bytes, unsigned int tx_bytes,
- unsigned int rx_errors, unsigned int tx_errors,
- unsigned int rx_dropped, unsigned int tx_dropped)
+ uint64_t rx_packets, uint64_t tx_packets,
+ uint64_t rx_bytes, uint64_t tx_bytes,
+ uint64_t rx_errors, uint64_t tx_errors,
+ uint64_t rx_dropped, uint64_t tx_dropped)
{
GHashTableIter iter;
gpointer key, value;
diff --git a/src/stats.c b/src/stats.c
index 792cdea..1880a65 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -46,7 +46,7 @@
#define TFR
#endif
-#define MAGIC 0xFA00B916
+#define MAGIC 0xFA01B916
/*
* Statistics counters are stored into a ring buffer which is stored
diff --git a/tools/stats-tool.c b/tools/stats-tool.c
index 7957c47..0a51c06 100644
--- a/tools/stats-tool.c
+++ b/tools/stats-tool.c
@@ -37,6 +37,7 @@
#include <stdlib.h>
#include <stdbool.h>
#include <errno.h>
+#include <inttypes.h>
#include <glib.h>
#include <glib/gstdio.h>
@@ -47,17 +48,17 @@
#define TFR
#endif
-#define MAGIC 0xFA00B916
+#define MAGIC 0xFA01B916
struct connman_stats_data {
- unsigned int rx_packets;
- unsigned int tx_packets;
- unsigned int rx_bytes;
- unsigned int tx_bytes;
- unsigned int rx_errors;
- unsigned int tx_errors;
- unsigned int rx_dropped;
- unsigned int tx_dropped;
+ uint64_t rx_packets;
+ uint64_t tx_packets;
+ uint64_t rx_bytes;
+ uint64_t tx_bytes;
+ uint64_t rx_errors;
+ uint64_t tx_errors;
+ uint64_t rx_dropped;
+ uint64_t tx_dropped;
unsigned int time;
};
@@ -220,7 +221,7 @@ static void stats_print_record(struct stats_record *rec)
char buffer[30];
strftime(buffer, 30, "%d-%m-%Y %T", localtime(&rec->ts));
- printf("%p %lld %s %01d %d %d %d %d %d %d %d %d %d\n",
+ printf("%p %lld %s %01d %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %d\n",
rec, (long long int)rec->ts, buffer,
rec->roaming,
rec->data.rx_packets,
@@ -310,21 +311,21 @@ static void stats_print_entries(struct stats_file *file)
static void stats_print_rec_diff(struct stats_record *begin,
struct stats_record *end)
{
- printf("\trx_packets: %d\n",
+ printf("\trx_packets: %" PRIu64 "\n",
end->data.rx_packets - begin->data.rx_packets);
- printf("\ttx_packets: %d\n",
+ printf("\ttx_packets: %" PRIu64 "\n",
end->data.tx_packets - begin->data.tx_packets);
- printf("\trx_bytes: %d\n",
+ printf("\trx_bytes: %" PRIu64 "\n",
end->data.rx_bytes - begin->data.rx_bytes);
- printf("\ttx_bytes: %d\n",
+ printf("\ttx_bytes: %" PRIu64 "\n",
end->data.tx_bytes - begin->data.tx_bytes);
- printf("\trx_errors: %d\n",
+ printf("\trx_errors: %" PRIu64 "\n",
end->data.rx_errors - begin->data.rx_errors);
- printf("\ttx_errors: %d\n",
+ printf("\ttx_errors: %" PRIu64 "\n",
end->data.tx_errors - begin->data.tx_errors);
- printf("\trx_dropped: %d\n",
+ printf("\trx_dropped: %" PRIu64 "\n",
end->data.rx_dropped - begin->data.rx_dropped);
- printf("\ttx_dropped: %d\n",
+ printf("\ttx_dropped: %" PRIu64 "\n",
end->data.tx_dropped - begin->data.tx_dropped);
printf("\ttime: %d\n",
end->data.time - begin->data.time);
diff --git a/vpn/vpn-ipconfig.c b/vpn/vpn-ipconfig.c
index c3e6145..791dee3 100644
--- a/vpn/vpn-ipconfig.c
+++ b/vpn/vpn-ipconfig.c
@@ -362,7 +362,7 @@ void __vpn_ipconfig_newlink(int index, unsigned short type,
unsigned int flags,
const char *address,
unsigned short mtu,
- struct rtnl_link_stats *stats)
+ struct rtnl_link_stats64 *stats)
{
struct vpn_ipdevice *ipdevice;
GString *str;
@@ -418,7 +418,7 @@ update:
g_string_free(str, TRUE);
}
-void __vpn_ipconfig_dellink(int index, struct rtnl_link_stats *stats)
+void __vpn_ipconfig_dellink(int index, struct rtnl_link_stats64 *stats)
{
struct vpn_ipdevice *ipdevice;
diff --git a/vpn/vpn-rtnl.c b/vpn/vpn-rtnl.c
index af24674..4a441ee 100644
--- a/vpn/vpn-rtnl.c
+++ b/vpn/vpn-rtnl.c
@@ -227,7 +227,7 @@ static const char *operstate2str(unsigned char operstate)
static void extract_link(struct ifinfomsg *msg, int bytes,
struct ether_addr *address, const char **ifname,
unsigned int *mtu, unsigned char *operstate,
- struct rtnl_link_stats *stats)
+ struct rtnl_link_stats64 *stats)
{
struct rtattr *attr;
@@ -246,10 +246,10 @@ static void extract_link(struct ifinfomsg *msg, int bytes,
if (mtu)
*mtu = *((unsigned int *) RTA_DATA(attr));
break;
- case IFLA_STATS:
+ case IFLA_STATS64:
if (stats)
memcpy(stats, RTA_DATA(attr),
- sizeof(struct rtnl_link_stats));
+ sizeof(struct rtnl_link_stats64));
break;
case IFLA_OPERSTATE:
if (operstate)
@@ -266,7 +266,7 @@ static void process_newlink(unsigned short type, int index, unsigned flags,
{
struct ether_addr address = {{ 0, 0, 0, 0, 0, 0 }};
struct ether_addr compare = {{ 0, 0, 0, 0, 0, 0 }};
- struct rtnl_link_stats stats;
+ struct rtnl_link_stats64 stats;
unsigned char operstate = 0xff;
struct interface_data *interface;
const char *ifname = NULL;
@@ -343,7 +343,7 @@ static void process_newlink(unsigned short type, int index, unsigned flags,
static void process_dellink(unsigned short type, int index, unsigned flags,
unsigned change, struct ifinfomsg *msg, int bytes)
{
- struct rtnl_link_stats stats;
+ struct rtnl_link_stats64 stats;
unsigned char operstate = 0xff;
const char *ifname = NULL;
GSList *list;
@@ -612,8 +612,8 @@ static void rtnl_link(struct nlmsghdr *hdr)
case IFLA_QDISC:
print_attr(attr, "qdisc");
break;
- case IFLA_STATS:
- print_attr(attr, "stats");
+ case IFLA_STATS64:
+ print_attr(attr, "stats64");
break;
case IFLA_COST:
print_attr(attr, "cost");
diff --git a/vpn/vpn.h b/vpn/vpn.h
index 1398d05..743fd32 100644
--- a/vpn/vpn.h
+++ b/vpn/vpn.h
@@ -60,13 +60,13 @@ void __vpn_ipconfig_unref_debug(struct vpn_ipconfig *ipconfig,
struct vpn_ipconfig *__vpn_ipconfig_create(int index, int family);
void __vpn_ipconfig_set_index(struct vpn_ipconfig *ipconfig,
int index);
-struct rtnl_link_stats;
+struct rtnl_link_stats64;
void __vpn_ipconfig_newlink(int index, unsigned short type,
unsigned int flags, const char *address,
unsigned short mtu,
- struct rtnl_link_stats *stats);
-void __vpn_ipconfig_dellink(int index, struct rtnl_link_stats *stats);
+ struct rtnl_link_stats64 *stats);
+void __vpn_ipconfig_dellink(int index, struct rtnl_link_stats64 *stats);
int __vpn_ipconfig_init(void);
void __vpn_ipconfig_cleanup(void);
--
1.9.0
_______________________________________________
connman mailing list
[email protected]
https://lists.connman.net/mailman/listinfo/connman
