Added additional counters in a bat_stats structure, which are exported through the ethtool api. The counters are specific to batman-adv and includes: forwarded packets management packets (OGMs at this point) translation table packets distributed arp table packets
I would like you all to check if the increments are added at the right locations and also if more counters would be relevant. (E.g. in bridge loop avoidance code?)
This is the reworked approach from the previous stat counters patch I send, where ethtool stats was suggested.
Signed-off-by: Martin Hundebøll martin@hundeboll.net --- bat_iv_ogm.c | 6 ++++- distributed-arp-table.c | 8 ++++++- routing.c | 9 +++++++ soft-interface.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++ translation-table.c | 8 +++++++ types.h | 17 +++++++++++++ 6 files changed, 107 insertions(+), 2 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 994369d..288326a 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -196,8 +196,10 @@ static void bat_iv_ogm_send_to_if(struct forw_packet *forw_packet,
/* create clone because function is called more than once */ skb = skb_clone(forw_packet->skb, GFP_ATOMIC); - if (skb) + if (skb) { + bat_priv->bat_stats.mgmt_tx++; send_skb_packet(skb, hard_iface, broadcast_addr); + } }
/* send a batman ogm packet */ @@ -956,6 +958,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, if (batman_ogm_packet->header.packet_type != BAT_IV_OGM) return;
+ bat_priv->bat_stats.mgmt_rx++; + /* could be changed by schedule_own_packet() */ if_incoming_seqno = atomic_read(&if_incoming->seqno);
diff --git a/distributed-arp-table.c b/distributed-arp-table.c index b43bece..cc3182e 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -395,9 +395,11 @@ bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
netif_rx(skb_new); bat_dbg(DBG_DAT, bat_priv, "ARP request replied locally\n"); - } else + } else { /* Send the request on the DHT */ + bat_priv->bat_stats.dat_request_tx++; ret = dht_send_data(bat_priv, skb, ip_dst, BAT_P_DAT_DHT_GET); + } out: if (n) neigh_release(n); @@ -450,6 +452,8 @@ bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv, if (!skb_new) goto out;
+ bat_priv->bat_stats.dat_reply_tx++; + unicast_4addr_send_skb(skb_new, bat_priv, BAT_P_DAT_CACHE_REPLY);
ret = true; @@ -488,6 +492,8 @@ bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv, arp_neigh_update(bat_priv, ip_src, hw_src); arp_neigh_update(bat_priv, ip_dst, hw_dst);
+ bat_priv->bat_stats.dat_reply_tx++; + /* Send the ARP reply to the candidates for both the IP addresses we * fetched from the ARP reply */ dht_send_data(bat_priv, skb, ip_src, BAT_P_DAT_DHT_PUT); diff --git a/routing.c b/routing.c index 795d3af..28ce7b9 100644 --- a/routing.c +++ b/routing.c @@ -600,6 +600,8 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if)
switch (tt_query->flags & TT_QUERY_TYPE_MASK) { case TT_REQUEST: + bat_priv->bat_stats.tt_request_rx++; + /* If we cannot provide an answer the tt_request is * forwarded */ if (!send_tt_response(bat_priv, tt_query)) { @@ -612,6 +614,8 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if) } break; case TT_RESPONSE: + bat_priv->bat_stats.tt_response_rx++; + if (is_my_mac(tt_query->dst)) { /* packet needs to be linearized to access the TT * changes */ @@ -663,6 +667,8 @@ int recv_roam_adv(struct sk_buff *skb, struct hard_iface *recv_if) if (is_broadcast_ether_addr(ethhdr->h_source)) goto out;
+ bat_priv->bat_stats.tt_roam_adv_rx++; + roam_adv_packet = (struct roam_adv_packet *)skb->data;
if (!is_my_mac(roam_adv_packet->dst)) @@ -869,6 +875,9 @@ static int route_unicast_packet(struct sk_buff *skb, struct hard_iface *recv_if) /* decrement ttl */ unicast_packet->header.ttl--;
+ /* Update stats counter */ + bat_priv->bat_stats.forward++; + /* route it */ send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = NET_RX_SUCCESS; diff --git a/soft-interface.c b/soft-interface.c index 92137af..095ee83 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -46,6 +46,10 @@ static void bat_get_drvinfo(struct net_device *dev, static u32 bat_get_msglevel(struct net_device *dev); static void bat_set_msglevel(struct net_device *dev, u32 value); static u32 bat_get_link(struct net_device *dev); +static void bat_get_strings(struct net_device *dev, u32 stringset, u8 *data); +static void bat_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, u64 *data); +static int bat_get_sset_count(struct net_device *dev, int stringset);
static const struct ethtool_ops bat_ethtool_ops = { .get_settings = bat_get_settings, @@ -53,6 +57,9 @@ static const struct ethtool_ops bat_ethtool_ops = { .get_msglevel = bat_get_msglevel, .set_msglevel = bat_set_msglevel, .get_link = bat_get_link, + .get_strings = bat_get_strings, + .get_ethtool_stats = bat_get_ethtool_stats, + .get_sset_count = bat_get_sset_count, };
int my_skb_head_push(struct sk_buff *skb, unsigned int len) @@ -498,3 +505,57 @@ static u32 bat_get_link(struct net_device *dev) { return 1; } + +/* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 */ +static const struct { + const char name[ETH_GSTRING_LEN]; +} bat_stats_strings[] = { + { "forward" }, + { "mgmt_tx" }, + { "mgmt_rx" }, + { "tt_request_tx" }, + { "tt_request_rx" }, + { "tt_response_tx" }, + { "tt_response_rx" }, + { "tt_roam_adv_tx" }, + { "tt_roam_adv_rx" }, + { "dat_request_tx" }, + { "dat_request_rx" }, + { "dat_reply_tx" }, + { "dat_reply_rx" }, +}; +#define BAT_STATS_NUM 13 + +static void bat_get_strings(struct net_device *dev, u32 stringset, u8 *data) +{ + if (stringset == ETH_SS_STATS) + memcpy(data, bat_stats_strings, sizeof(bat_stats_strings)); +} + +static void bat_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, u64 *data) +{ + struct bat_priv *bat_priv = netdev_priv(dev); + + data[0] = bat_priv->bat_stats.forward; + data[1] = bat_priv->bat_stats.mgmt_tx; + data[2] = bat_priv->bat_stats.mgmt_rx; + data[3] = bat_priv->bat_stats.tt_request_tx; + data[4] = bat_priv->bat_stats.tt_request_rx; + data[5] = bat_priv->bat_stats.tt_response_tx; + data[6] = bat_priv->bat_stats.tt_response_rx; + data[7] = bat_priv->bat_stats.tt_roam_adv_tx; + data[8] = bat_priv->bat_stats.tt_roam_adv_rx; + data[9] = bat_priv->bat_stats.dat_request_tx; + data[10] = bat_priv->bat_stats.dat_request_rx; + data[11] = bat_priv->bat_stats.dat_reply_tx; + data[12] = bat_priv->bat_stats.dat_reply_rx; +} + +static int bat_get_sset_count(struct net_device *dev, int stringset) +{ + if (stringset == ETH_SS_STATS) + return BAT_STATS_NUM; + + return -EOPNOTSUPP; +} diff --git a/translation-table.c b/translation-table.c index b3e608a..aa0afb1 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1352,6 +1352,8 @@ static int send_tt_request(struct bat_priv *bat_priv, dst_orig_node->orig, neigh_node->addr, (full_table ? 'F' : '.'));
+ bat_priv->bat_stats.tt_request_tx++; + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = 0;
@@ -1476,6 +1478,8 @@ static bool send_other_tt_response(struct bat_priv *bat_priv, res_dst_orig_node->orig, neigh_node->addr, req_dst_orig_node->orig, req_ttvn);
+ bat_priv->bat_stats.tt_response_tx++; + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = true; goto out; @@ -1592,6 +1596,8 @@ static bool send_my_tt_response(struct bat_priv *bat_priv, orig_node->orig, neigh_node->addr, (tt_response->flags & TT_FULL_TABLE ? 'F' : '.'));
+ bat_priv->bat_stats.tt_response_tx++; + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = true; goto out; @@ -1891,6 +1897,8 @@ static void send_roam_adv(struct bat_priv *bat_priv, uint8_t *client, "Sending ROAMING_ADV to %pM (client %pM) via %pM\n", orig_node->orig, client, neigh_node->addr);
+ bat_priv->bat_stats.tt_roam_adv_tx++; + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = 0;
diff --git a/types.h b/types.h index 15f538a..c4c1e13 100644 --- a/types.h +++ b/types.h @@ -162,9 +162,26 @@ struct bcast_duplist_entry { }; #endif
+struct bat_stats { + uint64_t forward; + uint64_t mgmt_tx; + uint64_t mgmt_rx; + uint64_t tt_request_tx; + uint64_t tt_request_rx; + uint64_t tt_response_tx; + uint64_t tt_response_rx; + uint64_t tt_roam_adv_tx; + uint64_t tt_roam_adv_rx; + uint64_t dat_request_tx; + uint64_t dat_request_rx; + uint64_t dat_reply_tx; + uint64_t dat_reply_rx; +}; + struct bat_priv { atomic_t mesh_state; struct net_device_stats stats; + struct bat_stats bat_stats; atomic_t aggregated_ogms; /* boolean */ atomic_t bonding; /* boolean */ atomic_t fragmentation; /* boolean */
On Tue, Apr 17, 2012 at 01:24:55 +0200, Martin Hundebøll wrote:
@@ -488,6 +492,8 @@ bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv, arp_neigh_update(bat_priv, ip_src, hw_src); arp_neigh_update(bat_priv, ip_dst, hw_dst);
- bat_priv->bat_stats.dat_reply_tx++;
- /* Send the ARP reply to the candidates for both the IP addresses we
dht_send_data(bat_priv, skb, ip_src, BAT_P_DAT_DHT_PUT);
- fetched from the ARP reply */
Actually in dht_send_data() the skb is cloned and sent multiple times. Should we count all the skbs that we send out?
@@ -498,3 +505,57 @@ static u32 bat_get_link(struct net_device *dev) { return 1; }
+/* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 */ +static const struct {
- const char name[ETH_GSTRING_LEN];
+} bat_stats_strings[] = {
- { "forward" },
- { "mgmt_tx" },
- { "mgmt_rx" },
- { "tt_request_tx" },
- { "tt_request_rx" },
- { "tt_response_tx" },
- { "tt_response_rx" },
- { "tt_roam_adv_tx" },
- { "tt_roam_adv_rx" },
- { "dat_request_tx" },
- { "dat_request_rx" },
- { "dat_reply_tx" },
- { "dat_reply_rx" },
+}; +#define BAT_STATS_NUM 13
Do you really need internal { } ? And why not using const char name* instead of name[N] ? All the strings are const char * and they are already in the constant space (however it is called).
+static void bat_get_ethtool_stats(struct net_device *dev,
struct ethtool_stats *stats, u64 *data)+{
- struct bat_priv *bat_priv = netdev_priv(dev);
- data[0] = bat_priv->bat_stats.forward;
- data[1] = bat_priv->bat_stats.mgmt_tx;
- data[2] = bat_priv->bat_stats.mgmt_rx;
- data[3] = bat_priv->bat_stats.tt_request_tx;
- data[4] = bat_priv->bat_stats.tt_request_rx;
- data[5] = bat_priv->bat_stats.tt_response_tx;
- data[6] = bat_priv->bat_stats.tt_response_rx;
- data[7] = bat_priv->bat_stats.tt_roam_adv_tx;
- data[8] = bat_priv->bat_stats.tt_roam_adv_rx;
- data[9] = bat_priv->bat_stats.dat_request_tx;
- data[10] = bat_priv->bat_stats.dat_request_rx;
- data[11] = bat_priv->bat_stats.dat_reply_tx;
- data[12] = bat_priv->bat_stats.dat_reply_rx;
+}
(guess mode ON) why using u64? what if the arch is 32bit? are the values correctly assigned anyway? what about longer addresses (do they exist?)? As Sven suggested on IRC, you probably want to use uintptr_t instead.
--- a/types.h +++ b/types.h @@ -162,9 +162,26 @@ struct bcast_duplist_entry { }; #endif
+struct bat_stats {
- uint64_t forward;
- uint64_t mgmt_tx;
- uint64_t mgmt_rx;
- uint64_t tt_request_tx;
- uint64_t tt_request_rx;
- uint64_t tt_response_tx;
- uint64_t tt_response_rx;
- uint64_t tt_roam_adv_tx;
- uint64_t tt_roam_adv_rx;
- uint64_t dat_request_tx;
- uint64_t dat_request_rx;
- uint64_t dat_reply_tx;
- uint64_t dat_reply_rx;
+};
see previous comment.
One general comment: be consistent with integer types. I would suggest to use uint*_t only, instead of mixing u* and uint*_t.
The last stuff we have to understand is if we can really use shared counters without having them as atomic_t or per_cpu variable...
Sven linked me this page: http://lwn.net/Articles/22911/ it seems that per_cpu variables should be used for network stats..but the effort to use them seems to be not negligible :-P
Cheers,
On Tue, Apr 17, 2012 at 09:12:42AM +0200, Antonio Quartulli wrote:
On Tue, Apr 17, 2012 at 01:24:55 +0200, Martin Hundebøll wrote:
+static void bat_get_ethtool_stats(struct net_device *dev,
struct ethtool_stats *stats, u64 *data)+{
- struct bat_priv *bat_priv = netdev_priv(dev);
- data[0] = bat_priv->bat_stats.forward;
- data[1] = bat_priv->bat_stats.mgmt_tx;
- data[2] = bat_priv->bat_stats.mgmt_rx;
- data[3] = bat_priv->bat_stats.tt_request_tx;
- data[4] = bat_priv->bat_stats.tt_request_rx;
- data[5] = bat_priv->bat_stats.tt_response_tx;
- data[6] = bat_priv->bat_stats.tt_response_rx;
- data[7] = bat_priv->bat_stats.tt_roam_adv_tx;
- data[8] = bat_priv->bat_stats.tt_roam_adv_rx;
- data[9] = bat_priv->bat_stats.dat_request_tx;
- data[10] = bat_priv->bat_stats.dat_request_rx;
- data[11] = bat_priv->bat_stats.dat_reply_tx;
- data[12] = bat_priv->bat_stats.dat_reply_rx;
+}
(guess mode ON) why using u64? what if the arch is 32bit? are the values correctly assigned anyway? what about longer addresses (do they exist?)? As Sven suggested on IRC, you probably want to use uintptr_t instead.
Sorry drop this part. I need to sleep more. Damn.
Hi Antonio,
Thank you for your comments.
On 04/17/2012 09:12 AM, Antonio Quartulli wrote:
On Tue, Apr 17, 2012 at 01:24:55 +0200, Martin Hundebøll wrote:
@@ -488,6 +492,8 @@ bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv, arp_neigh_update(bat_priv, ip_src, hw_src); arp_neigh_update(bat_priv, ip_dst, hw_dst);
- bat_priv->bat_stats.dat_reply_tx++;
- /* Send the ARP reply to the candidates for both the IP addresses we
dht_send_data(bat_priv, skb, ip_src, BAT_P_DAT_DHT_PUT);
- fetched from the ARP reply */
Actually in dht_send_data() the skb is cloned and sent multiple times. Should we count all the skbs that we send out?
I would say that it should be up to the author of the code :)
@@ -498,3 +505,57 @@ static u32 bat_get_link(struct net_device *dev) { return 1; }
+/* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 */ +static const struct {
- const char name[ETH_GSTRING_LEN];
+} bat_stats_strings[] = {
- { "forward" },
- { "mgmt_tx" },
- { "mgmt_rx" },
- { "tt_request_tx" },
- { "tt_request_rx" },
- { "tt_response_tx" },
- { "tt_response_rx" },
- { "tt_roam_adv_tx" },
- { "tt_roam_adv_rx" },
- { "dat_request_tx" },
- { "dat_request_rx" },
- { "dat_reply_tx" },
- { "dat_reply_rx" },
+}; +#define BAT_STATS_NUM 13
Do you really need internal { } ? And why not using const char name* instead of name[N] ? All the strings are const char * and they are already in the constant space (however it is called).
As described on IRC, get_ethtool_stats() expects the strings to be aligned to ETH_GSTRING_LEN and in this way, we can copy all the strings in one chunk.
+static void bat_get_ethtool_stats(struct net_device *dev,
struct ethtool_stats *stats, u64 *data)+{
- struct bat_priv *bat_priv = netdev_priv(dev);
- data[0] = bat_priv->bat_stats.forward;
- data[1] = bat_priv->bat_stats.mgmt_tx;
- data[2] = bat_priv->bat_stats.mgmt_rx;
- data[3] = bat_priv->bat_stats.tt_request_tx;
- data[4] = bat_priv->bat_stats.tt_request_rx;
- data[5] = bat_priv->bat_stats.tt_response_tx;
- data[6] = bat_priv->bat_stats.tt_response_rx;
- data[7] = bat_priv->bat_stats.tt_roam_adv_tx;
- data[8] = bat_priv->bat_stats.tt_roam_adv_rx;
- data[9] = bat_priv->bat_stats.dat_request_tx;
- data[10] = bat_priv->bat_stats.dat_request_rx;
- data[11] = bat_priv->bat_stats.dat_reply_tx;
- data[12] = bat_priv->bat_stats.dat_reply_rx;
+}
(guess mode ON) why using u64? what if the arch is 32bit? are the values correctly assigned anyway? what about longer addresses (do they exist?)? As Sven suggested on IRC, you probably want to use uintptr_t instead.
As you wrote in your later email, you should get more sleep :)
--- a/types.h +++ b/types.h @@ -162,9 +162,26 @@ struct bcast_duplist_entry { }; #endif
+struct bat_stats {
- uint64_t forward;
- uint64_t mgmt_tx;
- uint64_t mgmt_rx;
- uint64_t tt_request_tx;
- uint64_t tt_request_rx;
- uint64_t tt_response_tx;
- uint64_t tt_response_rx;
- uint64_t tt_roam_adv_tx;
- uint64_t tt_roam_adv_rx;
- uint64_t dat_request_tx;
- uint64_t dat_request_rx;
- uint64_t dat_reply_tx;
- uint64_t dat_reply_rx;
+};
see previous comment.
One general comment: be consistent with integer types. I would suggest to use uint*_t only, instead of mixing u* and uint*_t.
Okay, so if you change bat_get_strings() and bat_get_ethtool_stats() to take uint64_t it is OK, although it is called with u64? (This is what I would prefer.)
The last stuff we have to understand is if we can really use shared counters without having them as atomic_t or per_cpu variable...
Sven linked me this page: http://lwn.net/Articles/22911/ it seems that per_cpu variables should be used for network stats..but the effort to use them seems to be not negligible :-P
If we don't mind the performance penalty from using atomic_t, that can easily be changed. But keep in mind that I plan to add counters that are incremented for each and every packet.
To use per_cpu counters, I need to make dynamic allocations of these, which I would gladly do. I just didn't want to over complicate things in the first edition :)
Cheers!
On Tuesday, April 17, 2012 01:24:55 Martin Hundebøll wrote:
Added additional counters in a bat_stats structure, which are exported through the ethtool api. The counters are specific to batman-adv and includes: forwarded packets management packets (OGMs at this point) translation table packets distributed arp table packets
Looks very good! A few questions though:
@@ -869,6 +875,9 @@ static int route_unicast_packet(struct sk_buff *skb, struct hard_iface *recv_if) /* decrement ttl */ unicast_packet->header.ttl--;
- /* Update stats counter */
- bat_priv->bat_stats.forward++;
Here we only count the number of packets. Would it be possible to also count the number of bytes ? Similar to tx_packets and tx_bytes ? Same for management tx/bytes.
+struct bat_stats {
- uint64_t forward;
- uint64_t mgmt_tx;
- uint64_t mgmt_rx;
- uint64_t tt_request_tx;
- uint64_t tt_request_rx;
- uint64_t tt_response_tx;
- uint64_t tt_response_rx;
- uint64_t tt_roam_adv_tx;
- uint64_t tt_roam_adv_rx;
- uint64_t dat_request_tx;
- uint64_t dat_request_rx;
- uint64_t dat_reply_tx;
- uint64_t dat_reply_rx;
+};
How do we handle code segments that are not compiled into the module ? We simply leave this counters hanging around at 0 ?
Regards, Marek
Hi Marek,
On 04/17/2012 10:19 AM, Marek Lindner wrote:
On Tuesday, April 17, 2012 01:24:55 Martin Hundebøll wrote:
Added additional counters in a bat_stats structure, which are exported through the ethtool api. The counters are specific to batman-adv and includes: forwarded packets management packets (OGMs at this point) translation table packets distributed arp table packets
Looks very good! A few questions though:
@@ -869,6 +875,9 @@ static int route_unicast_packet(struct sk_buff *skb, struct hard_iface *recv_if) /* decrement ttl */ unicast_packet->header.ttl--;
- /* Update stats counter */
- bat_priv->bat_stats.forward++;
Here we only count the number of packets. Would it be possible to also count the number of bytes ? Similar to tx_packets and tx_bytes ? Same for management tx/bytes.
Sure, if we want it, I can make it :)
+struct bat_stats {
- uint64_t forward;
- uint64_t mgmt_tx;
- uint64_t mgmt_rx;
- uint64_t tt_request_tx;
- uint64_t tt_request_rx;
- uint64_t tt_response_tx;
- uint64_t tt_response_rx;
- uint64_t tt_roam_adv_tx;
- uint64_t tt_roam_adv_rx;
- uint64_t dat_request_tx;
- uint64_t dat_request_rx;
- uint64_t dat_reply_tx;
- uint64_t dat_reply_rx;
+};
How do we handle code segments that are not compiled into the module ? We simply leave this counters hanging around at 0 ?
As I understand it, ethtool doesn't mind "unused" counters in general, so it comes down to the memory footprint of batman-adv. Do we want to clutter the code with ifdef's to save the memory?
On Tue, Apr 17, 2012 at 03:24:48PM +0200, Martin Hundebøll wrote:
+struct bat_stats {
- uint64_t forward;
- uint64_t mgmt_tx;
- uint64_t mgmt_rx;
- uint64_t tt_request_tx;
- uint64_t tt_request_rx;
- uint64_t tt_response_tx;
- uint64_t tt_response_rx;
- uint64_t tt_roam_adv_tx;
- uint64_t tt_roam_adv_rx;
- uint64_t dat_request_tx;
- uint64_t dat_request_rx;
- uint64_t dat_reply_tx;
- uint64_t dat_reply_rx;
+};
How do we handle code segments that are not compiled into the module ? We simply leave this counters hanging around at 0 ?
As I understand it, ethtool doesn't mind "unused" counters in general, so it comes down to the memory footprint of batman-adv. Do we want to clutter the code with ifdef's to save the memory?
As we do for all the other component-private fields in bat_priv, I'd say that it would be better to do the same here by using ifdefs.
Cheers,
Added additional counters in a bat_stats structure, which are exported through the ethtool api. The counters are specific to batman-adv and includes: forwarded packets management packets (OGMs at this point) translation table packets distributed arp table packets
I would like you all to check if the increments are added at the right locations and also if more counters would be relevant. (E.g. in bridge loop avoidance code?)
This is the reworked approach from the previous stat counters patch I send, where ethtool stats was suggested.
v2: Changed to per_cpu variables. Added ifdefs for optional submodules.
Signed-off-by: Martin Hundebøll martin@hundeboll.net --- bat_iv_ogm.c | 6 +++- distributed-arp-table.c | 8 ++++- main.h | 22 +++++++++++++ routing.c | 9 ++++++ soft-interface.c | 81 +++++++++++++++++++++++++++++++++++++++++++++-- translation-table.c | 8 +++++ types.h | 19 +++++++++++ 7 files changed, 149 insertions(+), 4 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 994369d..327af94 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -196,8 +196,10 @@ static void bat_iv_ogm_send_to_if(struct forw_packet *forw_packet,
/* create clone because function is called more than once */ skb = skb_clone(forw_packet->skb, GFP_ATOMIC); - if (skb) + if (skb) { + inc_counter(bat_priv, mgmt_tx); send_skb_packet(skb, hard_iface, broadcast_addr); + } }
/* send a batman ogm packet */ @@ -956,6 +958,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, if (batman_ogm_packet->header.packet_type != BAT_IV_OGM) return;
+ inc_counter(bat_priv, mgmt_rx); + /* could be changed by schedule_own_packet() */ if_incoming_seqno = atomic_read(&if_incoming->seqno);
diff --git a/distributed-arp-table.c b/distributed-arp-table.c index b43bece..b771162 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -395,9 +395,11 @@ bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
netif_rx(skb_new); bat_dbg(DBG_DAT, bat_priv, "ARP request replied locally\n"); - } else + } else { /* Send the request on the DHT */ + inc_counter(bat_priv, dat_request_tx); ret = dht_send_data(bat_priv, skb, ip_dst, BAT_P_DAT_DHT_GET); + } out: if (n) neigh_release(n); @@ -450,6 +452,8 @@ bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv, if (!skb_new) goto out;
+ inc_counter(bat_priv, dat_reply_tx); + unicast_4addr_send_skb(skb_new, bat_priv, BAT_P_DAT_CACHE_REPLY);
ret = true; @@ -488,6 +492,8 @@ bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv, arp_neigh_update(bat_priv, ip_src, hw_src); arp_neigh_update(bat_priv, ip_dst, hw_dst);
+ inc_counter(bat_priv, dat_reply_tx); + /* Send the ARP reply to the candidates for both the IP addresses we * fetched from the ARP reply */ dht_send_data(bat_priv, skb, ip_src, BAT_P_DAT_DHT_PUT); diff --git a/main.h b/main.h index c8bfe28..015b4fa 100644 --- a/main.h +++ b/main.h @@ -150,6 +150,7 @@ enum dbg_level { #include <linux/kthread.h> /* kernel threads */ #include <linux/pkt_sched.h> /* schedule types */ #include <linux/workqueue.h> /* workqueue */ +#include <linux/percpu.h> #include <linux/slab.h> #include <net/sock.h> /* struct sock */ #include <linux/jiffies.h> @@ -259,4 +260,25 @@ static inline bool has_timed_out(unsigned long timestamp, unsigned int timeout) _dummy > smallest_signed_int(_dummy); }) #define seq_after(x, y) seq_before(y, x)
+/* Stop preemption on local cpu and increment the named counter */ +#define inc_counter(__bp, __counter) \ + do { \ + struct bat_stats *__bs; \ + int __cpu; \ + __cpu = get_cpu(); \ + __bs = per_cpu_ptr(__bp->bat_stats_cpu, __cpu); \ + __bs->__counter++; \ + put_cpu(); \ + } while (0); + +#define sum_counter(__bp, __counter, __sum) \ + do { \ + struct bat_stats *__bs; \ + int __cpu; \ + for_each_possible_cpu(__cpu) { \ + __bs = per_cpu_ptr(__bp->bat_stats_cpu, __cpu); \ + __sum += __bs->__counter; \ + } \ + } while (0); + #endif /* _NET_BATMAN_ADV_MAIN_H_ */ diff --git a/routing.c b/routing.c index 795d3af..bb5b338 100644 --- a/routing.c +++ b/routing.c @@ -600,6 +600,8 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if)
switch (tt_query->flags & TT_QUERY_TYPE_MASK) { case TT_REQUEST: + inc_counter(bat_priv, tt_request_rx); + /* If we cannot provide an answer the tt_request is * forwarded */ if (!send_tt_response(bat_priv, tt_query)) { @@ -612,6 +614,8 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if) } break; case TT_RESPONSE: + inc_counter(bat_priv, tt_response_rx); + if (is_my_mac(tt_query->dst)) { /* packet needs to be linearized to access the TT * changes */ @@ -663,6 +667,8 @@ int recv_roam_adv(struct sk_buff *skb, struct hard_iface *recv_if) if (is_broadcast_ether_addr(ethhdr->h_source)) goto out;
+ inc_counter(bat_priv, tt_roam_adv_rx); + roam_adv_packet = (struct roam_adv_packet *)skb->data;
if (!is_my_mac(roam_adv_packet->dst)) @@ -869,6 +875,9 @@ static int route_unicast_packet(struct sk_buff *skb, struct hard_iface *recv_if) /* decrement ttl */ unicast_packet->header.ttl--;
+ /* Update stats counter */ + inc_counter(bat_priv, forward); + /* route it */ send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = NET_RX_SUCCESS; diff --git a/soft-interface.c b/soft-interface.c index 92137af..b510d2e 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -46,6 +46,10 @@ static void bat_get_drvinfo(struct net_device *dev, static u32 bat_get_msglevel(struct net_device *dev); static void bat_set_msglevel(struct net_device *dev, u32 value); static u32 bat_get_link(struct net_device *dev); +static void bat_get_strings(struct net_device *dev, u32 stringset, u8 *data); +static void bat_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, u64 *data); +static int bat_get_sset_count(struct net_device *dev, int stringset);
static const struct ethtool_ops bat_ethtool_ops = { .get_settings = bat_get_settings, @@ -53,6 +57,9 @@ static const struct ethtool_ops bat_ethtool_ops = { .get_msglevel = bat_get_msglevel, .set_msglevel = bat_set_msglevel, .get_link = bat_get_link, + .get_strings = bat_get_strings, + .get_ethtool_stats = bat_get_ethtool_stats, + .get_sset_count = bat_get_sset_count, };
int my_skb_head_push(struct sk_buff *skb, unsigned int len) @@ -411,13 +418,17 @@ struct net_device *softif_create(const char *name) bat_priv->primary_if = NULL; bat_priv->num_ifaces = 0;
+ bat_priv->bat_stats_cpu = alloc_percpu(struct bat_stats); + if (!bat_priv->bat_stats_cpu) + goto unreg_soft_iface; + ret = bat_algo_select(bat_priv, bat_routing_algo); if (ret < 0) - goto unreg_soft_iface; + goto free_bat_stats;
ret = sysfs_add_meshif(soft_iface); if (ret < 0) - goto unreg_soft_iface; + goto free_bat_stats;
ret = debugfs_add_meshif(soft_iface); if (ret < 0) @@ -433,6 +444,8 @@ unreg_debugfs: debugfs_del_meshif(soft_iface); unreg_sysfs: sysfs_del_meshif(soft_iface); +free_bat_stats: + free_percpu(bat_priv->bat_stats_cpu); unreg_soft_iface: unregister_netdevice(soft_iface); return NULL; @@ -445,6 +458,8 @@ out:
void softif_destroy(struct net_device *soft_iface) { + struct bat_priv *bat_priv = netdev_priv(soft_iface); + free_percpu(bat_priv->bat_stats_cpu); debugfs_del_meshif(soft_iface); sysfs_del_meshif(soft_iface); mesh_free(soft_iface); @@ -498,3 +513,65 @@ static u32 bat_get_link(struct net_device *dev) { return 1; } + +/* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 */ +static const struct { + const char name[ETH_GSTRING_LEN]; +} bat_stats_strings[] = { + { "forward" }, + { "mgmt_tx" }, + { "mgmt_rx" }, + { "tt_request_tx" }, + { "tt_request_rx" }, + { "tt_response_tx" }, + { "tt_response_rx" }, + { "tt_roam_adv_tx" }, + { "tt_roam_adv_rx" }, +#ifdef CONFIG_BATMAN_ADV_DAT + { "dat_request_tx" }, + { "dat_request_rx" }, + { "dat_reply_tx" }, + { "dat_reply_rx" }, +#endif +}; +#define BAT_STATS_NUM (sizeof(bat_stats_strings)/ETH_GSTRING_LEN) + +static void bat_get_strings(struct net_device *dev, uint32_t stringset, + uint8_t *data) +{ + if (stringset == ETH_SS_STATS) + memcpy(data, bat_stats_strings, sizeof(bat_stats_strings)); +} + +static void bat_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, uint64_t *data) +{ + struct bat_priv *bat_priv = netdev_priv(dev); + int i = 0; + + memset(data, 0, BAT_STATS_NUM*sizeof(*data)); + + sum_counter(bat_priv, forward, data[i++]); + sum_counter(bat_priv, mgmt_tx, data[i++]); + sum_counter(bat_priv, mgmt_rx, data[i++]); + sum_counter(bat_priv, tt_request_tx, data[i++]); + sum_counter(bat_priv, tt_request_rx, data[i++]); + sum_counter(bat_priv, tt_response_tx, data[i++]); + sum_counter(bat_priv, tt_response_rx, data[i++]); + sum_counter(bat_priv, tt_roam_adv_tx, data[i++]); + sum_counter(bat_priv, tt_roam_adv_rx, data[i++]); +#ifdef CONFIG_BATMAN_ADV_DAT + sum_counter(bat_priv, dat_request_tx, data[i++]); + sum_counter(bat_priv, dat_request_rx, data[i++]); + sum_counter(bat_priv, dat_reply_tx, data[i++]); + sum_counter(bat_priv, dat_reply_rx, data[i++]); +#endif +} + +static int bat_get_sset_count(struct net_device *dev, int stringset) +{ + if (stringset == ETH_SS_STATS) + return BAT_STATS_NUM; + + return -EOPNOTSUPP; +} diff --git a/translation-table.c b/translation-table.c index b3e608a..32aa22c 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1352,6 +1352,8 @@ static int send_tt_request(struct bat_priv *bat_priv, dst_orig_node->orig, neigh_node->addr, (full_table ? 'F' : '.'));
+ inc_counter(bat_priv, tt_request_tx); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = 0;
@@ -1476,6 +1478,8 @@ static bool send_other_tt_response(struct bat_priv *bat_priv, res_dst_orig_node->orig, neigh_node->addr, req_dst_orig_node->orig, req_ttvn);
+ inc_counter(bat_priv, tt_response_tx); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = true; goto out; @@ -1592,6 +1596,8 @@ static bool send_my_tt_response(struct bat_priv *bat_priv, orig_node->orig, neigh_node->addr, (tt_response->flags & TT_FULL_TABLE ? 'F' : '.'));
+ inc_counter(bat_priv, tt_response_tx); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = true; goto out; @@ -1891,6 +1897,8 @@ static void send_roam_adv(struct bat_priv *bat_priv, uint8_t *client, "Sending ROAMING_ADV to %pM (client %pM) via %pM\n", orig_node->orig, client, neigh_node->addr);
+ inc_counter(bat_priv, tt_roam_adv_tx); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = 0;
diff --git a/types.h b/types.h index 15f538a..dd87e8b 100644 --- a/types.h +++ b/types.h @@ -162,9 +162,28 @@ struct bcast_duplist_entry { }; #endif
+struct bat_stats { + uint64_t forward; + uint64_t mgmt_tx; + uint64_t mgmt_rx; + uint64_t tt_request_tx; + uint64_t tt_request_rx; + uint64_t tt_response_tx; + uint64_t tt_response_rx; + uint64_t tt_roam_adv_tx; + uint64_t tt_roam_adv_rx; +#ifdef CONFIG_BATMAN_ADV_DAT + uint64_t dat_request_tx; + uint64_t dat_request_rx; + uint64_t dat_reply_tx; + uint64_t dat_reply_rx; +#endif +}; + struct bat_priv { atomic_t mesh_state; struct net_device_stats stats; + struct bat_stats *bat_stats_cpu; /* Per cpu counters */ atomic_t aggregated_ogms; /* boolean */ atomic_t bonding; /* boolean */ atomic_t fragmentation; /* boolean */
Added additional counters in a bat_stats structure, which are exported through the ethtool api. The counters are specific to batman-adv and includes: forwarded packets management packets (OGMs at this point) translation table packets distributed arp table packets
I would like you all to check if the increments are added at the right locations and also if more counters would be relevant. (E.g. in bridge loop avoidance code?)
This is the reworked approach from the previous stat counters patch I send, where ethtool stats was suggested.
v2: Changed to per_cpu variables. Added ifdefs for optional submodules. v3: Change from "struct bat_stats" to uint64_t array.
Signed-off-by: Martin Hundebøll martin@hundeboll.net --- bat_iv_ogm.c | 6 ++++- distributed-arp-table.c | 8 +++++- main.h | 24 +++++++++++++++++ routing.c | 9 +++++++ soft-interface.c | 67 +++++++++++++++++++++++++++++++++++++++++++++-- translation-table.c | 8 ++++++ types.h | 20 ++++++++++++++ 7 files changed, 138 insertions(+), 4 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 994369d..5758a21 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -196,8 +196,10 @@ static void bat_iv_ogm_send_to_if(struct forw_packet *forw_packet,
/* create clone because function is called more than once */ skb = skb_clone(forw_packet->skb, GFP_ATOMIC); - if (skb) + if (skb) { + inc_counter(bat_priv, BAT_CNT_MGMT_TX); send_skb_packet(skb, hard_iface, broadcast_addr); + } }
/* send a batman ogm packet */ @@ -956,6 +958,8 @@ static void bat_iv_ogm_process(const struct ethhdr *ethhdr, if (batman_ogm_packet->header.packet_type != BAT_IV_OGM) return;
+ inc_counter(bat_priv, BAT_CNT_MGMT_RX); + /* could be changed by schedule_own_packet() */ if_incoming_seqno = atomic_read(&if_incoming->seqno);
diff --git a/distributed-arp-table.c b/distributed-arp-table.c index b43bece..a2d3821 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -395,9 +395,11 @@ bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
netif_rx(skb_new); bat_dbg(DBG_DAT, bat_priv, "ARP request replied locally\n"); - } else + } else { /* Send the request on the DHT */ + inc_counter(bat_priv, BAT_CNT_DAT_REQUEST_TX); ret = dht_send_data(bat_priv, skb, ip_dst, BAT_P_DAT_DHT_GET); + } out: if (n) neigh_release(n); @@ -450,6 +452,8 @@ bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv, if (!skb_new) goto out;
+ inc_counter(bat_priv, BAT_CNT_DAT_REPLY_TX); + unicast_4addr_send_skb(skb_new, bat_priv, BAT_P_DAT_CACHE_REPLY);
ret = true; @@ -488,6 +492,8 @@ bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv, arp_neigh_update(bat_priv, ip_src, hw_src); arp_neigh_update(bat_priv, ip_dst, hw_dst);
+ inc_counter(bat_priv, BAT_CNT_DAT_REPLY_TX); + /* Send the ARP reply to the candidates for both the IP addresses we * fetched from the ARP reply */ dht_send_data(bat_priv, skb, ip_src, BAT_P_DAT_DHT_PUT); diff --git a/main.h b/main.h index c8bfe28..bb4baa1 100644 --- a/main.h +++ b/main.h @@ -150,6 +150,7 @@ enum dbg_level { #include <linux/kthread.h> /* kernel threads */ #include <linux/pkt_sched.h> /* schedule types */ #include <linux/workqueue.h> /* workqueue */ +#include <linux/percpu.h> #include <linux/slab.h> #include <net/sock.h> /* struct sock */ #include <linux/jiffies.h> @@ -259,4 +260,27 @@ static inline bool has_timed_out(unsigned long timestamp, unsigned int timeout) _dummy > smallest_signed_int(_dummy); }) #define seq_after(x, y) seq_before(y, x)
+/* Stop preemption on local cpu while incrementing the counter */ +static inline void inc_counter(struct bat_priv *bat_priv, size_t idx) +{ + int cpu = get_cpu(); + per_cpu_ptr(bat_priv->bat_counters, cpu)[idx]++; + put_cpu(); +} + +/* Sum and return the cpu-local counters for index 'idx' */ +static inline uint64_t sum_counter(struct bat_priv *bat_priv, size_t idx) +{ + uint64_t *counters; + int cpu; + int sum = 0; + + for_each_possible_cpu(cpu) { + counters = per_cpu_ptr(bat_priv->bat_counters, cpu); + sum += counters[idx]; + } + + return sum; +} + #endif /* _NET_BATMAN_ADV_MAIN_H_ */ diff --git a/routing.c b/routing.c index 795d3af..c984cc3 100644 --- a/routing.c +++ b/routing.c @@ -600,6 +600,8 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if)
switch (tt_query->flags & TT_QUERY_TYPE_MASK) { case TT_REQUEST: + inc_counter(bat_priv, BAT_CNT_TT_REQUEST_RX); + /* If we cannot provide an answer the tt_request is * forwarded */ if (!send_tt_response(bat_priv, tt_query)) { @@ -612,6 +614,8 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if) } break; case TT_RESPONSE: + inc_counter(bat_priv, BAT_CNT_TT_RESPONSE_RX); + if (is_my_mac(tt_query->dst)) { /* packet needs to be linearized to access the TT * changes */ @@ -663,6 +667,8 @@ int recv_roam_adv(struct sk_buff *skb, struct hard_iface *recv_if) if (is_broadcast_ether_addr(ethhdr->h_source)) goto out;
+ inc_counter(bat_priv, BAT_CNT_TT_ROAM_ADV_RX); + roam_adv_packet = (struct roam_adv_packet *)skb->data;
if (!is_my_mac(roam_adv_packet->dst)) @@ -869,6 +875,9 @@ static int route_unicast_packet(struct sk_buff *skb, struct hard_iface *recv_if) /* decrement ttl */ unicast_packet->header.ttl--;
+ /* Update stats counter */ + inc_counter(bat_priv, BAT_CNT_FORWARD); + /* route it */ send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = NET_RX_SUCCESS; diff --git a/soft-interface.c b/soft-interface.c index 92137af..41147b5 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -46,6 +46,10 @@ static void bat_get_drvinfo(struct net_device *dev, static u32 bat_get_msglevel(struct net_device *dev); static void bat_set_msglevel(struct net_device *dev, u32 value); static u32 bat_get_link(struct net_device *dev); +static void bat_get_strings(struct net_device *dev, u32 stringset, u8 *data); +static void bat_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, u64 *data); +static int bat_get_sset_count(struct net_device *dev, int stringset);
static const struct ethtool_ops bat_ethtool_ops = { .get_settings = bat_get_settings, @@ -53,6 +57,9 @@ static const struct ethtool_ops bat_ethtool_ops = { .get_msglevel = bat_get_msglevel, .set_msglevel = bat_set_msglevel, .get_link = bat_get_link, + .get_strings = bat_get_strings, + .get_ethtool_stats = bat_get_ethtool_stats, + .get_sset_count = bat_get_sset_count, };
int my_skb_head_push(struct sk_buff *skb, unsigned int len) @@ -411,13 +418,18 @@ struct net_device *softif_create(const char *name) bat_priv->primary_if = NULL; bat_priv->num_ifaces = 0;
+ bat_priv->bat_counters = __alloc_percpu(sizeof(uint64_t)*BAT_CNT_NUM, + __alignof__(uint64_t)); + if (!bat_priv->bat_counters) + goto unreg_soft_iface; + ret = bat_algo_select(bat_priv, bat_routing_algo); if (ret < 0) - goto unreg_soft_iface; + goto free_bat_counters;
ret = sysfs_add_meshif(soft_iface); if (ret < 0) - goto unreg_soft_iface; + goto free_bat_counters;
ret = debugfs_add_meshif(soft_iface); if (ret < 0) @@ -433,6 +445,8 @@ unreg_debugfs: debugfs_del_meshif(soft_iface); unreg_sysfs: sysfs_del_meshif(soft_iface); +free_bat_counters: + free_percpu(bat_priv->bat_counters); unreg_soft_iface: unregister_netdevice(soft_iface); return NULL; @@ -445,6 +459,8 @@ out:
void softif_destroy(struct net_device *soft_iface) { + struct bat_priv *bat_priv = netdev_priv(soft_iface); + free_percpu(bat_priv->bat_counters); debugfs_del_meshif(soft_iface); sysfs_del_meshif(soft_iface); mesh_free(soft_iface); @@ -498,3 +514,50 @@ static u32 bat_get_link(struct net_device *dev) { return 1; } + +/* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 */ +static const struct { + const char name[ETH_GSTRING_LEN]; +} bat_counters_strings[] = { + { "forward" }, + { "mgmt_tx" }, + { "mgmt_rx" }, + { "tt_request_tx" }, + { "tt_request_rx" }, + { "tt_response_tx" }, + { "tt_response_rx" }, + { "tt_roam_adv_tx" }, + { "tt_roam_adv_rx" }, +#ifdef CONFIG_BATMAN_ADV_DAT + { "dat_request_tx" }, + { "dat_request_rx" }, + { "dat_reply_tx" }, + { "dat_reply_rx" }, +#endif +}; + +static void bat_get_strings(struct net_device *dev, uint32_t stringset, + uint8_t *data) +{ + if (stringset == ETH_SS_STATS) + memcpy(data, bat_counters_strings, + sizeof(bat_counters_strings)); +} + +static void bat_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, uint64_t *data) +{ + struct bat_priv *bat_priv = netdev_priv(dev); + int i; + + for (i = 0; i < BAT_CNT_NUM; i++) + data[i] = sum_counter(bat_priv, i); +} + +static int bat_get_sset_count(struct net_device *dev, int stringset) +{ + if (stringset == ETH_SS_STATS) + return BAT_CNT_NUM; + + return -EOPNOTSUPP; +} diff --git a/translation-table.c b/translation-table.c index b3e608a..7f5dffe 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1352,6 +1352,8 @@ static int send_tt_request(struct bat_priv *bat_priv, dst_orig_node->orig, neigh_node->addr, (full_table ? 'F' : '.'));
+ inc_counter(bat_priv, BAT_CNT_TT_REQUEST_TX); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = 0;
@@ -1476,6 +1478,8 @@ static bool send_other_tt_response(struct bat_priv *bat_priv, res_dst_orig_node->orig, neigh_node->addr, req_dst_orig_node->orig, req_ttvn);
+ inc_counter(bat_priv, BAT_CNT_TT_RESPONSE_TX); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = true; goto out; @@ -1592,6 +1596,8 @@ static bool send_my_tt_response(struct bat_priv *bat_priv, orig_node->orig, neigh_node->addr, (tt_response->flags & TT_FULL_TABLE ? 'F' : '.'));
+ inc_counter(bat_priv, BAT_CNT_TT_RESPONSE_TX); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = true; goto out; @@ -1891,6 +1897,8 @@ static void send_roam_adv(struct bat_priv *bat_priv, uint8_t *client, "Sending ROAMING_ADV to %pM (client %pM) via %pM\n", orig_node->orig, client, neigh_node->addr);
+ inc_counter(bat_priv, BAT_CNT_TT_ROAM_ADV_TX); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = 0;
diff --git a/types.h b/types.h index 15f538a..55fef02 100644 --- a/types.h +++ b/types.h @@ -162,9 +162,29 @@ struct bcast_duplist_entry { }; #endif
+enum bat_counters { + BAT_CNT_FORWARD, + BAT_CNT_MGMT_TX, + BAT_CNT_MGMT_RX, + BAT_CNT_TT_REQUEST_TX, + BAT_CNT_TT_REQUEST_RX, + BAT_CNT_TT_RESPONSE_TX, + BAT_CNT_TT_RESPONSE_RX, + BAT_CNT_TT_ROAM_ADV_TX, + BAT_CNT_TT_ROAM_ADV_RX, +#ifdef CONFIG_BATMAN_ADV_DAT + BAT_CNT_DAT_REQUEST_TX, + BAT_CNT_DAT_REQUEST_RX, + BAT_CNT_DAT_REPLY_TX, + BAT_CNT_DAT_REPLY_RX, +#endif + BAT_CNT_NUM, +}; + struct bat_priv { atomic_t mesh_state; struct net_device_stats stats; + uint64_t *bat_counters; /* Per cpu counters */ atomic_t aggregated_ogms; /* boolean */ atomic_t bonding; /* boolean */ atomic_t fragmentation; /* boolean */
On Wednesday, April 18, 2012 21:35:57 Martin Hundebøll wrote:
Added additional counters in a bat_stats structure, which are exported through the ethtool api. The counters are specific to batman-adv and includes: forwarded packets management packets (OGMs at this point) translation table packets distributed arp table packets
I would like you all to check if the increments are added at the right locations and also if more counters would be relevant. (E.g. in bridge loop avoidance code?)
This is the reworked approach from the previous stat counters patch I send, where ethtool stats was suggested.
This patch looks pretty good! I'd say we can merge it unless someone objects?!
Would it possible to count the number of bytes for the forwarded packets in addition to the packet count ? As I recall you also wanted to move free_percpu() into mesh_free() ?
Regards, Marek
Added additional counters in a bat_stats structure, which are exported through the ethtool api. The counters are specific to batman-adv and includes: forwarded packets and bytes management packets and bytes (aggregated OGMs at this point) translation table packets distributed arp table packets
New counters are added by extending "enum bat_counters" in types.h and adding corresponding descriptive string(s) to bat_counters_strings in soft-iface.c.
Counters are increased by calling add_counter() and incremented by one by calling inc_counter().
v2: Changed to per_cpu variables. Added ifdefs for optional submodules. v3: Change from "struct bat_stats" to uint64_t array. v4: Added byte counters. Moved free function. Added spaces.
Signed-off-by: Martin Hundebøll martin@hundeboll.net --- README | 5 ++++ bat_iv_ogm.c | 9 +++++- distributed-arp-table.c | 8 +++++- main.c | 2 ++ main.h | 27 ++++++++++++++++++ routing.c | 10 +++++++ soft-interface.c | 71 +++++++++++++++++++++++++++++++++++++++++++++-- translation-table.c | 8 ++++++ types.h | 23 +++++++++++++++ 9 files changed, 159 insertions(+), 4 deletions(-)
diff --git a/README b/README index 82c075f..49c03d5 100644 --- a/README +++ b/README @@ -212,6 +212,11 @@ The debug output can be changed at runtime using the file
will enable debug messages for when routes change.
+Counters for different types of packets entering and leaving the +batman-adv module are available through ethtool: + +# ethtool --statistics bat0 +
BATCTL ------ diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c index 994369d..b222ed9 100644 --- a/bat_iv_ogm.c +++ b/bat_iv_ogm.c @@ -196,8 +196,12 @@ static void bat_iv_ogm_send_to_if(struct forw_packet *forw_packet,
/* create clone because function is called more than once */ skb = skb_clone(forw_packet->skb, GFP_ATOMIC); - if (skb) + if (skb) { + inc_counter(bat_priv, BAT_CNT_MGMT_TX); + add_counter(bat_priv, BAT_CNT_MGMT_TX_BYTES, + skb->len + ETH_HLEN); send_skb_packet(skb, hard_iface, broadcast_addr); + } }
/* send a batman ogm packet */ @@ -1204,6 +1208,9 @@ static int bat_iv_ogm_receive(struct sk_buff *skb, if (bat_priv->bat_algo_ops->bat_ogm_emit != bat_iv_ogm_emit) return NET_RX_DROP;
+ inc_counter(bat_priv, BAT_CNT_MGMT_RX); + add_counter(bat_priv, BAT_CNT_MGMT_RX_BYTES, skb->len + ETH_HLEN); + packet_len = skb_headlen(skb); ethhdr = (struct ethhdr *)skb_mac_header(skb); packet_buff = skb->data; diff --git a/distributed-arp-table.c b/distributed-arp-table.c index b43bece..a2d3821 100644 --- a/distributed-arp-table.c +++ b/distributed-arp-table.c @@ -395,9 +395,11 @@ bool dat_snoop_outgoing_arp_request(struct bat_priv *bat_priv,
netif_rx(skb_new); bat_dbg(DBG_DAT, bat_priv, "ARP request replied locally\n"); - } else + } else { /* Send the request on the DHT */ + inc_counter(bat_priv, BAT_CNT_DAT_REQUEST_TX); ret = dht_send_data(bat_priv, skb, ip_dst, BAT_P_DAT_DHT_GET); + } out: if (n) neigh_release(n); @@ -450,6 +452,8 @@ bool dat_snoop_incoming_arp_request(struct bat_priv *bat_priv, if (!skb_new) goto out;
+ inc_counter(bat_priv, BAT_CNT_DAT_REPLY_TX); + unicast_4addr_send_skb(skb_new, bat_priv, BAT_P_DAT_CACHE_REPLY);
ret = true; @@ -488,6 +492,8 @@ bool dat_snoop_outgoing_arp_reply(struct bat_priv *bat_priv, arp_neigh_update(bat_priv, ip_src, hw_src); arp_neigh_update(bat_priv, ip_dst, hw_dst);
+ inc_counter(bat_priv, BAT_CNT_DAT_REPLY_TX); + /* Send the ARP reply to the candidates for both the IP addresses we * fetched from the ARP reply */ dht_send_data(bat_priv, skb, ip_src, BAT_P_DAT_DHT_PUT); diff --git a/main.c b/main.c index 0757c2d..5a193f5 100644 --- a/main.c +++ b/main.c @@ -153,6 +153,8 @@ void mesh_free(struct net_device *soft_iface)
bla_free(bat_priv);
+ free_percpu(bat_priv->bat_counters); + atomic_set(&bat_priv->mesh_state, MESH_INACTIVE); }
diff --git a/main.h b/main.h index c8bfe28..9dc7971 100644 --- a/main.h +++ b/main.h @@ -150,6 +150,7 @@ enum dbg_level { #include <linux/kthread.h> /* kernel threads */ #include <linux/pkt_sched.h> /* schedule types */ #include <linux/workqueue.h> /* workqueue */ +#include <linux/percpu.h> #include <linux/slab.h> #include <net/sock.h> /* struct sock */ #include <linux/jiffies.h> @@ -259,4 +260,30 @@ static inline bool has_timed_out(unsigned long timestamp, unsigned int timeout) _dummy > smallest_signed_int(_dummy); }) #define seq_after(x, y) seq_before(y, x)
+/* Stop preemption on local cpu while incrementing the counter */ +static inline void add_counter(struct bat_priv *bat_priv, size_t idx, + size_t count) +{ + int cpu = get_cpu(); + per_cpu_ptr(bat_priv->bat_counters, cpu)[idx] += count; + put_cpu(); +} + +#define inc_counter(b, i) add_counter(b, i, 1) + +/* Sum and return the cpu-local counters for index 'idx' */ +static inline uint64_t sum_counter(struct bat_priv *bat_priv, size_t idx) +{ + uint64_t *counters; + int cpu; + int sum = 0; + + for_each_possible_cpu(cpu) { + counters = per_cpu_ptr(bat_priv->bat_counters, cpu); + sum += counters[idx]; + } + + return sum; +} + #endif /* _NET_BATMAN_ADV_MAIN_H_ */ diff --git a/routing.c b/routing.c index 795d3af..de420c6 100644 --- a/routing.c +++ b/routing.c @@ -600,6 +600,8 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if)
switch (tt_query->flags & TT_QUERY_TYPE_MASK) { case TT_REQUEST: + inc_counter(bat_priv, BAT_CNT_TT_REQUEST_RX); + /* If we cannot provide an answer the tt_request is * forwarded */ if (!send_tt_response(bat_priv, tt_query)) { @@ -612,6 +614,8 @@ int recv_tt_query(struct sk_buff *skb, struct hard_iface *recv_if) } break; case TT_RESPONSE: + inc_counter(bat_priv, BAT_CNT_TT_RESPONSE_RX); + if (is_my_mac(tt_query->dst)) { /* packet needs to be linearized to access the TT * changes */ @@ -663,6 +667,8 @@ int recv_roam_adv(struct sk_buff *skb, struct hard_iface *recv_if) if (is_broadcast_ether_addr(ethhdr->h_source)) goto out;
+ inc_counter(bat_priv, BAT_CNT_TT_ROAM_ADV_RX); + roam_adv_packet = (struct roam_adv_packet *)skb->data;
if (!is_my_mac(roam_adv_packet->dst)) @@ -869,6 +875,10 @@ static int route_unicast_packet(struct sk_buff *skb, struct hard_iface *recv_if) /* decrement ttl */ unicast_packet->header.ttl--;
+ /* Update stats counter */ + inc_counter(bat_priv, BAT_CNT_FORWARD); + add_counter(bat_priv, BAT_CNT_FORWARD_BYTES, skb->len + ETH_HLEN); + /* route it */ send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = NET_RX_SUCCESS; diff --git a/soft-interface.c b/soft-interface.c index 92137af..ab13af9 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -46,6 +46,10 @@ static void bat_get_drvinfo(struct net_device *dev, static u32 bat_get_msglevel(struct net_device *dev); static void bat_set_msglevel(struct net_device *dev, u32 value); static u32 bat_get_link(struct net_device *dev); +static void bat_get_strings(struct net_device *dev, u32 stringset, u8 *data); +static void bat_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, u64 *data); +static int bat_get_sset_count(struct net_device *dev, int stringset);
static const struct ethtool_ops bat_ethtool_ops = { .get_settings = bat_get_settings, @@ -53,6 +57,9 @@ static const struct ethtool_ops bat_ethtool_ops = { .get_msglevel = bat_get_msglevel, .set_msglevel = bat_set_msglevel, .get_link = bat_get_link, + .get_strings = bat_get_strings, + .get_ethtool_stats = bat_get_ethtool_stats, + .get_sset_count = bat_get_sset_count, };
int my_skb_head_push(struct sk_buff *skb, unsigned int len) @@ -411,13 +418,18 @@ struct net_device *softif_create(const char *name) bat_priv->primary_if = NULL; bat_priv->num_ifaces = 0;
+ bat_priv->bat_counters = __alloc_percpu(sizeof(uint64_t) * BAT_CNT_NUM, + __alignof__(uint64_t)); + if (!bat_priv->bat_counters) + goto unreg_soft_iface; + ret = bat_algo_select(bat_priv, bat_routing_algo); if (ret < 0) - goto unreg_soft_iface; + goto free_bat_counters;
ret = sysfs_add_meshif(soft_iface); if (ret < 0) - goto unreg_soft_iface; + goto free_bat_counters;
ret = debugfs_add_meshif(soft_iface); if (ret < 0) @@ -433,6 +445,8 @@ unreg_debugfs: debugfs_del_meshif(soft_iface); unreg_sysfs: sysfs_del_meshif(soft_iface); +free_bat_counters: + free_percpu(bat_priv->bat_counters); unreg_soft_iface: unregister_netdevice(soft_iface); return NULL; @@ -498,3 +512,56 @@ static u32 bat_get_link(struct net_device *dev) { return 1; } + +/* Inspired by drivers/net/ethernet/dlink/sundance.c:1702 + * Declare each description string in struct.name[] to get fixed sized buffer + * and compile time checking for strings longer than ETH_GSTRING_LEN. + */ +static const struct { + const char name[ETH_GSTRING_LEN]; +} bat_counters_strings[] = { + { "forward" }, + { "forward_bytes" }, + { "mgmt_tx" }, + { "mgmt_tx_bytes" }, + { "mgmt_rx" }, + { "mgmt_rx_bytes" }, + { "tt_request_tx" }, + { "tt_request_rx" }, + { "tt_response_tx" }, + { "tt_response_rx" }, + { "tt_roam_adv_tx" }, + { "tt_roam_adv_rx" }, +#ifdef CONFIG_BATMAN_ADV_DAT + { "dat_request_tx" }, + { "dat_request_rx" }, + { "dat_reply_tx" }, + { "dat_reply_rx" }, +#endif +}; + +static void bat_get_strings(struct net_device *dev, uint32_t stringset, + uint8_t *data) +{ + if (stringset == ETH_SS_STATS) + memcpy(data, bat_counters_strings, + sizeof(bat_counters_strings)); +} + +static void bat_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, uint64_t *data) +{ + struct bat_priv *bat_priv = netdev_priv(dev); + int i; + + for (i = 0; i < BAT_CNT_NUM; i++) + data[i] = sum_counter(bat_priv, i); +} + +static int bat_get_sset_count(struct net_device *dev, int stringset) +{ + if (stringset == ETH_SS_STATS) + return BAT_CNT_NUM; + + return -EOPNOTSUPP; +} diff --git a/translation-table.c b/translation-table.c index b3e608a..7f5dffe 100644 --- a/translation-table.c +++ b/translation-table.c @@ -1352,6 +1352,8 @@ static int send_tt_request(struct bat_priv *bat_priv, dst_orig_node->orig, neigh_node->addr, (full_table ? 'F' : '.'));
+ inc_counter(bat_priv, BAT_CNT_TT_REQUEST_TX); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = 0;
@@ -1476,6 +1478,8 @@ static bool send_other_tt_response(struct bat_priv *bat_priv, res_dst_orig_node->orig, neigh_node->addr, req_dst_orig_node->orig, req_ttvn);
+ inc_counter(bat_priv, BAT_CNT_TT_RESPONSE_TX); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = true; goto out; @@ -1592,6 +1596,8 @@ static bool send_my_tt_response(struct bat_priv *bat_priv, orig_node->orig, neigh_node->addr, (tt_response->flags & TT_FULL_TABLE ? 'F' : '.'));
+ inc_counter(bat_priv, BAT_CNT_TT_RESPONSE_TX); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = true; goto out; @@ -1891,6 +1897,8 @@ static void send_roam_adv(struct bat_priv *bat_priv, uint8_t *client, "Sending ROAMING_ADV to %pM (client %pM) via %pM\n", orig_node->orig, client, neigh_node->addr);
+ inc_counter(bat_priv, BAT_CNT_TT_ROAM_ADV_TX); + send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr); ret = 0;
diff --git a/types.h b/types.h index 15f538a..57ba44f 100644 --- a/types.h +++ b/types.h @@ -162,9 +162,32 @@ struct bcast_duplist_entry { }; #endif
+enum bat_counters { + BAT_CNT_FORWARD, + BAT_CNT_FORWARD_BYTES, + BAT_CNT_MGMT_TX, + BAT_CNT_MGMT_TX_BYTES, + BAT_CNT_MGMT_RX, + BAT_CNT_MGMT_RX_BYTES, + BAT_CNT_TT_REQUEST_TX, + BAT_CNT_TT_REQUEST_RX, + BAT_CNT_TT_RESPONSE_TX, + BAT_CNT_TT_RESPONSE_RX, + BAT_CNT_TT_ROAM_ADV_TX, + BAT_CNT_TT_ROAM_ADV_RX, +#ifdef CONFIG_BATMAN_ADV_DAT + BAT_CNT_DAT_REQUEST_TX, + BAT_CNT_DAT_REQUEST_RX, + BAT_CNT_DAT_REPLY_TX, + BAT_CNT_DAT_REPLY_RX, +#endif + BAT_CNT_NUM, +}; + struct bat_priv { atomic_t mesh_state; struct net_device_stats stats; + uint64_t *bat_counters; /* Per cpu counters */ atomic_t aggregated_ogms; /* boolean */ atomic_t bonding; /* boolean */ atomic_t fragmentation; /* boolean */
On Friday, April 20, 2012 23:02:45 Martin Hundebøll wrote:
Added additional counters in a bat_stats structure, which are exported through the ethtool api. The counters are specific to batman-adv and includes: forwarded packets and bytes management packets and bytes (aggregated OGMs at this point) translation table packets distributed arp table packets
New counters are added by extending "enum bat_counters" in types.h and adding corresponding descriptive string(s) to bat_counters_strings in soft-iface.c.
Counters are increased by calling add_counter() and incremented by one by calling inc_counter().
Applied in revision 560985e.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Marek Lindner -
Martin Hundebøll