* [PATCH iproute2-next 1/6] ipaddress: Unify print_link_stats() and print_link_stats64()
2018-02-02 13:10 [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
@ 2018-02-02 13:10 ` Serhey Popovych
2018-02-02 13:10 ` [PATCH iproute2-next 2/6] ip: Introduce get_rtnl_link_stats_rta() to get link statistics Serhey Popovych
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Serhey Popovych @ 2018-02-02 13:10 UTC (permalink / raw)
To: netdev
To show real differences between these two variants adjust whitespace
intendation and use print_uint() instead of print_int() as all members
in both @struct rtnl_link_stats and @struct rtnl_link_stats64 are
unsigned.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ipaddress.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b18b6f4..ae419f0 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -610,8 +610,7 @@ static void print_link_stats64(FILE *fp, const struct rtnl_link_stats64 *s,
print_uint(PRINT_JSON, "multicast", NULL, s->multicast);
if (s->rx_compressed)
print_uint(PRINT_JSON,
- "compressed",
- NULL, s->rx_compressed);
+ "compressed", NULL, s->rx_compressed);
/* RX error stats */
if (show_stats > 1) {
@@ -648,8 +647,7 @@ static void print_link_stats64(FILE *fp, const struct rtnl_link_stats64 *s,
print_uint(PRINT_JSON, "collisions", NULL, s->collisions);
if (s->tx_compressed)
print_uint(PRINT_JSON,
- "compressed",
- NULL, s->tx_compressed);
+ "compressed", NULL, s->tx_compressed);
/* TX error stats */
if (show_stats > 1) {
@@ -669,9 +667,9 @@ static void print_link_stats64(FILE *fp, const struct rtnl_link_stats64 *s,
print_uint(PRINT_JSON, "carrier_changes", NULL,
rta_getattr_u32(carrier_changes));
}
+
close_json_object();
close_json_object();
-
} else {
/* RX stats */
fprintf(fp, " RX: bytes packets errors dropped overrun mcast %s%s",
@@ -692,7 +690,6 @@ static void print_link_stats64(FILE *fp, const struct rtnl_link_stats64 *s,
fprintf(fp, "%s", _SL_);
fprintf(fp, " RX errors: length crc frame fifo missed%s%s",
s->rx_nohandler ? " nohandler" : "", _SL_);
-
fprintf(fp, " ");
print_num(fp, 8, s->rx_length_errors);
print_num(fp, 7, s->rx_crc_errors);
@@ -701,7 +698,6 @@ static void print_link_stats64(FILE *fp, const struct rtnl_link_stats64 *s,
print_num(fp, 7, s->rx_missed_errors);
if (s->rx_nohandler)
print_num(fp, 7, s->rx_nohandler);
-
}
fprintf(fp, "%s", _SL_);
@@ -754,9 +750,8 @@ static void print_link_stats32(FILE *fp, const struct rtnl_link_stats *s,
print_uint(PRINT_JSON, "over_errors", NULL, s->rx_over_errors);
print_uint(PRINT_JSON, "multicast", NULL, s->multicast);
if (s->rx_compressed)
- print_int(PRINT_JSON,
- "compressed",
- NULL, s->rx_compressed);
+ print_uint(PRINT_JSON,
+ "compressed", NULL, s->rx_compressed);
/* RX error stats */
if (show_stats > 1) {
@@ -776,9 +771,8 @@ static void print_link_stats32(FILE *fp, const struct rtnl_link_stats *s,
"missed_errors",
NULL, s->rx_missed_errors);
if (s->rx_nohandler)
- print_int(PRINT_JSON,
- "nohandler",
- NULL, s->rx_nohandler);
+ print_uint(PRINT_JSON,
+ "nohandler", NULL, s->rx_nohandler);
}
close_json_object();
@@ -793,9 +787,8 @@ static void print_link_stats32(FILE *fp, const struct rtnl_link_stats *s,
NULL, s->tx_carrier_errors);
print_uint(PRINT_JSON, "collisions", NULL, s->collisions);
if (s->tx_compressed)
- print_int(PRINT_JSON,
- "compressed",
- NULL, s->tx_compressed);
+ print_uint(PRINT_JSON,
+ "compressed", NULL, s->tx_compressed);
/* TX error stats */
if (show_stats > 1) {
@@ -812,9 +805,7 @@ static void print_link_stats32(FILE *fp, const struct rtnl_link_stats *s,
"heartbeat_errors",
NULL, s->tx_heartbeat_errors);
if (carrier_changes)
- print_uint(PRINT_JSON,
- "carrier_changes",
- NULL,
+ print_uint(PRINT_JSON, "carrier_changes", NULL,
rta_getattr_u32(carrier_changes));
}
@@ -825,7 +816,6 @@ static void print_link_stats32(FILE *fp, const struct rtnl_link_stats *s,
fprintf(fp, " RX: bytes packets errors dropped overrun mcast %s%s",
s->rx_compressed ? "compressed" : "", _SL_);
-
fprintf(fp, " ");
print_num(fp, 10, s->rx_bytes);
print_num(fp, 8, s->rx_packets);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH iproute2-next 2/6] ip: Introduce get_rtnl_link_stats_rta() to get link statistics
2018-02-02 13:10 [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
2018-02-02 13:10 ` [PATCH iproute2-next 1/6] ipaddress: Unify print_link_stats() and print_link_stats64() Serhey Popovych
@ 2018-02-02 13:10 ` Serhey Popovych
2018-02-02 13:10 ` [PATCH iproute2-next 3/6] tunnel: Split statistic getting and printing Serhey Popovych
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Serhey Popovych @ 2018-02-02 13:10 UTC (permalink / raw)
To: netdev
Assume all statistics in ip(8) represented either by IFLA_STATS64 or
IFLA_STATS is 64 bit. It is clean that we can store __u32 counters of
@struct rtnl_link_stats in __u64 counters in @struct rtnl_link_stats64.
New get_rtnl_link_stats_rta() follows __print_link_stats() behaviour on
handling of stats attribute: copy no more than size of data structure
and no less than attribute length zeroing rest.
Drop print_link_stats32() as it's functionality can be handled by 64bit
variant. Move code from __print_link_stats() to print_link_stats64() and
finally rename print_link_stats64() to __print_link_stats().
More users of introduced function will come in future.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
include/utils.h | 3 +
ip/ipaddress.c | 171 +++----------------------------------------------------
lib/utils.c | 45 +++++++++++++++
3 files changed, 56 insertions(+), 163 deletions(-)
diff --git a/include/utils.h b/include/utils.h
index 0394268..ffb2a2c 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -294,6 +294,9 @@ int make_path(const char *path, mode_t mode);
char *find_cgroup2_mount(void);
int get_command_name(const char *pid, char *comm, size_t len);
+int get_rtnl_link_stats_rta(struct rtnl_link_stats64 *stats64,
+ struct rtattr *tb[]);
+
#ifdef NEED_STRLCPY
size_t strlcpy(char *dst, const char *src, size_t size);
size_t strlcat(char *dst, const char *src, size_t size);
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index ae419f0..078c6cf 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -594,152 +594,18 @@ static void print_vf_stats64(FILE *fp, struct rtattr *vfstats)
}
}
-static void print_link_stats64(FILE *fp, const struct rtnl_link_stats64 *s,
- const struct rtattr *carrier_changes)
+static void __print_link_stats(FILE *fp, struct rtattr *tb[])
{
- if (is_json_context()) {
- open_json_object("stats64");
-
- /* RX stats */
- open_json_object("rx");
- print_uint(PRINT_JSON, "bytes", NULL, s->rx_bytes);
- print_uint(PRINT_JSON, "packets", NULL, s->rx_packets);
- print_uint(PRINT_JSON, "errors", NULL, s->rx_errors);
- print_uint(PRINT_JSON, "dropped", NULL, s->rx_dropped);
- print_uint(PRINT_JSON, "over_errors", NULL, s->rx_over_errors);
- print_uint(PRINT_JSON, "multicast", NULL, s->multicast);
- if (s->rx_compressed)
- print_uint(PRINT_JSON,
- "compressed", NULL, s->rx_compressed);
-
- /* RX error stats */
- if (show_stats > 1) {
- print_uint(PRINT_JSON,
- "length_errors",
- NULL, s->rx_length_errors);
- print_uint(PRINT_JSON,
- "crc_errors",
- NULL, s->rx_crc_errors);
- print_uint(PRINT_JSON,
- "frame_errors",
- NULL, s->rx_frame_errors);
- print_uint(PRINT_JSON,
- "fifo_errors",
- NULL, s->rx_fifo_errors);
- print_uint(PRINT_JSON,
- "missed_errors",
- NULL, s->rx_missed_errors);
- if (s->rx_nohandler)
- print_uint(PRINT_JSON,
- "nohandler", NULL, s->rx_nohandler);
- }
- close_json_object();
-
- /* TX stats */
- open_json_object("tx");
- print_uint(PRINT_JSON, "bytes", NULL, s->tx_bytes);
- print_uint(PRINT_JSON, "packets", NULL, s->tx_packets);
- print_uint(PRINT_JSON, "errors", NULL, s->tx_errors);
- print_uint(PRINT_JSON, "dropped", NULL, s->tx_dropped);
- print_uint(PRINT_JSON,
- "carrier_errors",
- NULL, s->tx_carrier_errors);
- print_uint(PRINT_JSON, "collisions", NULL, s->collisions);
- if (s->tx_compressed)
- print_uint(PRINT_JSON,
- "compressed", NULL, s->tx_compressed);
-
- /* TX error stats */
- if (show_stats > 1) {
- print_uint(PRINT_JSON,
- "aborted_errors",
- NULL, s->tx_aborted_errors);
- print_uint(PRINT_JSON,
- "fifo_errors",
- NULL, s->tx_fifo_errors);
- print_uint(PRINT_JSON,
- "window_errors",
- NULL, s->tx_window_errors);
- print_uint(PRINT_JSON,
- "heartbeat_errors",
- NULL, s->tx_heartbeat_errors);
- if (carrier_changes)
- print_uint(PRINT_JSON, "carrier_changes", NULL,
- rta_getattr_u32(carrier_changes));
- }
-
- close_json_object();
- close_json_object();
- } else {
- /* RX stats */
- fprintf(fp, " RX: bytes packets errors dropped overrun mcast %s%s",
- s->rx_compressed ? "compressed" : "", _SL_);
-
- fprintf(fp, " ");
- print_num(fp, 10, s->rx_bytes);
- print_num(fp, 8, s->rx_packets);
- print_num(fp, 7, s->rx_errors);
- print_num(fp, 7, s->rx_dropped);
- print_num(fp, 7, s->rx_over_errors);
- print_num(fp, 7, s->multicast);
- if (s->rx_compressed)
- print_num(fp, 7, s->rx_compressed);
-
- /* RX error stats */
- if (show_stats > 1) {
- fprintf(fp, "%s", _SL_);
- fprintf(fp, " RX errors: length crc frame fifo missed%s%s",
- s->rx_nohandler ? " nohandler" : "", _SL_);
- fprintf(fp, " ");
- print_num(fp, 8, s->rx_length_errors);
- print_num(fp, 7, s->rx_crc_errors);
- print_num(fp, 7, s->rx_frame_errors);
- print_num(fp, 7, s->rx_fifo_errors);
- print_num(fp, 7, s->rx_missed_errors);
- if (s->rx_nohandler)
- print_num(fp, 7, s->rx_nohandler);
- }
- fprintf(fp, "%s", _SL_);
-
- /* TX stats */
- fprintf(fp, " TX: bytes packets errors dropped carrier collsns %s%s",
- s->tx_compressed ? "compressed" : "", _SL_);
-
- fprintf(fp, " ");
- print_num(fp, 10, s->tx_bytes);
- print_num(fp, 8, s->tx_packets);
- print_num(fp, 7, s->tx_errors);
- print_num(fp, 7, s->tx_dropped);
- print_num(fp, 7, s->tx_carrier_errors);
- print_num(fp, 7, s->collisions);
- if (s->tx_compressed)
- print_num(fp, 7, s->tx_compressed);
-
- /* TX error stats */
- if (show_stats > 1) {
- fprintf(fp, "%s", _SL_);
- fprintf(fp, " TX errors: aborted fifo window heartbeat");
- if (carrier_changes)
- fprintf(fp, " transns");
- fprintf(fp, "%s", _SL_);
+ const struct rtattr *carrier_changes = tb[IFLA_CARRIER_CHANGES];
+ struct rtnl_link_stats64 _s, *s = &_s;
+ int ret;
- fprintf(fp, " ");
- print_num(fp, 8, s->tx_aborted_errors);
- print_num(fp, 7, s->tx_fifo_errors);
- print_num(fp, 7, s->tx_window_errors);
- print_num(fp, 7, s->tx_heartbeat_errors);
- if (carrier_changes)
- print_num(fp, 7,
- rta_getattr_u32(carrier_changes));
- }
- }
-}
+ ret = get_rtnl_link_stats_rta(s, tb);
+ if (ret < 0)
+ return;
-static void print_link_stats32(FILE *fp, const struct rtnl_link_stats *s,
- const struct rtattr *carrier_changes)
-{
if (is_json_context()) {
- open_json_object("stats");
+ open_json_object((ret == sizeof(*s)) ? "stats64" : "stats");
/* RX stats */
open_json_object("rx");
@@ -876,27 +742,6 @@ static void print_link_stats32(FILE *fp, const struct rtnl_link_stats *s,
}
}
-static void __print_link_stats(FILE *fp, struct rtattr **tb)
-{
- const struct rtattr *carrier_changes = tb[IFLA_CARRIER_CHANGES];
-
- if (tb[IFLA_STATS64]) {
- struct rtnl_link_stats64 stats = { 0 };
-
- memcpy(&stats, RTA_DATA(tb[IFLA_STATS64]),
- MIN(RTA_PAYLOAD(tb[IFLA_STATS64]), sizeof(stats)));
-
- print_link_stats64(fp, &stats, carrier_changes);
- } else if (tb[IFLA_STATS]) {
- struct rtnl_link_stats stats = { 0 };
-
- memcpy(&stats, RTA_DATA(tb[IFLA_STATS]),
- MIN(RTA_PAYLOAD(tb[IFLA_STATS]), sizeof(stats)));
-
- print_link_stats32(fp, &stats, carrier_changes);
- }
-}
-
static void print_link_stats(FILE *fp, struct nlmsghdr *n)
{
struct ifinfomsg *ifi = NLMSG_DATA(n);
diff --git a/lib/utils.c b/lib/utils.c
index 8e15625..d86c2ee 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1431,6 +1431,51 @@ int get_real_family(int rtm_type, int rtm_family)
return rtm_family;
}
+/* Based on copy_rtnl_link_stats() from kernel at net/core/rtnetlink.c */
+static void copy_rtnl_link_stats64(struct rtnl_link_stats64 *stats64,
+ const struct rtnl_link_stats *stats)
+{
+ __u64 *a = (__u64 *)stats64;
+ const __u32 *b = (const __u32 *)stats;
+ const __u32 *e = b + sizeof(*stats) / sizeof(*b);
+
+ while (b < e)
+ *a++ = *b++;
+}
+
+int get_rtnl_link_stats_rta(struct rtnl_link_stats64 *stats64,
+ struct rtattr *tb[])
+{
+ struct rtnl_link_stats stats;
+ void *s;
+ struct rtattr *rta;
+ int size, len;
+
+ if (tb[IFLA_STATS64]) {
+ rta = tb[IFLA_STATS64];
+ size = sizeof(struct rtnl_link_stats64);
+ s = stats64;
+ } else if (tb[IFLA_STATS]) {
+ rta = tb[IFLA_STATS];
+ size = sizeof(struct rtnl_link_stats);
+ s = &stats;
+ } else {
+ return -1;
+ }
+
+ len = RTA_PAYLOAD(rta);
+ if (len < size)
+ memset(s + len, 0, size - len);
+ else
+ len = size;
+
+ memcpy(s, RTA_DATA(rta), len);
+
+ if (s != stats64)
+ copy_rtnl_link_stats64(stats64, s);
+ return size;
+}
+
#ifdef NEED_STRLCPY
size_t strlcpy(char *dst, const char *src, size_t size)
{
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH iproute2-next 3/6] tunnel: Split statistic getting and printing
2018-02-02 13:10 [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
2018-02-02 13:10 ` [PATCH iproute2-next 1/6] ipaddress: Unify print_link_stats() and print_link_stats64() Serhey Popovych
2018-02-02 13:10 ` [PATCH iproute2-next 2/6] ip: Introduce get_rtnl_link_stats_rta() to get link statistics Serhey Popovych
@ 2018-02-02 13:10 ` Serhey Popovych
2018-02-02 13:10 ` [PATCH iproute2-next 4/6] iptunnel/ip6tunnel: Code cleanups Serhey Popovych
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Serhey Popovych @ 2018-02-02 13:10 UTC (permalink / raw)
To: netdev
This is first step to move tunnel code to use rtnl dump interface
instead of /proc/net/dev read.
Make tnl_print_stats() to accept @struct rtnl_link_stats64 parameter,
introduce tnl_get_stats() that will parse line from /proc/net/dev into
@struct rtnl_link_stats64.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ip6tunnel.c | 8 ++++++--
ip/iptunnel.c | 8 ++++++--
ip/tunnel.c | 57 +++++++++++++++++++++++++++++++++++---------------------
ip/tunnel.h | 5 ++++-
4 files changed, 52 insertions(+), 26 deletions(-)
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index 783e28a..3e99559 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -390,8 +390,12 @@ static int do_tunnels_list(struct ip6_tnl_parm2 *p)
if (!ip6_tnl_parm_match(p, &p1))
continue;
print_tunnel(&p1);
- if (show_stats)
- tnl_print_stats(ptr);
+ if (show_stats) {
+ struct rtnl_link_stats64 s;
+
+ if (!tnl_get_stats(ptr, &s))
+ tnl_print_stats(&s);
+ }
printf("\n");
}
err = 0;
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 0aa3b33..6639055 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -425,8 +425,12 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
(p->i_key && p1.i_key != p->i_key))
continue;
print_tunnel(&p1);
- if (show_stats)
- tnl_print_stats(ptr);
+ if (show_stats) {
+ struct rtnl_link_stats64 s;
+
+ if (!tnl_get_stats(ptr, &s))
+ tnl_print_stats(&s);
+ }
printf("\n");
}
err = 0;
diff --git a/ip/tunnel.c b/ip/tunnel.c
index 948d5f7..06533cf 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -307,30 +307,45 @@ void tnl_print_endpoint(const char *name, const struct rtattr *rta, int family)
}
}
-/* tnl_print_stats - print tunnel statistics
- *
- * @buf - tunnel interface's line in /proc/net/dev,
- * starting past the interface name and following colon
- */
-void tnl_print_stats(const char *buf)
+int tnl_get_stats(const char *buf, struct rtnl_link_stats64 *s)
{
- unsigned long rx_bytes, rx_packets, rx_errs, rx_drops,
- rx_fifo, rx_frame,
- tx_bytes, tx_packets, tx_errs, tx_drops,
- tx_fifo, tx_colls, tx_carrier, rx_multi;
-
- if (sscanf(buf, "%lu%lu%lu%lu%lu%lu%lu%*d%lu%lu%lu%lu%lu%lu%lu",
- &rx_bytes, &rx_packets, &rx_errs, &rx_drops,
- &rx_fifo, &rx_frame, &rx_multi,
- &tx_bytes, &tx_packets, &tx_errs, &tx_drops,
- &tx_fifo, &tx_colls, &tx_carrier) != 14)
- return;
+ /* rx */
+ __u64 *rx_bytes = &s->rx_bytes;
+ __u64 *rx_packets = &s->rx_packets;
+ __u64 *rx_errs = &s->rx_errors;
+ __u64 *rx_drops = &s->rx_dropped;
+ __u64 *rx_fifo = &s->rx_fifo_errors;
+ __u64 *rx_frame = &s->rx_frame_errors;
+ __u64 *rx_multi = &s->multicast;
+ /* tx */
+ __u64 *tx_bytes = &s->tx_bytes;
+ __u64 *tx_packets = &s->tx_packets;
+ __u64 *tx_errs = &s->tx_errors;
+ __u64 *tx_drops = &s->tx_dropped;
+ __u64 *tx_fifo = &s->tx_fifo_errors;
+ __u64 *tx_carrier = &s->tx_carrier_errors;
+ __u64 *tx_colls = &s->collisions;
+
+ if (sscanf(buf,
+ "%llu%llu%llu%llu%llu%llu%llu%*d%llu%llu%llu%llu%llu%llu%llu",
+ rx_bytes, rx_packets, rx_errs, rx_drops,
+ rx_fifo, rx_frame, rx_multi,
+ tx_bytes, tx_packets, tx_errs, tx_drops,
+ tx_fifo, tx_colls, tx_carrier) != 14)
+ return -1;
+ return 0;
+}
+
+void tnl_print_stats(const struct rtnl_link_stats64 *s)
+{
printf("%s", _SL_);
printf("RX: Packets Bytes Errors CsumErrs OutOfSeq Mcasts%s", _SL_);
- printf(" %-10ld %-12ld %-6ld %-8ld %-8ld %-8ld%s",
- rx_packets, rx_bytes, rx_errs, rx_frame, rx_fifo, rx_multi, _SL_);
+ printf(" %-10lld %-12lld %-6lld %-8lld %-8lld %-8lld%s",
+ s->rx_packets, s->rx_bytes, s->rx_errors, s->rx_frame_errors,
+ s->rx_fifo_errors, s->multicast, _SL_);
printf("TX: Packets Bytes Errors DeadLoop NoRoute NoBufs%s", _SL_);
- printf(" %-10ld %-12ld %-6ld %-8ld %-8ld %-6ld",
- tx_packets, tx_bytes, tx_errs, tx_colls, tx_carrier, tx_drops);
+ printf(" %-10lld %-12lld %-6lld %-8lld %-8lld %-6lld",
+ s->tx_packets, s->tx_bytes, s->tx_errors, s->collisions,
+ s->tx_carrier_errors, s->tx_dropped);
}
diff --git a/ip/tunnel.h b/ip/tunnel.h
index 5bd27c3..5fe488b 100644
--- a/ip/tunnel.h
+++ b/ip/tunnel.h
@@ -24,6 +24,7 @@
#include <linux/types.h>
struct rtattr;
+struct rtnl_link_stats64;
const char *tnl_strproto(__u8 proto);
@@ -39,6 +40,8 @@ void tnl_print_encap(struct rtattr *tb[],
int encap_sport, int encap_dport);
void tnl_print_endpoint(const char *name,
const struct rtattr *rta, int family);
-void tnl_print_stats(const char *buf);
+void tnl_print_stats(const struct rtnl_link_stats64 *s);
+
+int tnl_get_stats(const char *buf, struct rtnl_link_stats64 *s);
#endif
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH iproute2-next 4/6] iptunnel/ip6tunnel: Code cleanups
2018-02-02 13:10 [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
` (2 preceding siblings ...)
2018-02-02 13:10 ` [PATCH iproute2-next 3/6] tunnel: Split statistic getting and printing Serhey Popovych
@ 2018-02-02 13:10 ` Serhey Popovych
2018-02-07 2:16 ` David Ahern
2018-02-02 13:10 ` [PATCH iproute2-next 5/6] iptunnel/ip6tunnel: Use netlink to walk through tunnels list Serhey Popovych
` (2 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Serhey Popovych @ 2018-02-02 13:10 UTC (permalink / raw)
To: netdev
Use switch () instead of if () to compare tunnel type to fit into 80
columns and make code more readable. Print "\n" using fputc().
In iptunnel.c abstract tunnel parameters matching code in iptunnel.c
into ip_tunnel_parm_match() helper to conform with ip6tunnel.c.
In ip6tunnel.c no need to call ll_name_to_index() with name twice: just
use found previously index. Do not initialize @p1.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ip6tunnel.c | 30 ++++++++++++++++--------------
ip/iptunnel.c | 35 ++++++++++++++++++++++++++---------
2 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index 3e99559..0e53c23 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -357,7 +357,7 @@ static int do_tunnels_list(struct ip6_tnl_parm2 *p)
while (fgets(buf, sizeof(buf), fp) != NULL) {
char name[IFNAMSIZ];
int index, type;
- struct ip6_tnl_parm2 p1 = {};
+ struct ip6_tnl_parm2 p1;
char *ptr;
buf[sizeof(buf) - 1] = '\0';
@@ -376,16 +376,19 @@ static int do_tunnels_list(struct ip6_tnl_parm2 *p)
fprintf(stderr, "Failed to get type of \"%s\"\n", name);
continue;
}
- if (type != ARPHRD_TUNNEL6 && type != ARPHRD_IP6GRE)
+ switch (type) {
+ case ARPHRD_TUNNEL6:
+ case ARPHRD_IP6GRE:
+ break;
+ default:
continue;
+ }
ip6_tnl_parm_init(&p1, 0);
if (type == ARPHRD_IP6GRE)
p1.proto = IPPROTO_GRE;
+ p1.link = index;
strcpy(p1.name, name);
- p1.link = ll_name_to_index(p1.name);
- if (p1.link == 0)
- continue;
- if (tnl_get_ioctl(p1.name, &p1))
+ if (tnl_get_ioctl(name, &p1))
continue;
if (!ip6_tnl_parm_match(p, &p1))
continue;
@@ -396,7 +399,7 @@ static int do_tunnels_list(struct ip6_tnl_parm2 *p)
if (!tnl_get_stats(ptr, &s))
tnl_print_stats(&s);
}
- printf("\n");
+ fputc('\n', stdout);
}
err = 0;
end:
@@ -416,14 +419,13 @@ static int do_show(int argc, char **argv)
return -1;
if (!p.name[0] || show_stats)
- do_tunnels_list(&p);
- else {
- if (tnl_get_ioctl(p.name, &p))
- return -1;
- print_tunnel(&p);
- printf("\n");
- }
+ return do_tunnels_list(&p);
+
+ if (tnl_get_ioctl(p.name, &p))
+ return -1;
+ print_tunnel(&p);
+ fputc('\n', stdout);
return 0;
}
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 6639055..dba5942 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -373,6 +373,20 @@ static void print_tunnel(struct ip_tunnel_parm *p)
printf("%s Checksum output packets.", _SL_);
}
+/*
+ * @p1: user specified parameter
+ * @p2: database entry
+ */
+static int ip_tunnel_parm_match(const struct ip_tunnel_parm *p1,
+ const struct ip_tunnel_parm *p2)
+{
+ return ((!p1->link || p1->link == p2->link) &&
+ (!p1->name[0] || strcmp(p1->name, p2->name) == 0) &&
+ (!p1->iph.daddr || p1->iph.daddr == p2->iph.daddr) &&
+ (!p1->iph.saddr || p1->iph.saddr == p2->iph.saddr) &&
+ (!p1->i_key || p1->i_key == p2->i_key));
+}
+
static int do_tunnels_list(struct ip_tunnel_parm *p)
{
char buf[512];
@@ -394,7 +408,7 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
while (fgets(buf, sizeof(buf), fp) != NULL) {
char name[IFNAMSIZ];
int index, type;
- struct ip_tunnel_parm p1 = {};
+ struct ip_tunnel_parm p1;
char *ptr;
buf[sizeof(buf) - 1] = 0;
@@ -414,15 +428,18 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
fprintf(stderr, "Failed to get type of \"%s\"\n", name);
continue;
}
- if (type != ARPHRD_TUNNEL && type != ARPHRD_IPGRE && type != ARPHRD_SIT)
+ switch (type) {
+ case ARPHRD_TUNNEL:
+ case ARPHRD_IPGRE:
+ case ARPHRD_SIT:
+ break;
+ default:
continue;
+ }
+ memset(p1, 0, sizeof(p1));
if (tnl_get_ioctl(name, &p1))
continue;
- if ((p->link && p1.link != p->link) ||
- (p->name[0] && strcmp(p1.name, p->name)) ||
- (p->iph.daddr && p1.iph.daddr != p->iph.daddr) ||
- (p->iph.saddr && p1.iph.saddr != p->iph.saddr) ||
- (p->i_key && p1.i_key != p->i_key))
+ if (!ip_tunnel_parm_match(p, &p1))
continue;
print_tunnel(&p1);
if (show_stats) {
@@ -431,7 +448,7 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
if (!tnl_get_stats(ptr, &s))
tnl_print_stats(&s);
}
- printf("\n");
+ fputc('\n', stdout);
}
err = 0;
end:
@@ -456,7 +473,7 @@ static int do_show(int argc, char **argv)
return -1;
print_tunnel(&p);
- printf("\n");
+ fputc('\n', stdout);
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH iproute2-next 4/6] iptunnel/ip6tunnel: Code cleanups
2018-02-02 13:10 ` [PATCH iproute2-next 4/6] iptunnel/ip6tunnel: Code cleanups Serhey Popovych
@ 2018-02-07 2:16 ` David Ahern
2018-02-07 2:19 ` David Ahern
2018-02-07 6:15 ` Serhey Popovych
0 siblings, 2 replies; 12+ messages in thread
From: David Ahern @ 2018-02-07 2:16 UTC (permalink / raw)
To: Serhey Popovych, netdev
On 2/2/18 6:10 AM, Serhey Popovych wrote:
> @@ -414,15 +428,18 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
> fprintf(stderr, "Failed to get type of \"%s\"\n", name);
> continue;
> }
> - if (type != ARPHRD_TUNNEL && type != ARPHRD_IPGRE && type != ARPHRD_SIT)
> + switch (type) {
> + case ARPHRD_TUNNEL:
> + case ARPHRD_IPGRE:
> + case ARPHRD_SIT:
> + break;
> + default:
> continue;
> + }
> + memset(p1, 0, sizeof(p1));
Shouldn't that be &p1 for the first arg? I get a compile failure:
ip
CC iptunnel.o
CC ip6tunnel.o
iptunnel.c: In function ‘do_tunnels_list’:
iptunnel.c:439:10: error: incompatible type for argument 1 of ‘memset’
memset(p1, 0, sizeof(p1));
^~
In file included from iptunnel.c:15:0:
/usr/include/string.h:62:14: note: expected ‘void *’ but argument is of
type ‘struct ip_tunnel_parm’
extern void *memset (void *__s, int __c, size_t __n) __THROW __nonnull
((1));
^~~~~~
../config.mk:48: recipe for target 'iptunnel.o' failed
> if (tnl_get_ioctl(name, &p1))
> continue;
> - if ((p->link && p1.link != p->link) ||
> - (p->name[0] && strcmp(p1.name, p->name)) ||
> - (p->iph.daddr && p1.iph.daddr != p->iph.daddr) ||
> - (p->iph.saddr && p1.iph.saddr != p->iph.saddr) ||
> - (p->i_key && p1.i_key != p->i_key))
> + if (!ip_tunnel_parm_match(p, &p1))
> continue;
> print_tunnel(&p1);
> if (show_stats) {
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH iproute2-next 4/6] iptunnel/ip6tunnel: Code cleanups
2018-02-07 2:16 ` David Ahern
@ 2018-02-07 2:19 ` David Ahern
2018-02-07 6:20 ` Serhey Popovych
2018-02-07 6:15 ` Serhey Popovych
1 sibling, 1 reply; 12+ messages in thread
From: David Ahern @ 2018-02-07 2:19 UTC (permalink / raw)
To: Serhey Popovych, netdev
On 2/6/18 7:16 PM, David Ahern wrote:
> On 2/2/18 6:10 AM, Serhey Popovych wrote:
>> @@ -414,15 +428,18 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
>> fprintf(stderr, "Failed to get type of \"%s\"\n", name);
>> continue;
>> }
>> - if (type != ARPHRD_TUNNEL && type != ARPHRD_IPGRE && type != ARPHRD_SIT)
>> + switch (type) {
>> + case ARPHRD_TUNNEL:
>> + case ARPHRD_IPGRE:
>> + case ARPHRD_SIT:
>> + break;
>> + default:
>> continue;
>> + }
>> + memset(p1, 0, sizeof(p1));
>
> Shouldn't that be &p1 for the first arg? I get a compile failure:
>
> ip
> CC iptunnel.o
> CC ip6tunnel.o
> iptunnel.c: In function ‘do_tunnels_list’:
> iptunnel.c:439:10: error: incompatible type for argument 1 of ‘memset’
> memset(p1, 0, sizeof(p1));
> ^~
> In file included from iptunnel.c:15:0:
> /usr/include/string.h:62:14: note: expected ‘void *’ but argument is of
> type ‘struct ip_tunnel_parm’
> extern void *memset (void *__s, int __c, size_t __n) __THROW __nonnull
> ((1));
> ^~~~~~
> ../config.mk:48: recipe for target 'iptunnel.o' failed
>
Fixed by patch 5 which deletes do_tunnels_list. So why have a cleanup
patch that changes code you then delete?
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH iproute2-next 4/6] iptunnel/ip6tunnel: Code cleanups
2018-02-07 2:19 ` David Ahern
@ 2018-02-07 6:20 ` Serhey Popovych
0 siblings, 0 replies; 12+ messages in thread
From: Serhey Popovych @ 2018-02-07 6:20 UTC (permalink / raw)
To: David Ahern, netdev
[-- Attachment #1.1: Type: text/plain, Size: 1722 bytes --]
David Ahern wrote:
> On 2/6/18 7:16 PM, David Ahern wrote:
>> On 2/2/18 6:10 AM, Serhey Popovych wrote:
>>> @@ -414,15 +428,18 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
>>> fprintf(stderr, "Failed to get type of \"%s\"\n", name);
>>> continue;
>>> }
>>> - if (type != ARPHRD_TUNNEL && type != ARPHRD_IPGRE && type != ARPHRD_SIT)
>>> + switch (type) {
>>> + case ARPHRD_TUNNEL:
>>> + case ARPHRD_IPGRE:
>>> + case ARPHRD_SIT:
>>> + break;
>>> + default:
>>> continue;
>>> + }
>>> + memset(p1, 0, sizeof(p1));
>>
>> Shouldn't that be &p1 for the first arg? I get a compile failure:
>>
>> ip
>> CC iptunnel.o
>> CC ip6tunnel.o
>> iptunnel.c: In function ‘do_tunnels_list’:
>> iptunnel.c:439:10: error: incompatible type for argument 1 of ‘memset’
>> memset(p1, 0, sizeof(p1));
>> ^~
>> In file included from iptunnel.c:15:0:
>> /usr/include/string.h:62:14: note: expected ‘void *’ but argument is of
>> type ‘struct ip_tunnel_parm’
>> extern void *memset (void *__s, int __c, size_t __n) __THROW __nonnull
>> ((1));
>> ^~~~~~
>> ../config.mk:48: recipe for target 'iptunnel.o' failed
>>
>
> Fixed by patch 5 which deletes do_tunnels_list. So why have a cleanup
> patch that changes code you then delete?
>
There at least two reasons:
1) Abstract tunnel matching code into a function that will be used as
callback ->match() in upcoming change.
2) Make do_tunnels_list() ip and ipv6 variants diff contain only
real differences to show upcoming change where common
do_tunnels_list() introduced is correct.
Will update comment for this patch in v2.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH iproute2-next 4/6] iptunnel/ip6tunnel: Code cleanups
2018-02-07 2:16 ` David Ahern
2018-02-07 2:19 ` David Ahern
@ 2018-02-07 6:15 ` Serhey Popovych
1 sibling, 0 replies; 12+ messages in thread
From: Serhey Popovych @ 2018-02-07 6:15 UTC (permalink / raw)
To: David Ahern, netdev
[-- Attachment #1.1: Type: text/plain, Size: 1645 bytes --]
David Ahern wrote:
> On 2/2/18 6:10 AM, Serhey Popovych wrote:
>> @@ -414,15 +428,18 @@ static int do_tunnels_list(struct ip_tunnel_parm *p)
>> fprintf(stderr, "Failed to get type of \"%s\"\n", name);
>> continue;
>> }
>> - if (type != ARPHRD_TUNNEL && type != ARPHRD_IPGRE && type != ARPHRD_SIT)
>> + switch (type) {
>> + case ARPHRD_TUNNEL:
>> + case ARPHRD_IPGRE:
>> + case ARPHRD_SIT:
>> + break;
>> + default:
>> continue;
>> + }
>> + memset(p1, 0, sizeof(p1));
>
> Shouldn't that be &p1 for the first arg? I get a compile failure:
Yes, definitely, sorry for that. Will fix in v2.
>
> ip
> CC iptunnel.o
> CC ip6tunnel.o
> iptunnel.c: In function ‘do_tunnels_list’:
> iptunnel.c:439:10: error: incompatible type for argument 1 of ‘memset’
> memset(p1, 0, sizeof(p1));
> ^~
> In file included from iptunnel.c:15:0:
> /usr/include/string.h:62:14: note: expected ‘void *’ but argument is of
> type ‘struct ip_tunnel_parm’
> extern void *memset (void *__s, int __c, size_t __n) __THROW __nonnull
> ((1));
> ^~~~~~
> ../config.mk:48: recipe for target 'iptunnel.o' failed
>
>> if (tnl_get_ioctl(name, &p1))
>> continue;
>> - if ((p->link && p1.link != p->link) ||
>> - (p->name[0] && strcmp(p1.name, p->name)) ||
>> - (p->iph.daddr && p1.iph.daddr != p->iph.daddr) ||
>> - (p->iph.saddr && p1.iph.saddr != p->iph.saddr) ||
>> - (p->i_key && p1.i_key != p->i_key))
>> + if (!ip_tunnel_parm_match(p, &p1))
>> continue;
>> print_tunnel(&p1);
>> if (show_stats) {
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH iproute2-next 5/6] iptunnel/ip6tunnel: Use netlink to walk through tunnels list
2018-02-02 13:10 [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
` (3 preceding siblings ...)
2018-02-02 13:10 ` [PATCH iproute2-next 4/6] iptunnel/ip6tunnel: Code cleanups Serhey Popovych
@ 2018-02-02 13:10 ` Serhey Popovych
2018-02-02 13:10 ` [PATCH iproute2-next 6/6] tuntap: Use netlink to walk through tuntap list Serhey Popovych
2018-02-02 13:21 ` [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
6 siblings, 0 replies; 12+ messages in thread
From: Serhey Popovych @ 2018-02-02 13:10 UTC (permalink / raw)
To: netdev
Both tunnels use legacy /proc/net/dev interface to get tunnel device and
it's statistics. This may cause problems for cases when procfs either
not mounted or not unshare(2)d for given network namespace.
Use netlink to walk through list of tunnel devices which is network
namespace aware and provides additional information such as statistics
in the dump message.
Since both address family specific variants of do_tunnels_list() nearly
the same, except for tunnel parameters structure initialization,
matching and printing we can introduce common one in tunnel.c.
To implement address family specific parts introduce new data structure
@struct tnl_print_nlmsg_info what contains all necessary information as
well as pointers to ->init(), ->match() and ->print() callbacks.
Annotate data structures by const where appropriate.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ip6tunnel.c | 113 +++++++++++++++---------------------------------------
ip/iptunnel.c | 106 +++++++++++++-------------------------------------
ip/tunnel.c | 117 +++++++++++++++++++++++++++++++++++++++++---------------
ip/tunnel.h | 20 ++++++++--
4 files changed, 159 insertions(+), 197 deletions(-)
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index 0e53c23..c7fa082 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -67,8 +67,9 @@ static void usage(void)
exit(-1);
}
-static void print_tunnel(struct ip6_tnl_parm2 *p)
+static void print_tunnel(const void *t)
{
+ const struct ip6_tnl_parm2 *p = t;
char s1[1024];
char s2[1024];
@@ -313,13 +314,24 @@ static void ip6_tnl_parm_init(struct ip6_tnl_parm2 *p, int apply_default)
}
}
-/*
- * @p1: user specified parameter
- * @p2: database entry
- */
-static int ip6_tnl_parm_match(const struct ip6_tnl_parm2 *p1,
- const struct ip6_tnl_parm2 *p2)
+static void ip6_tnl_parm_initialize(const struct tnl_print_nlmsg_info *info)
+{
+ const struct ifinfomsg *ifi = info->ifi;
+ const struct ip6_tnl_parm2 *p1 = info->p1;
+ struct ip6_tnl_parm2 *p2 = info->p2;
+
+ ip6_tnl_parm_init(p2, 0);
+ if (ifi->ifi_type == ARPHRD_IP6GRE)
+ p2->proto = IPPROTO_GRE;
+ p2->link = ifi->ifi_index;
+ strcpy(p2->name, p1->name);
+}
+
+static bool ip6_tnl_parm_match(const struct tnl_print_nlmsg_info *info)
{
+ const struct ip6_tnl_parm2 *p1 = info->p1;
+ const struct ip6_tnl_parm2 *p2 = info->p2;
+
return ((!p1->link || p1->link == p2->link) &&
(!p1->name[0] || strcmp(p1->name, p2->name) == 0) &&
(IN6_IS_ADDR_UNSPECIFIED(&p1->laddr) ||
@@ -336,90 +348,27 @@ static int ip6_tnl_parm_match(const struct ip6_tnl_parm2 *p1,
(!p1->flags || (p1->flags & p2->flags)));
}
-static int do_tunnels_list(struct ip6_tnl_parm2 *p)
-{
- char buf[512];
- int err = -1;
- FILE *fp = fopen("/proc/net/dev", "r");
-
- if (fp == NULL) {
- perror("fopen");
- return -1;
- }
-
- /* skip two lines at the begenning of the file */
- if (!fgets(buf, sizeof(buf), fp) ||
- !fgets(buf, sizeof(buf), fp)) {
- fprintf(stderr, "/proc/net/dev read error\n");
- goto end;
- }
-
- while (fgets(buf, sizeof(buf), fp) != NULL) {
- char name[IFNAMSIZ];
- int index, type;
- struct ip6_tnl_parm2 p1;
- char *ptr;
-
- buf[sizeof(buf) - 1] = '\0';
- if ((ptr = strchr(buf, ':')) == NULL ||
- (*ptr++ = 0, sscanf(buf, "%s", name) != 1)) {
- fprintf(stderr, "Wrong format for /proc/net/dev. Giving up.\n");
- goto end;
- }
- if (p->name[0] && strcmp(p->name, name))
- continue;
- index = ll_name_to_index(name);
- if (index == 0)
- continue;
- type = ll_index_to_type(index);
- if (type == -1) {
- fprintf(stderr, "Failed to get type of \"%s\"\n", name);
- continue;
- }
- switch (type) {
- case ARPHRD_TUNNEL6:
- case ARPHRD_IP6GRE:
- break;
- default:
- continue;
- }
- ip6_tnl_parm_init(&p1, 0);
- if (type == ARPHRD_IP6GRE)
- p1.proto = IPPROTO_GRE;
- p1.link = index;
- strcpy(p1.name, name);
- if (tnl_get_ioctl(name, &p1))
- continue;
- if (!ip6_tnl_parm_match(p, &p1))
- continue;
- print_tunnel(&p1);
- if (show_stats) {
- struct rtnl_link_stats64 s;
-
- if (!tnl_get_stats(ptr, &s))
- tnl_print_stats(&s);
- }
- fputc('\n', stdout);
- }
- err = 0;
- end:
- fclose(fp);
- return err;
-}
-
static int do_show(int argc, char **argv)
{
- struct ip6_tnl_parm2 p;
+ struct ip6_tnl_parm2 p, p1;
- ll_init_map(&rth);
ip6_tnl_parm_init(&p, 0);
p.proto = 0; /* default to any */
if (parse_args(argc, argv, SIOCGETTUNNEL, &p) < 0)
return -1;
- if (!p.name[0] || show_stats)
- return do_tunnels_list(&p);
+ if (!p.name[0] || show_stats) {
+ struct tnl_print_nlmsg_info info = {
+ .p1 = &p,
+ .p2 = &p1,
+ .init = ip6_tnl_parm_initialize,
+ .match = ip6_tnl_parm_match,
+ .print = print_tunnel,
+ };
+
+ return do_tunnels_list(&info);
+ }
if (tnl_get_ioctl(p.name, &p))
return -1;
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index dba5942..1f04f95 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -286,8 +286,9 @@ static int do_del(int argc, char **argv)
return tnl_del_ioctl(tnl_defname(&p) ? : p.name, p.name, &p);
}
-static void print_tunnel(struct ip_tunnel_parm *p)
+static void print_tunnel(const void *t)
{
+ const struct ip_tunnel_parm *p = t;
struct ip_tunnel_6rd ip6rd = {};
char s1[1024];
char s2[1024];
@@ -373,13 +374,19 @@ static void print_tunnel(struct ip_tunnel_parm *p)
printf("%s Checksum output packets.", _SL_);
}
-/*
- * @p1: user specified parameter
- * @p2: database entry
- */
-static int ip_tunnel_parm_match(const struct ip_tunnel_parm *p1,
- const struct ip_tunnel_parm *p2)
+
+static void ip_tunnel_parm_initialize(const struct tnl_print_nlmsg_info *info)
+{
+ struct ip_tunnel_parm *p2 = info->p2;
+
+ memset(p2, 0, sizeof(*p2));
+}
+
+static bool ip_tunnel_parm_match(const struct tnl_print_nlmsg_info *info)
{
+ const struct ip_tunnel_parm *p1 = info->p1;
+ const struct ip_tunnel_parm *p2 = info->p2;
+
return ((!p1->link || p1->link == p2->link) &&
(!p1->name[0] || strcmp(p1->name, p2->name) == 0) &&
(!p1->iph.daddr || p1->iph.daddr == p2->iph.daddr) &&
@@ -387,87 +394,26 @@ static int ip_tunnel_parm_match(const struct ip_tunnel_parm *p1,
(!p1->i_key || p1->i_key == p2->i_key));
}
-static int do_tunnels_list(struct ip_tunnel_parm *p)
-{
- char buf[512];
- int err = -1;
- FILE *fp = fopen("/proc/net/dev", "r");
-
- if (fp == NULL) {
- perror("fopen");
- return -1;
- }
-
- /* skip header lines */
- if (!fgets(buf, sizeof(buf), fp) ||
- !fgets(buf, sizeof(buf), fp)) {
- fprintf(stderr, "/proc/net/dev read error\n");
- goto end;
- }
-
- while (fgets(buf, sizeof(buf), fp) != NULL) {
- char name[IFNAMSIZ];
- int index, type;
- struct ip_tunnel_parm p1;
- char *ptr;
-
- buf[sizeof(buf) - 1] = 0;
- ptr = strchr(buf, ':');
- if (ptr == NULL ||
- (*ptr++ = 0, sscanf(buf, "%s", name) != 1)) {
- fprintf(stderr, "Wrong format for /proc/net/dev. Giving up.\n");
- goto end;
- }
- if (p->name[0] && strcmp(p->name, name))
- continue;
- index = ll_name_to_index(name);
- if (index == 0)
- continue;
- type = ll_index_to_type(index);
- if (type == -1) {
- fprintf(stderr, "Failed to get type of \"%s\"\n", name);
- continue;
- }
- switch (type) {
- case ARPHRD_TUNNEL:
- case ARPHRD_IPGRE:
- case ARPHRD_SIT:
- break;
- default:
- continue;
- }
- memset(p1, 0, sizeof(p1));
- if (tnl_get_ioctl(name, &p1))
- continue;
- if (!ip_tunnel_parm_match(p, &p1))
- continue;
- print_tunnel(&p1);
- if (show_stats) {
- struct rtnl_link_stats64 s;
-
- if (!tnl_get_stats(ptr, &s))
- tnl_print_stats(&s);
- }
- fputc('\n', stdout);
- }
- err = 0;
- end:
- fclose(fp);
- return err;
-}
-
static int do_show(int argc, char **argv)
{
- struct ip_tunnel_parm p;
+ struct ip_tunnel_parm p, p1;
const char *basedev;
- ll_init_map(&rth);
if (parse_args(argc, argv, SIOCGETTUNNEL, &p) < 0)
return -1;
basedev = tnl_defname(&p);
- if (!basedev)
- return do_tunnels_list(&p);
+ if (!basedev) {
+ struct tnl_print_nlmsg_info info = {
+ .p1 = &p,
+ .p2 = &p1,
+ .init = ip_tunnel_parm_initialize,
+ .match = ip_tunnel_parm_match,
+ .print = print_tunnel,
+ };
+
+ return do_tunnels_list(&info);
+ }
if (tnl_get_ioctl(p.name[0] ? p.name : basedev, &p))
return -1;
diff --git a/ip/tunnel.c b/ip/tunnel.c
index 06533cf..7030995 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -33,6 +33,7 @@
#include <linux/if.h>
#include <linux/ip.h>
#include <linux/if_tunnel.h>
+#include <linux/if_arp.h>
#include "utils.h"
#include "tunnel.h"
@@ -307,37 +308,7 @@ void tnl_print_endpoint(const char *name, const struct rtattr *rta, int family)
}
}
-int tnl_get_stats(const char *buf, struct rtnl_link_stats64 *s)
-{
- /* rx */
- __u64 *rx_bytes = &s->rx_bytes;
- __u64 *rx_packets = &s->rx_packets;
- __u64 *rx_errs = &s->rx_errors;
- __u64 *rx_drops = &s->rx_dropped;
- __u64 *rx_fifo = &s->rx_fifo_errors;
- __u64 *rx_frame = &s->rx_frame_errors;
- __u64 *rx_multi = &s->multicast;
- /* tx */
- __u64 *tx_bytes = &s->tx_bytes;
- __u64 *tx_packets = &s->tx_packets;
- __u64 *tx_errs = &s->tx_errors;
- __u64 *tx_drops = &s->tx_dropped;
- __u64 *tx_fifo = &s->tx_fifo_errors;
- __u64 *tx_carrier = &s->tx_carrier_errors;
- __u64 *tx_colls = &s->collisions;
-
- if (sscanf(buf,
- "%llu%llu%llu%llu%llu%llu%llu%*d%llu%llu%llu%llu%llu%llu%llu",
- rx_bytes, rx_packets, rx_errs, rx_drops,
- rx_fifo, rx_frame, rx_multi,
- tx_bytes, tx_packets, tx_errs, tx_drops,
- tx_fifo, tx_colls, tx_carrier) != 14)
- return -1;
-
- return 0;
-}
-
-void tnl_print_stats(const struct rtnl_link_stats64 *s)
+static void tnl_print_stats(const struct rtnl_link_stats64 *s)
{
printf("%s", _SL_);
printf("RX: Packets Bytes Errors CsumErrs OutOfSeq Mcasts%s", _SL_);
@@ -349,3 +320,87 @@ void tnl_print_stats(const struct rtnl_link_stats64 *s)
s->tx_packets, s->tx_bytes, s->tx_errors, s->collisions,
s->tx_carrier_errors, s->tx_dropped);
}
+
+static int print_nlmsg_tunnel(const struct sockaddr_nl *who,
+ struct nlmsghdr *n, void *arg)
+{
+ struct tnl_print_nlmsg_info *info = arg;
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct rtattr *tb[IFLA_MAX+1];
+ const char *name, *n1;
+
+ if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
+ return 0;
+
+ if (n->nlmsg_len < NLMSG_LENGTH(sizeof(*ifi)))
+ return -1;
+
+ if (preferred_family == AF_INET) {
+ switch (ifi->ifi_type) {
+ case ARPHRD_TUNNEL:
+ case ARPHRD_IPGRE:
+ case ARPHRD_SIT:
+ break;
+ default:
+ return 0;
+ }
+ } else {
+ switch (ifi->ifi_type) {
+ case ARPHRD_TUNNEL6:
+ case ARPHRD_IP6GRE:
+ break;
+ default:
+ return 0;
+ }
+ }
+
+ parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), IFLA_PAYLOAD(n));
+
+ if (!tb[IFLA_IFNAME])
+ return 0;
+
+ name = rta_getattr_str(tb[IFLA_IFNAME]);
+
+ /* Assume p1->name[IFNAMSIZ] is first field of structure */
+ n1 = info->p1;
+ if (n1[0] && strcmp(n1, name))
+ return 0;
+
+ info->ifi = ifi;
+ info->init(info);
+
+ /* TODO: parse netlink attributes */
+ if (tnl_get_ioctl(name, info->p2))
+ return 0;
+
+ if (!info->match(info))
+ return 0;
+
+ info->print(info->p2);
+ if (show_stats) {
+ struct rtnl_link_stats64 s;
+
+ if (get_rtnl_link_stats_rta(&s, tb) <= 0)
+ return -1;
+
+ tnl_print_stats(&s);
+ }
+ fputc('\n', stdout);
+
+ return 0;
+}
+
+int do_tunnels_list(struct tnl_print_nlmsg_info *info)
+{
+ if (rtnl_wilddump_request(&rth, preferred_family, RTM_GETLINK) < 0) {
+ perror("Cannot send dump request\n");
+ return -1;
+ }
+
+ if (rtnl_dump_filter(&rth, print_nlmsg_tunnel, info) < 0) {
+ fprintf(stderr, "Dump terminated\n");
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/ip/tunnel.h b/ip/tunnel.h
index 5fe488b..e530d07 100644
--- a/ip/tunnel.h
+++ b/ip/tunnel.h
@@ -21,10 +21,25 @@
#ifndef __TUNNEL_H__
#define __TUNNEL_H__ 1
+#include <stdbool.h>
#include <linux/types.h>
struct rtattr;
-struct rtnl_link_stats64;
+struct ifinfomsg;
+
+extern struct rtnl_handle rth;
+
+struct tnl_print_nlmsg_info {
+ const struct ifinfomsg *ifi;
+ const void *p1;
+ void *p2;
+
+ void (*init)(const struct tnl_print_nlmsg_info *info);
+ bool (*match)(const struct tnl_print_nlmsg_info *info);
+ void (*print)(const void *t);
+};
+
+int do_tunnels_list(struct tnl_print_nlmsg_info *info);
const char *tnl_strproto(__u8 proto);
@@ -40,8 +55,5 @@ void tnl_print_encap(struct rtattr *tb[],
int encap_sport, int encap_dport);
void tnl_print_endpoint(const char *name,
const struct rtattr *rta, int family);
-void tnl_print_stats(const struct rtnl_link_stats64 *s);
-
-int tnl_get_stats(const char *buf, struct rtnl_link_stats64 *s);
#endif
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH iproute2-next 6/6] tuntap: Use netlink to walk through tuntap list
2018-02-02 13:10 [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
` (4 preceding siblings ...)
2018-02-02 13:10 ` [PATCH iproute2-next 5/6] iptunnel/ip6tunnel: Use netlink to walk through tunnels list Serhey Popovych
@ 2018-02-02 13:10 ` Serhey Popovych
2018-02-02 13:21 ` [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
6 siblings, 0 replies; 12+ messages in thread
From: Serhey Popovych @ 2018-02-02 13:10 UTC (permalink / raw)
To: netdev
It seems bad idea to depend on sysfs being mounted and reflected to the
current network namespace. Same applies to procfs.
Instead netlink should be used to talk to the kernel and get list of
specific network devices among with their parameters.
Support for kernel netlink message filtering by passing IFLA_INFO_KIND
in RTM_GETLINK request: if kernel does not support filtering by the kind
we will check it in reply anyway. Check for ifi->ifi_type to be either
ARPHRD_NONE or ARPHRD_ETHER to seed up things a bit without kernel level
filtering.
Unfortunately tun driver does not implement dumping it's configuration
via netlink and we still need to use read_prop() which depends on sysfs
to get additional tun device information.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/iptuntap.c | 121 +++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 91 insertions(+), 30 deletions(-)
diff --git a/ip/iptuntap.c b/ip/iptuntap.c
index 09f2be2..4628db2 100644
--- a/ip/iptuntap.c
+++ b/ip/iptuntap.c
@@ -20,6 +20,7 @@
#include <sys/ioctl.h>
#include <linux/if.h>
#include <linux/if_tun.h>
+#include <linux/if_arp.h>
#include <pwd.h>
#include <grp.h>
#include <fcntl.h>
@@ -31,6 +32,8 @@
#include "utils.h"
#include "ip_common.h"
+static const char drv_name[] = "tun";
+
#define TUNDEV "/dev/net/tun"
static void usage(void) __attribute__((noreturn));
@@ -348,43 +351,101 @@ next:
globfree(&globbuf);
}
+static int tuntap_filter_req(struct nlmsghdr *nlh, int reqlen)
+{
+ struct rtattr *linkinfo;
+ int err;
-static int do_show(int argc, char **argv)
+ linkinfo = addattr_nest(nlh, reqlen, IFLA_LINKINFO);
+
+ err = addattr_l(nlh, reqlen, IFLA_INFO_KIND,
+ drv_name, sizeof(drv_name) - 1);
+ if (err)
+ return err;
+
+ addattr_nest_end(nlh, linkinfo);
+
+ return 0;
+}
+
+static int print_tuntap(const struct sockaddr_nl *who,
+ struct nlmsghdr *n, void *arg)
{
- DIR *dir;
- struct dirent *d;
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ struct rtattr *tb[IFLA_MAX+1];
+ struct rtattr *linkinfo[IFLA_INFO_MAX+1];
+ const char *name, *kind;
long flags, owner = -1, group = -1;
- dir = opendir("/sys/class/net");
- if (!dir) {
- perror("opendir");
+ if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
+ return 0;
+
+ if (n->nlmsg_len < NLMSG_LENGTH(sizeof(*ifi)))
return -1;
+
+ switch (ifi->ifi_type) {
+ case ARPHRD_NONE:
+ case ARPHRD_ETHER:
+ break;
+ default:
+ return 0;
}
- while ((d = readdir(dir))) {
- if (d->d_name[0] == '.' &&
- (d->d_name[1] == 0 || d->d_name[1] == '.'))
- continue;
-
- if (read_prop(d->d_name, "tun_flags", &flags))
- continue;
-
- read_prop(d->d_name, "owner", &owner);
- read_prop(d->d_name, "group", &group);
-
- printf("%s:", d->d_name);
- print_flags(flags);
- if (owner != -1)
- printf(" user %ld", owner);
- if (group != -1)
- printf(" group %ld", group);
- printf("\n");
- if (show_details) {
- printf("\tAttached to processes:");
- show_processes(d->d_name);
- printf("\n");
- }
+
+ parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), IFLA_PAYLOAD(n));
+
+ if (!tb[IFLA_IFNAME])
+ return 0;
+
+ if (!tb[IFLA_LINKINFO])
+ return 0;
+
+ parse_rtattr_nested(linkinfo, IFLA_INFO_MAX, tb[IFLA_LINKINFO]);
+
+ if (!linkinfo[IFLA_INFO_KIND])
+ return 0;
+
+ kind = rta_getattr_str(linkinfo[IFLA_INFO_KIND]);
+ if (strcmp(kind, drv_name))
+ return 0;
+
+ name = rta_getattr_str(tb[IFLA_IFNAME]);
+
+ if (read_prop(name, "tun_flags", &flags))
+ return 0;
+ if (read_prop(name, "owner", &owner))
+ return 0;
+ if (read_prop(name, "group", &group))
+ return 0;
+
+ printf("%s:", name);
+ print_flags(flags);
+ if (owner != -1)
+ printf(" user %ld", owner);
+ if (group != -1)
+ printf(" group %ld", group);
+ fputc('\n', stdout);
+ if (show_details) {
+ printf("\tAttached to processes:");
+ show_processes(name);
+ fputc('\n', stdout);
}
- closedir(dir);
+
+ return 0;
+}
+
+static int do_show(int argc, char **argv)
+{
+ if (rtnl_wilddump_req_filter_fn(&rth, AF_UNSPEC, RTM_GETLINK,
+ tuntap_filter_req) < 0) {
+ perror("Cannot send dump request\n");
+ return -1;
+ }
+
+ if (rtnl_dump_filter(&rth, print_tuntap, NULL) < 0) {
+ fprintf(stderr, "Dump terminated\n");
+ return -1;
+ }
+
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list
2018-02-02 13:10 [PATCH iproute2-next 0/6] ip: Use netlink to walk through network device list Serhey Popovych
` (5 preceding siblings ...)
2018-02-02 13:10 ` [PATCH iproute2-next 6/6] tuntap: Use netlink to walk through tuntap list Serhey Popovych
@ 2018-02-02 13:21 ` Serhey Popovych
6 siblings, 0 replies; 12+ messages in thread
From: Serhey Popovych @ 2018-02-02 13:21 UTC (permalink / raw)
To: netdev
[-- Attachment #1.1: Type: text/plain, Size: 3307 bytes --]
Serhey Popovych wrote:
> In this seris I replace /proc/net/dev and /sys/class/net usage for walk
> through network device list in iptunnel/ip6tunnel and iptuntap with
> netlink dump.
>
> Following changed since RFC was sent:
>
> 1) Treat @struct rtnl_link_stats and @struct rtnl_link_stats64 as
> array with __u32 and __u64 elements respectively in
> copy_rtnl_link_stats64() as suggested by Stephen Hemminger.
>
> 2) Remove @name and @size parameters from @struct tnl_print_nlmsg_info
> since we can get them easily from other data.
>
> Testing.
> ========
>
> Following script is used to ensure I didn't broke things too much:
>
> \#!/bin/bash
>
> iproute2_dir="$1"
> iface='gre1'
>
> pushd "$iproute2_dir" &>/dev/null
>
> for i in new old; do
> DIR="/tmp/$i"
> mkdir -p "$DIR"
>
> ln -snf ip.$i ip/ip
>
> for o in '' -s -d; do
> ip/ip $o tunnel show >"$DIR/ip${o}-tunnel-show"
> ip/ip -4 $o tunnel show >"$DIR/ip-4${o}-tunnel-show"
> ip/ip -6 $o tunnel show >"$DIR/ip-6${o}-tunnel-show"
> ip/ip $o tunnel show dev "$iface" \
> >"$DIR/ip${o}-tunnel-show-$iface"
> ip/ip $o tuntap show >"$DIR/ip${o}-tuntap-show"
> done
> done
> rm -f ip/ip
>
> diff -urN /tmp/{old,new} |sed -n -Ee'/^(-{3}|\+{3})[[:space:]]+/!p'
> rc=$?
>
> popd &>/dev/null
> exit $rc
>
> Results:
> ========
>
> ...
> fopen /sys/class/net/ipip1/tun_flags: No such file or directory
> fopen /sys/class/net/ipip2/tun_flags: No such file or directory
> fopen /sys/class/net/gre10/tun_flags: No such file or directory
> ...
Note that this comes from ip.old (before series applied) on 3.2 kernel.
> diff -urN /tmp/old/ip-d-tuntap-show /tmp/new/ip-d-tuntap-show
> @@ -1,4 +1,4 @@
> -tun1: tap user 1004 group 27
> - Attached to processes:
> tun0: tun user 1000 group 27
> Attached to processes:
> +tun1: tap user 1004 group 27
> + Attached to processes:
> diff -urN /tmp/old/ip-s-tuntap-show /tmp/new/ip-s-tuntap-show
> @@ -1,2 +1,2 @@
> -tun1: tap user 1004 group 27
> tun0: tun user 1000 group 27
> +tun1: tap user 1004 group 27
> diff -urN /tmp/old/ip-tuntap-show /tmp/new/ip-tuntap-show
> @@ -1,2 +1,2 @@
> -tun1: tap user 1004 group 27
> tun0: tun user 1000 group 27
> +tun1: tap user 1004 group 27
>
> So basically only print order for ip tuntap get changes. Rest is intact.
>
> Thanks,
> Serhii
>
> Serhey Popovych (6):
> ipaddress: Unify print_link_stats() and print_link_stats64()
> ip: Introduce get_rtnl_link_stats_rta() to get link statistics
> tunnel: Split statistic getting and printing
> iptunnel/ip6tunnel: Code cleanups
> iptunnel/ip6tunnel: Use netlink to walk through tunnels list
> tuntap: Use netlink to walk through tuntap list
>
> include/utils.h | 3 +
> ip/ip6tunnel.c | 115 +++++++++++----------------------
> ip/ipaddress.c | 189 ++++---------------------------------------------------
> ip/iptunnel.c | 93 +++++++++------------------
> ip/iptuntap.c | 121 ++++++++++++++++++++++++++---------
> ip/tunnel.c | 114 ++++++++++++++++++++++++++-------
> ip/tunnel.h | 17 ++++-
> lib/utils.c | 45 +++++++++++++
> 8 files changed, 324 insertions(+), 373 deletions(-)
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread