From fd28ce3ae7c11542c4681d45040ac1970b0ba702 Mon Sep 17 00:00:00 2001 From: Joe Stringer Date: Tue, 26 Nov 2013 05:44:18 +0000 Subject: [PATCH 1/1] bridge: Refresh STP statistics separately from status Currently, we refresh STP status (id, state, role) alongside statistics (rx, tx, errors), all within instant_stats_run(). This patch splits statistics out, and refreshes them with the 5 second stats instead. This paves the way to reducing execution of instant_stats_run(). Signed-off-by: Joe Stringer Signed-off-by: Ethan Jackson Acked-by: Ethan Jackson --- ofproto/ofproto-dpif.c | 19 ++++++++++++++++++ ofproto/ofproto-provider.h | 10 ++++++++++ ofproto/ofproto.c | 21 +++++++++++++++++++ ofproto/ofproto.h | 6 ++++++ vswitchd/bridge.c | 41 +++++++++++++++++++++++++++++++------- 5 files changed, 90 insertions(+), 7 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 5e0a7f0aa..ff77903dd 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -2179,6 +2179,24 @@ get_stp_port_status(struct ofport *ofport_, s->state = stp_port_get_state(sp); s->sec_in_state = (time_msec() - ofport->stp_state_entered) / 1000; s->role = stp_port_get_role(sp); + + return 0; +} + +static int +get_stp_port_stats(struct ofport *ofport_, + struct ofproto_port_stp_stats *s) +{ + struct ofport_dpif *ofport = ofport_dpif_cast(ofport_); + struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto); + struct stp_port *sp = ofport->stp_port; + + if (!ofproto->stp || !sp) { + s->enabled = false; + return 0; + } + + s->enabled = true; stp_port_get_counts(sp, &s->tx_count, &s->rx_count, &s->error_count); return 0; @@ -6367,6 +6385,7 @@ const struct ofproto_class ofproto_dpif_class = { get_stp_status, set_stp_port, get_stp_port_status, + get_stp_port_stats, set_queues, bundle_set, bundle_remove, diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 2844e4cf5..54d97f186 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -1530,6 +1530,16 @@ struct ofproto_class { int (*get_stp_port_status)(struct ofport *ofport, struct ofproto_port_stp_status *s); + /* Retrieves spanning tree protocol (STP) port statistics of 'ofport'. + * + * Stores STP state for 'ofport' in 's'. If the 'enabled' member is + * false, the other member values are not meaningful. + * + * EOPNOTSUPP as a return value indicates that this ofproto_class does not + * support STP, as does a null pointer. */ + int (*get_stp_port_stats)(struct ofport *ofport, + struct ofproto_port_stp_stats *s); + /* Registers meta-data associated with the 'n_qdscp' Qualities of Service * 'queues' attached to 'ofport'. This data is not intended to be * sufficient to implement QoS. Instead, providers may use this diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index b282abee4..3a6032853 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -884,6 +884,27 @@ ofproto_port_get_stp_status(struct ofproto *ofproto, ofp_port_t ofp_port, ? ofproto->ofproto_class->get_stp_port_status(ofport, s) : EOPNOTSUPP); } + +/* Retrieves STP port statistics of 'ofp_port' on 'ofproto' and stores it in + * 's'. If the 'enabled' member in 's' is false, then the other members + * are not meaningful. + * + * Returns 0 if successful, otherwise a positive errno value.*/ +int +ofproto_port_get_stp_stats(struct ofproto *ofproto, ofp_port_t ofp_port, + struct ofproto_port_stp_stats *s) +{ + struct ofport *ofport = ofproto_get_port(ofproto, ofp_port); + if (!ofport) { + VLOG_WARN_RL(&rl, "%s: cannot get STP stats on nonexistent " + "port %"PRIu16, ofproto->name, ofp_port); + return ENODEV; + } + + return (ofproto->ofproto_class->get_stp_port_stats + ? ofproto->ofproto_class->get_stp_port_stats(ofport, s) + : EOPNOTSUPP); +} /* Queue DSCP configuration. */ diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 903d1f4f7..d734eab5d 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -115,6 +115,10 @@ struct ofproto_port_stp_status { enum stp_state state; unsigned int sec_in_state; enum stp_role role; +}; + +struct ofproto_port_stp_stats { + bool enabled; /* If false, ignore other members. */ int tx_count; /* Number of BPDUs transmitted. */ int rx_count; /* Number of valid BPDUs received. */ int error_count; /* Number of bad BPDUs received. */ @@ -281,6 +285,8 @@ int ofproto_port_set_stp(struct ofproto *, ofp_port_t ofp_port, const struct ofproto_port_stp_settings *); int ofproto_port_get_stp_status(struct ofproto *, ofp_port_t ofp_port, struct ofproto_port_stp_status *); +int ofproto_port_get_stp_stats(struct ofproto *, ofp_port_t ofp_port, + struct ofproto_port_stp_stats *); int ofproto_port_set_queues(struct ofproto *, ofp_port_t ofp_port, const struct ofproto_port_queue *, size_t n_queues); diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index c2307be1d..2b11c5b18 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2032,8 +2032,6 @@ port_refresh_stp_status(struct port *port) struct ofproto *ofproto = port->bridge->ofproto; struct iface *iface; struct ofproto_port_stp_status status; - char *keys[3]; - int64_t int_values[3]; struct smap smap; if (port_is_synthetic(port)) { @@ -2047,14 +2045,12 @@ port_refresh_stp_status(struct port *port) } iface = CONTAINER_OF(list_front(&port->ifaces), struct iface, port_elem); - if (ofproto_port_get_stp_status(ofproto, iface->ofp_port, &status)) { return; } if (!status.enabled) { ovsrec_port_set_status(port->cfg, NULL); - ovsrec_port_set_statistics(port->cfg, NULL, NULL, 0); return; } @@ -2066,14 +2062,43 @@ port_refresh_stp_status(struct port *port) smap_add(&smap, "stp_role", stp_role_name(status.role)); ovsrec_port_set_status(port->cfg, &smap); smap_destroy(&smap); +} + +static void +port_refresh_stp_stats(struct port *port) +{ + struct ofproto *ofproto = port->bridge->ofproto; + struct iface *iface; + struct ofproto_port_stp_stats stats; + char *keys[3]; + int64_t int_values[3]; + + if (port_is_synthetic(port)) { + return; + } + + /* STP doesn't currently support bonds. */ + if (!list_is_singleton(&port->ifaces)) { + return; + } + + iface = CONTAINER_OF(list_front(&port->ifaces), struct iface, port_elem); + if (ofproto_port_get_stp_stats(ofproto, iface->ofp_port, &stats)) { + return; + } + + if (!stats.enabled) { + ovsrec_port_set_statistics(port->cfg, NULL, NULL, 0); + return; + } /* Set Statistics column. */ keys[0] = "stp_tx_count"; - int_values[0] = status.tx_count; + int_values[0] = stats.tx_count; keys[1] = "stp_rx_count"; - int_values[1] = status.rx_count; + int_values[1] = stats.rx_count; keys[2] = "stp_error_count"; - int_values[2] = status.error_count; + int_values[2] = stats.error_count; ovsrec_port_set_statistics(port->cfg, keys, int_values, ARRAY_SIZE(int_values)); @@ -2499,6 +2524,8 @@ bridge_run(void) iface_refresh_stats(iface); iface_refresh_status(iface); } + + port_refresh_stp_stats(port); } HMAP_FOR_EACH (m, hmap_node, &br->mirrors) { -- 2.43.0