From 919f377efefb9c97a5206923f41651da559ad3fa Mon Sep 17 00:00:00 2001 From: Mohsin Kazmi Date: Wed, 15 Nov 2017 14:21:59 +0100 Subject: stats: Fix per interface stats Change-Id: I94618933719abb6ada1272bcf76f4f5304043873 Signed-off-by: Mohsin Kazmi --- src/vpp/stats/stats.c | 241 ++++++++++++++++++++++++-------------------------- src/vpp/stats/stats.h | 24 ----- 2 files changed, 115 insertions(+), 150 deletions(-) diff --git a/src/vpp/stats/stats.c b/src/vpp/stats/stats.c index fe9ff1c5e1a..ef9f61bbf60 100644 --- a/src/vpp/stats/stats.c +++ b/src/vpp/stats/stats.c @@ -687,15 +687,20 @@ static void uword *p; i32 retval = 0; vl_api_registration_t *reg; - int i; - u32 swif; + u32 i, swif, num = 0; + + num = ntohl (mp->num); - // Validate we have good sw_if_indexes before registering - for (i = 0; i < mp->num; i++) + /* + * Validate sw_if_indexes before registering + */ + for (i = 0; i < num; i++) { - swif = mp->sw_ifs[i]; + swif = ntohl (mp->sw_ifs[i]); - /* Check its a real sw_if_index that the client is allowed to see */ + /* + * Check its a real sw_if_index that the client is allowed to see + */ if (swif != ~0) { if (pool_is_free_index (sm->interface_main->sw_interfaces, swif)) @@ -706,23 +711,24 @@ static void } } - for (i = 0; i < mp->num; i++) + for (i = 0; i < num; i++) { - swif = mp->sw_ifs[i]; + swif = ntohl (mp->sw_ifs[i]); rp.client_index = mp->client_index; rp.client_pid = mp->pid; handle_client_registration (&rp, IDX_PER_INTERFACE_COMBINED_COUNTERS, - swif, mp->enable_disable); + swif, ntohl (mp->enable_disable)); } reply: reg = vl_api_client_index_to_registration (mp->client_index); if (!reg) { - for (i = 0; i < mp->num; i++) + for (i = 0; i < num; i++) { - swif = mp->sw_ifs[i]; + swif = ntohl (mp->sw_ifs[i]); + sm->enable_poller = clear_client_for_stat (IDX_PER_INTERFACE_COMBINED_COUNTERS, swif, mp->client_index); @@ -748,53 +754,21 @@ do_combined_per_interface_counters (stats_main_t * sm) vl_shmem_hdr_t *shmem_hdr = am->shmem_hdr; vl_api_registration_t *vl_reg; vlib_combined_counter_main_t *cm; - /* - * items_this_message will eventually be used to optimise the batching - * of per client messages for each stat. For now setting this to 1 then - * iterate. This will not affect API. - * - * FIXME instead of enqueueing here, this should be sent to a batch - * storer for per-client transmission. Each "mp" sent would be a single entry - * and if a client is listening to other sw_if_indexes for same, it would be - * appended to that *mp - */ - u32 items_this_message = 1; - vnet_combined_counter_t *vp = 0; + vl_api_vnet_combined_counter_t *vp = 0; vlib_counter_t v; - int i, j; - u32 timestamp; + u32 i, j; vpe_client_stats_registration_t *reg; vpe_client_registration_t *client; u32 *sw_if_index = 0; - /* - FIXME(s): - - capturing the timestamp of the counters "when VPP knew them" is important. - Less so is that the timing of the delivery to the control plane be in the same - timescale. - - i.e. As long as the control plane can delta messages from VPP and work out - velocity etc based on the timestamp, it can do so in a more "batch mode". - - It would be beneficial to keep a "per-client" message queue, and then - batch all the stat messages for a client into one message, with - discrete timestamps. - - Given this particular API is for "per interface" one assumes that the scale - is less than the ~0 case, which the prior API is suited for. - */ vnet_interface_counter_lock (im); - timestamp = vlib_time_now (sm->vlib_main); - vec_reset_length (sm->regs_tmp); /* *INDENT-OFF* */ pool_foreach (reg, sm->stats_registrations[IDX_PER_INTERFACE_COMBINED_COUNTERS], - ({ - vec_add1 (sm->regs_tmp, reg); - })); + ({ vec_add1 (sm->regs_tmp, reg); })); /* *INDENT-ON* */ for (i = 0; i < vec_len (sm->regs_tmp); i++) @@ -810,13 +784,10 @@ do_combined_per_interface_counters (stats_main_t * sm) vec_reset_length (sm->clients_tmp); /* *INDENT-OFF* */ - pool_foreach (client, reg->clients, ({ - vec_add1 (sm->clients_tmp, client); - })); + pool_foreach (client, reg->clients, ({ vec_add1 (sm->clients_tmp, + client);})); /* *INDENT-ON* */ - //FIXME - should be doing non-variant part of mp here and managing - // any alloc per client in that vec_foreach for (j = 0; j < vec_len (sm->clients_tmp); j++) { client = sm->clients_tmp[j]; @@ -831,35 +802,59 @@ do_combined_per_interface_counters (stats_main_t * sm) reg->item, client->client_index); continue; } + mp = vl_msg_api_alloc_as_if_client (sizeof (*mp) + sizeof (*vp)); + memset (mp, 0, sizeof (*mp)); - mp = vl_msg_api_alloc (sizeof (*mp) + - (items_this_message * - (sizeof (*vp) /* rx */ ))); - - // FIXME when optimising for items_this_message > 1 need to include a - // SIMPLE_INTERFACE_BATCH_SIZE check. mp->_vl_msg_id = ntohs (VL_API_VNET_PER_INTERFACE_COMBINED_COUNTERS); - mp->count = items_this_message; - mp->timestamp = timestamp; - vp = (vnet_combined_counter_t *) mp->data; - + /* + * count will eventually be used to optimise the batching + * of per client messages for each stat. For now setting this to 1 then + * iterate. This will not affect API. + * + * FIXME instead of enqueueing here, this should be sent to a batch + * storer for per-client transmission. Each "mp" sent would be a single entry + * and if a client is listening to other sw_if_indexes for same, it would be + * appended to that *mp + * + * + * FIXME(s): + * - capturing the timestamp of the counters "when VPP knew them" is important. + * Less so is that the timing of the delivery to the control plane be in the same + * timescale. + + * i.e. As long as the control plane can delta messages from VPP and work out + * velocity etc based on the timestamp, it can do so in a more "batch mode". + + * It would be beneficial to keep a "per-client" message queue, and then + * batch all the stat messages for a client into one message, with + * discrete timestamps. + + * Given this particular API is for "per interface" one assumes that the scale + * is less than the ~0 case, which the prior API is suited for. + */ + + /* + * 1 message per api call for now + */ + mp->count = htonl (1); + mp->timestamp = htonl (vlib_time_now (sm->vlib_main)); + + vp = (vl_api_vnet_combined_counter_t *) mp->data; vp->sw_if_index = htonl (reg->item); + im = &vnet_get_main ()->interface_main; cm = im->combined_sw_if_counters + VNET_INTERFACE_COUNTER_RX; vlib_get_combined_counter (cm, reg->item, &v); - clib_mem_unaligned (&vp->rx_packets, u64) - = clib_host_to_net_u64 (v.packets); + clib_mem_unaligned (&vp->rx_packets, u64) = + clib_host_to_net_u64 (v.packets); clib_mem_unaligned (&vp->rx_bytes, u64) = clib_host_to_net_u64 (v.bytes); - - - /* TX vlib_counter_t packets/bytes */ cm = im->combined_sw_if_counters + VNET_INTERFACE_COUNTER_TX; vlib_get_combined_counter (cm, reg->item, &v); - clib_mem_unaligned (&vp->tx_packets, u64) - = clib_host_to_net_u64 (v.packets); + clib_mem_unaligned (&vp->tx_packets, u64) = + clib_host_to_net_u64 (v.packets); clib_mem_unaligned (&vp->tx_bytes, u64) = clib_host_to_net_u64 (v.bytes); @@ -886,12 +881,13 @@ static void uword *p; i32 retval = 0; vl_api_registration_t *reg; - int i; - u32 swif; + u32 i, swif, num = 0; - for (i = 0; i < mp->num; i++) + num = ntohl (mp->num); + + for (i = 0; i < num; i++) { - swif = mp->sw_ifs[i]; + swif = ntohl (mp->sw_ifs[i]); /* Check its a real sw_if_index that the client is allowed to see */ if (swif != ~0) @@ -904,14 +900,14 @@ static void } } - for (i = 0; i < mp->num; i++) + for (i = 0; i < num; i++) { - swif = mp->sw_ifs[i]; + swif = ntohl (mp->sw_ifs[i]); rp.client_index = mp->client_index; rp.client_pid = mp->pid; handle_client_registration (&rp, IDX_PER_INTERFACE_SIMPLE_COUNTERS, - swif, mp->enable_disable); + swif, ntohl (mp->enable_disable)); } reply: @@ -920,9 +916,9 @@ reply: /* Client may have disconnected abruptly, clean up */ if (!reg) { - for (i = 0; i < mp->num; i++) + for (i = 0; i < num; i++) { - swif = mp->sw_ifs[i]; + swif = ntohl (mp->sw_ifs[i]); sm->enable_poller = clear_client_for_stat (IDX_PER_INTERFACE_SIMPLE_COUNTERS, swif, mp->client_index); @@ -950,52 +946,21 @@ do_simple_per_interface_counters (stats_main_t * sm) vl_shmem_hdr_t *shmem_hdr = am->shmem_hdr; vl_api_registration_t *vl_reg; vlib_simple_counter_main_t *cm; - /* - * items_this_message will eventually be used to optimise the batching - * of per client messages for each stat. For now setting this to 1 then - * iterate. This will not affect API. - * - * FIXME instead of enqueueing here, this should be sent to a batch - * storer for per-client transmission. Each "mp" sent would be a single entry - * and if a client is listening to other sw_if_indexes for same, it would be - * appended to that *mp - */ - u32 items_this_message = 1; - int i, j, size; + u32 i, j, size; vpe_client_stats_registration_t *reg; vpe_client_registration_t *client; - u32 timestamp; - u32 count; - vnet_simple_counter_t *vp = 0; + u32 timestamp, count; + vl_api_vnet_simple_counter_t *vp = 0; counter_t v; - /* - FIXME(s): - - capturing the timestamp of the counters "when VPP knew them" is important. - Less so is that the timing of the delivery to the control plane be in the same - timescale. - - i.e. As long as the control plane can delta messages from VPP and work out - velocity etc based on the timestamp, it can do so in a more "batch mode". - - It would be beneficial to keep a "per-client" message queue, and then - batch all the stat messages for a client into one message, with - discrete timestamps. - - Given this particular API is for "per interface" one assumes that the scale - is less than the ~0 case, which the prior API is suited for. - */ vnet_interface_counter_lock (im); - timestamp = vlib_time_now (sm->vlib_main); - vec_reset_length (sm->regs_tmp); /* *INDENT-OFF* */ pool_foreach (reg, - sm->stats_registrations[IDX_PER_INTERFACE_SIMPLE_COUNTERS], ({ - vec_add1 (sm->regs_tmp, reg); - })); + sm->stats_registrations[IDX_PER_INTERFACE_SIMPLE_COUNTERS], + ({ vec_add1 (sm->regs_tmp, reg); })); /* *INDENT-ON* */ for (i = 0; i < vec_len (sm->regs_tmp); i++) @@ -1011,13 +976,10 @@ do_simple_per_interface_counters (stats_main_t * sm) vec_reset_length (sm->clients_tmp); /* *INDENT-OFF* */ - pool_foreach (client, reg->clients, ({ - vec_add1 (sm->clients_tmp, client); - })); + pool_foreach (client, reg->clients, ({ vec_add1 (sm->clients_tmp, + client);})); /* *INDENT-ON* */ - //FIXME - should be doing non-variant part of mp here and managing - // any alloc per client in that vec_foreach for (j = 0; j < vec_len (sm->clients_tmp); j++) { client = sm->clients_tmp[j]; @@ -1032,19 +994,46 @@ do_simple_per_interface_counters (stats_main_t * sm) continue; } - size = (sizeof (*mp) + (items_this_message * (sizeof (u64) * 10))); - mp = vl_msg_api_alloc (size); - // FIXME when optimising for items_this_message > 1 need to include a - // SIMPLE_INTERFACE_BATCH_SIZE check. + mp = vl_msg_api_alloc_as_if_client (sizeof (*mp) + sizeof (*vp)); + memset (mp, 0, sizeof (*mp)); mp->_vl_msg_id = ntohs (VL_API_VNET_PER_INTERFACE_SIMPLE_COUNTERS); - mp->count = items_this_message; - mp->timestamp = timestamp; - vp = (vnet_simple_counter_t *) mp->data; + /* + * count will eventually be used to optimise the batching + * of per client messages for each stat. For now setting this to 1 then + * iterate. This will not affect API. + * + * FIXME instead of enqueueing here, this should be sent to a batch + * storer for per-client transmission. Each "mp" sent would be a single entry + * and if a client is listening to other sw_if_indexes for same, it would be + * appended to that *mp + * + * + * FIXME(s): + * - capturing the timestamp of the counters "when VPP knew them" is important. + * Less so is that the timing of the delivery to the control plane be in the same + * timescale. + + * i.e. As long as the control plane can delta messages from VPP and work out + * velocity etc based on the timestamp, it can do so in a more "batch mode". + + * It would be beneficial to keep a "per-client" message queue, and then + * batch all the stat messages for a client into one message, with + * discrete timestamps. + + * Given this particular API is for "per interface" one assumes that the scale + * is less than the ~0 case, which the prior API is suited for. + */ + + /* + * 1 message per api call for now + */ + mp->count = htonl (1); + mp->timestamp = htonl (vlib_time_now (sm->vlib_main)); + vp = (vl_api_vnet_simple_counter_t *) mp->data; vp->sw_if_index = htonl (reg->item); - //FIXME will be simpler with a preprocessor macro // VNET_INTERFACE_COUNTER_DROP cm = im->sw_if_counters + VNET_INTERFACE_COUNTER_DROP; v = vlib_get_simple_counter (cm, reg->item); diff --git a/src/vpp/stats/stats.h b/src/vpp/stats/stats.h index dd679a65db5..629ac987151 100644 --- a/src/vpp/stats/stats.h +++ b/src/vpp/stats/stats.h @@ -51,30 +51,6 @@ typedef CLIB_PACKED (struct }) ip4_route_t; /* *INDENT-ON* */ -/* see interface.api */ -typedef struct -{ - u32 sw_if_index; - u64 drop; - u64 punt; - u64 rx_ip4; - u64 rx_ip6; - u64 rx_no_buffer; - u64 rx_miss; - u64 rx_error; - u64 tx_error; - u64 rx_mpls; -} vnet_simple_counter_t; - -typedef struct -{ - u32 sw_if_index; - u64 rx_packets; /**< packet counter */ - u64 rx_bytes; /**< byte counter */ - u64 tx_packets; /**< packet counter */ - u64 tx_bytes; /**< byte counter */ -} vnet_combined_counter_t; - typedef struct { ip6_address_t address; -- cgit 1.2.3-korg