@@ -39,6 +39,7 @@
#include "openvswitch/ofp-util.h"
#include "openvswitch/vlog.h"
#include "lib/random.h"
+#include "unixctl.h"
#include "lib/dhcp.h"
#include "ovn-controller.h"
@@ -161,6 +162,7 @@ VLOG_DEFINE_THIS_MODULE(pinctrl);
static struct ovs_mutex pinctrl_mutex = OVS_MUTEX_INITIALIZER;
static struct seq *pinctrl_handler_seq;
static struct seq *pinctrl_main_seq;
+struct hmap pinctrl_stats;
static void *pinctrl_handler(void *arg);
@@ -488,6 +490,7 @@ pinctrl_init(void)
pinctrl.br_int_name = NULL;
pinctrl_handler_seq = seq_create();
pinctrl_main_seq = seq_create();
+ hmap_init(&pinctrl_stats);
latch_init(&pinctrl.pinctrl_thread_exit);
pinctrl.pinctrl_thread = ovs_thread_create("ovn_pinctrl", pinctrl_handler,
@@ -2668,6 +2671,169 @@ exit:
dp_packet_uninit(pkt_out_ptr);
}
+struct pinctrl_stats_entry {
+ struct hmap_node hmap_node;
+ int64_t count[N_ACTION_OPCODES];
+ int64_t port_key;
+ int64_t dp_key;
+};
+
+static void
+pinctrl_stats_wait(struct ovsdb_idl_txn *ovnsb_idl_txn)
+{
+ if (ovnsb_idl_txn && !hmap_is_empty(&pinctrl_stats)) {
+ poll_immediate_wake();
+ }
+}
+
+static void
+flush_pinctrl_stats(void)
+{
+ struct pinctrl_stats_entry *entry;
+ HMAP_FOR_EACH_POP (entry, hmap_node, &pinctrl_stats) {
+ free(entry);
+ }
+}
+
+static void
+pinctrl_stats_destroy(void)
+{
+ flush_pinctrl_stats();
+ hmap_destroy(&pinctrl_stats);
+}
+
+static struct pinctrl_stats_entry *
+find_or_create_pinctrl_stats_entry(int64_t dp_key, uint32_t port_key)
+{
+ uint32_t hash = hash_2words(dp_key, port_key);
+ struct pinctrl_stats_entry *entry;
+
+ HMAP_FOR_EACH_WITH_HASH (entry, hmap_node, hash, &pinctrl_stats) {
+ if (entry->dp_key == dp_key && entry->port_key == port_key) {
+ return entry;
+ }
+ }
+
+ entry = xzalloc(sizeof *entry);
+ entry->dp_key = dp_key;
+ entry->port_key = port_key;
+ hmap_insert(&pinctrl_stats, &entry->hmap_node, hash);
+
+ return entry;
+}
+
+static void
+pinctrl_log_stats(uint32_t opcode, const struct flow *md)
+{
+ int64_t dp_key = ntohll(md->metadata);
+ uint32_t port_key = md->regs[MFF_LOG_INPORT - MFF_REG0];
+
+ struct pinctrl_stats_entry *entry =
+ find_or_create_pinctrl_stats_entry(dp_key, port_key);
+ ovs_assert(entry != NULL);
+ ++entry->count[opcode];
+
+ notify_pinctrl_main();
+}
+
+static void
+get_map_from_pb_stats(const struct sbrec_port_binding_statistics *stats,
+ struct simap *map)
+{
+ for (size_t i = 0; i < stats->n_pinctrl_ops; i++) {
+ simap_put(map, stats->key_pinctrl_ops[i],
+ stats->value_pinctrl_ops[i]);
+ }
+}
+
+static const struct sbrec_port_binding_statistics *
+get_pb_chassis_stats(struct ovsdb_idl_txn *ovnsb_idl_txn,
+ const struct sbrec_port_binding *pb,
+ const struct sbrec_chassis *chassis)
+{
+ const struct sbrec_port_binding_statistics *stats;
+
+ for (size_t i = 0; i < pb->n_statistics; i++) {
+ stats = pb->statistics[i];
+ if (!strcmp(stats->chassis_name, chassis->name)) {
+ return stats;
+ }
+ }
+
+ /* Stats not found for this chassis. Add it in now */
+ stats = sbrec_port_binding_statistics_insert(ovnsb_idl_txn);
+ sbrec_port_binding_statistics_set_chassis_name(stats, chassis->name);
+
+ struct sbrec_port_binding_statistics **new_stats =
+ xmalloc(sizeof *new_stats * (pb->n_statistics + 1));
+ nullable_memcpy(new_stats, pb->statistics,
+ sizeof *new_stats * pb->n_statistics);
+ new_stats[pb->n_statistics] =
+ CONST_CAST(struct sbrec_port_binding_statistics *, stats);
+
+ sbrec_port_binding_set_statistics(pb, new_stats, pb->n_statistics + 1);
+ free(new_stats);
+
+ return stats;
+}
+
+static void
+run_pinctrl_stats(struct ovsdb_idl_txn *ovnsb_idl_txn,
+ struct ovsdb_idl_index *sbrec_port_binding_by_key,
+ struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
+ const struct sbrec_chassis *chassis)
+{
+ if (!ovnsb_idl_txn) {
+ return;
+ }
+
+ struct pinctrl_stats_entry *entry;
+ HMAP_FOR_EACH_POP(entry, hmap_node, &pinctrl_stats) {
+ const struct sbrec_port_binding *pb =
+ lport_lookup_by_key(sbrec_datapath_binding_by_key,
+ sbrec_port_binding_by_key,
+ entry->dp_key, entry->port_key);
+
+ if (!pb) {
+ free(entry);
+ continue;
+ }
+
+ const struct sbrec_port_binding_statistics *pb_chassis_stats =
+ get_pb_chassis_stats(ovnsb_idl_txn, pb, chassis);
+
+ struct simap pb_stats_map = SIMAP_INITIALIZER(&pb_stats_map);
+ get_map_from_pb_stats(pb_chassis_stats, &pb_stats_map);
+
+ size_t i;
+ for (i = 0; i < N_ACTION_OPCODES; i++) {
+ simap_increase(&pb_stats_map, get_action_opcode_name(i),
+ entry->count[i]);
+ }
+
+ size_t num_stats = simap_count(&pb_stats_map);
+ const char **ops = xmalloc(num_stats * sizeof *ops);
+ int64_t *counts = xmalloc(num_stats * sizeof *counts);
+
+ struct simap_node *node;
+ i = 0;
+ SIMAP_FOR_EACH (node, &pb_stats_map) {
+ ops[i] = node->name;
+ counts[i] = node->data;
+ i++;
+ }
+
+ sbrec_port_binding_statistics_set_pinctrl_ops(pb_chassis_stats, ops,
+ counts,
+ num_stats);
+
+ simap_destroy(&pb_stats_map);
+ free(ops);
+ free(counts);
+ free(entry);
+ }
+}
+
/* Called with in the pinctrl_handler thread context. */
static void
process_packet_in(struct rconn *swconn, const struct ofp_header *msg)
@@ -2818,6 +2984,10 @@ process_packet_in(struct rconn *swconn, const struct ofp_header *msg)
ntohl(ah->opcode));
break;
}
+
+ ovs_mutex_lock(&pinctrl_mutex);
+ pinctrl_log_stats(ntohl(ah->opcode), &pin.flow_metadata.flow);
+ ovs_mutex_unlock(&pinctrl_mutex);
}
/* Called with in the pinctrl_handler thread context. */
@@ -3020,6 +3190,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
local_datapaths);
sync_svc_monitors(ovnsb_idl_txn, svc_mon_table, sbrec_port_binding_by_name,
chassis);
+ run_pinctrl_stats(ovnsb_idl_txn, sbrec_port_binding_by_key,
+ sbrec_datapath_binding_by_key, chassis);
ovs_mutex_unlock(&pinctrl_mutex);
}
@@ -3543,6 +3715,7 @@ pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn)
wait_put_vport_bindings(ovnsb_idl_txn);
int64_t new_seq = seq_read(pinctrl_main_seq);
seq_wait(pinctrl_main_seq, new_seq);
+ pinctrl_stats_wait(ovnsb_idl_txn);
}
/* Called by ovn-controller. */
@@ -3565,6 +3738,7 @@ pinctrl_destroy(void)
destroy_svc_monitors();
seq_destroy(pinctrl_main_seq);
seq_destroy(pinctrl_handler_seq);
+ pinctrl_stats_destroy();
}
/* Implementation of the "put_arp" and "put_nd" OVN actions. These
@@ -50,4 +50,8 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn);
void pinctrl_destroy(void);
+struct pinctrl_stats_info {
+ struct ovsdb_idl *ovnsb_idl;
+};
+
#endif /* controller/pinctrl.h */
@@ -455,6 +455,42 @@ void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
OVNACTS
#undef OVNACT
+#define ACTION_OPCODES \
+ ACTION_OPCODE(ARP, "arp") \
+ ACTION_OPCODE(PUT_ARP, "put_arp") \
+ ACTION_OPCODE(PUT_DHCP_OPTS, "put_dhcp_opts") \
+ ACTION_OPCODE(ND_NA, "nd_na") \
+ ACTION_OPCODE(PUT_ND, "put_nd") \
+ ACTION_OPCODE(PUT_DHCPV6_OPTS, "put_dhcpv6_opts") \
+ ACTION_OPCODE(DNS_LOOKUP, "dns_lookup") \
+ ACTION_OPCODE(LOG, "log") \
+ ACTION_OPCODE(PUT_ND_RA_OPTS, "put_nd_ra_opts") \
+ ACTION_OPCODE(ND_NS, "nd_ns") \
+ ACTION_OPCODE(ICMP, "icmp") \
+ ACTION_OPCODE(TCP_RESET, "tcp_reset") \
+ ACTION_OPCODE(ND_NA_ROUTER, "nd_na_router") \
+ ACTION_OPCODE(PUT_ICMP4_FRAG_MTU, "put_icmp4_frag_mtu") \
+ ACTION_OPCODE(ICMP4_ERROR, "icmp4_error") \
+ ACTION_OPCODE(EVENT, "trigger_event") \
+ ACTION_OPCODE(IGMP, "igmp") \
+ ACTION_OPCODE(BIND_VPORT, "bind_vport") \
+ ACTION_OPCODE(HANDLE_SVC_CHECK, "handle_svc_check") \
+ ACTION_OPCODE(DHCP6_SERVER, "handle_dcpv6_reply") \
+ ACTION_OPCODE(ICMP6_ERROR, "icmp6_error") \
+ ACTION_OPCODE(PUT_ICMP6_FRAG_MTU, "put_icmp6_frag_mtu")
+
+enum OVS_PACKED_ENUM action_opcode {
+#define ACTION_OPCODE(ENUM, NAME) ACTION_OPCODE_##ENUM,
+ ACTION_OPCODES
+#undef ACTION_OPCODE
+ N_ACTION_OPCODES,
+};
+
+const char *get_action_opcode_name(enum action_opcode opcode);
+
+#if 0
+Old enum definition of opcodes. Serves as good documentation for
+actions and their arguments.
enum action_opcode {
/* "arp { ...actions... }".
*
@@ -606,6 +642,7 @@ enum action_opcode {
* action header. */
ACTION_OPCODE_PUT_ICMP6_FRAG_MTU,
};
+#endif
/* Header. */
struct action_header {
@@ -55,6 +55,18 @@ OVNACTS
/* Helpers. */
+const char *action_opcode_names [] = {
+#define ACTION_OPCODE(ENUM, NAME) NAME,
+ ACTION_OPCODES
+#undef ACTION_OPCODE
+};
+
+const char *
+get_action_opcode_name(enum action_opcode opcode)
+{
+ return action_opcode_names[opcode];
+}
+
/* Implementation of ovnact_put_<ENUM>(). */
void *
ovnact_put(struct ofpbuf *ovnacts, enum ovnact_type type, size_t len)
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
"version": "2.9.0",
- "cksum": "223619766 22548",
+ "cksum": "819260930 23105",
"tables": {
"SB_Global": {
"columns": {
@@ -166,6 +166,11 @@
"refType": "strong"},
"min": 0,
"max": "unlimited"}},
+ "statistics": {
+ "type": {"key": {"type": "uuid",
+ "refTable": "Port_Binding_Statistics"},
+ "min": 0,
+ "max": "unlimited"}},
"ha_chassis_group": {
"type": {"key": {"type": "uuid",
"refTable": "HA_Chassis_Group",
@@ -446,6 +451,13 @@
"type": {"key": "string", "value": "string",
"min": 0, "max": "unlimited"}}},
"indexes": [["logical_port", "ip", "port", "protocol"]],
- "isRoot": true}
+ "isRoot": true},
+ "Port_Binding_Statistics": {
+ "columns": {
+ "chassis_name": {"type": "string"},
+ "pinctrl_ops": {
+ "type": {"key": "string", "value": "integer",
+ "min": 0, "max": "unlimited"}}},
+ "isRoot": false}
}
}
@@ -2933,6 +2933,12 @@ tcp.flags = RST;
VLAN on the physical network. The VLAN ID is used to match
incoming traffic and is also added to outgoing traffic.
</column>
+
+ <column name="statistics">
+ An array of <ref table="Port_Binding_Statistics" /> references. A
+ separate row is created for each chassis on which this port binding
+ logs packets in.
+ </column>
</group>
<group title="VTEP Options">
@@ -4083,4 +4089,20 @@ tcp.flags = RST;
</column>
</group>
</table>
+ <table name="Port_Binding_Statistics">
+ <p>
+ This table stores statistics about port bindings.
+ </p>
+
+ <column name="chassis_name">
+ The chassis on which the statistics were recorded.
+ </column>
+
+ <column name="pinctrl_ops">
+ An array storing the number of times a particular packet in operation
+ has occurred on this port binding. The array indices correspond to
+ internal numeric representations of the operations.
+ </column>
+
+ </table>
</database>
@@ -54,6 +54,7 @@
#include "timeval.h"
#include "util.h"
#include "svec.h"
+#include "ovn/actions.h"
VLOG_DEFINE_THIS_MODULE(sbctl);
@@ -1387,6 +1388,95 @@ cmd_set_ssl(struct ctl_context *ctx)
sbrec_sb_global_set_ssl(sb_global, ssl);
}
+static void
+pre_cmd_print_port_statistics(struct ctl_context *ctx)
+{
+ ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_statistics);
+ ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_datapath);
+ ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_logical_port);
+ ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_chassis);
+
+ ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_name);
+
+ ovsdb_idl_add_column(ctx->idl, &sbrec_datapath_binding_col_external_ids);
+
+ ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_statistics_col_chassis_name);
+ ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_statistics_col_pinctrl_ops);
+}
+
+static void
+cmd_print_port_statistics(struct ctl_context *ctx)
+{
+ struct ovsdb_idl *ovnsb_idl = ctx->idl;
+
+ const struct sbrec_port_binding *pb;
+ struct shash dps = SHASH_INITIALIZER(&dps);
+
+ struct pb_list_node {
+ const struct sbrec_port_binding *pb;
+ struct ovs_list list;
+ };
+
+ /* Organize stats by datapath */
+ SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) {
+ if (!pb->n_statistics || !pb->datapath) {
+ continue;
+ }
+ const char *dp_name = smap_get_def(&pb->datapath->external_ids, "name",
+ "(none)");
+ struct ovs_list *pb_list;
+ pb_list = shash_find_data(&dps, dp_name);
+ if (!pb_list) {
+ pb_list = xmalloc(sizeof *pb_list);
+ ovs_list_init(pb_list);
+ shash_add(&dps, dp_name, pb_list);
+ }
+ struct pb_list_node *node = xzalloc(sizeof *node);
+ node->pb = pb;
+ ovs_list_push_back(pb_list, &node->list);
+ }
+
+ struct shash_node *node;
+ SHASH_FOR_EACH (node, &dps) {
+ const char *dp_name = node->name;
+ struct ovs_list *pb_list = node->data;
+ ds_put_format(&ctx->output, "%s\n", dp_name);
+ struct pb_list_node *pb_node;
+ LIST_FOR_EACH_POP (pb_node, list, pb_list) {
+ pb = pb_node->pb;
+ ds_put_format(&ctx->output, " %s\n", pb->logical_port);
+ for (int i = 0; i < pb->n_statistics; i++) {
+ const struct sbrec_port_binding_statistics *stats;
+ stats = pb->statistics[i];
+ ds_put_format(&ctx->output, " %s\n", stats->chassis_name);
+ for (int j = 0; j < stats->n_pinctrl_ops; j++) {
+ ds_put_format(&ctx->output, " %s: %" PRId64 "\n",
+ stats->key_pinctrl_ops[j],
+ stats->value_pinctrl_ops[j]);
+ }
+ }
+ free(pb_node);
+ }
+ }
+
+ shash_destroy_free_data(&dps);
+}
+static void
+pre_cmd_clear_port_statistics(struct ctl_context *ctx)
+{
+ ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_statistics);
+}
+
+static void
+cmd_clear_port_statistics(struct ctl_context *ctx)
+{
+ struct ovsdb_idl *ovnsb_idl = ctx->idl;
+ const struct sbrec_port_binding *pb;
+
+ SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) {
+ sbrec_port_binding_set_statistics(pb, NULL, 0);
+ }
+}
static const struct ctl_table_class tables[SBREC_N_TABLES] = {
[SBREC_TABLE_CHASSIS].row_ids[0] = {&sbrec_chassis_col_name, NULL, NULL},
@@ -1696,6 +1786,8 @@ static const struct ctl_command_syntax sbctl_commands[] = {
{"set-ssl", 3, 5,
"PRIVATE-KEY CERTIFICATE CA-CERT [SSL-PROTOS [SSL-CIPHERS]]",
pre_cmd_set_ssl, cmd_set_ssl, NULL, "--bootstrap", RW},
+ {"print-port-statistics", 0, 0, "", pre_cmd_print_port_statistics, cmd_print_port_statistics, NULL, "", RO},
+ {"clear-port-statistics", 0, 0, "", pre_cmd_clear_port_statistics, cmd_clear_port_statistics, NULL, "", RW},
{NULL, 0, 0, NULL, NULL, NULL, NULL, NULL, RO},
};
This introduces centralized statistics gathering for Port Bindings. In this initial patch, the only statistics collected are counts of types of actions handled by pinctrl. Port_Bindings now have a list of Port_Binding_Statistics added to them. Each instance of Port_Binding_Statistics on the Port_Binding corresponds to a specific chassis on which the statistic was captured. Two ovn-sbctl commands are added which allow for the statistics to be printed or cleared. The formatting of the output could stand to be improved. This patch is being presented as an RFC: 1) Is this sort of table layout appropriate for the task? 2) Are the operations added to pinctrl.c appropriate? In other words, do you foresee performance-related issues from the added statistics-gathering overhead? The current incarnation triggers an ovn-controller run for every added statistic, but it may be more appropriate to run every X seconds instead. 3) What other pinctrl-related statistics would be appropriate to add in a first patch? Currently, this patch adds only the number of times a certain action opcode was handled. There is no individual packet data collected, no count of successes vs. errors, no timestamps for when the actions were handled, etc. Signed-off-by: Mark Michelson <mmichels@redhat.com> --- controller/pinctrl.c | 174 ++++++++++++++++++++++++++++++++++++++++++ controller/pinctrl.h | 4 + include/ovn/actions.h | 37 +++++++++ lib/actions.c | 12 +++ ovn-sb.ovsschema | 16 +++- ovn-sb.xml | 22 ++++++ utilities/ovn-sbctl.c | 92 ++++++++++++++++++++++ 7 files changed, 355 insertions(+), 2 deletions(-)