From 144216a3358fc3ddc8a8253dd85978f112cb7bbc Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Fri, 19 Aug 2011 13:58:56 -0700 Subject: [PATCH] cfm: Remove cfm_remote_mpid configuration. According to the 802.1ag specification, users should be able to configure the CFM module with a list of remote endpoints with which the local endpoint should have connectivity. Commit 93b8df3853 "cfm: Remove Maintenance_Point and Monitor tables." changed the behavior so that only one remote endpoint could be specified. This commit takes it further, by disallowing specification of any remote endpoints. Due to this change, the semantics of the fault flag are slightly different. Before, a fault was triggered if any of the configured remote endpoints were unreachable (or with RDI), or if any unconfigured remote endpoints were reachable. Now a fault is triggered if no remote endpoints are reachable at all, or if reachable endpoints have set their RDI. Bug #7014. --- lib/cfm.c | 111 +++++++++++++++---------------------- lib/cfm.h | 3 - vswitchd/bridge.c | 7 +-- vswitchd/vswitch.ovsschema | 9 +-- vswitchd/vswitch.xml | 35 ++++++++---- 5 files changed, 72 insertions(+), 93 deletions(-) diff --git a/lib/cfm.c b/lib/cfm.c index 2a79d633..6494f09c 100644 --- a/lib/cfm.c +++ b/lib/cfm.c @@ -36,6 +36,8 @@ VLOG_DEFINE_THIS_MODULE(cfm); +#define CFM_MAX_RMPS 256 + /* Ethernet destination address of CCM packets. */ static const uint8_t eth_addr_ccm[6] = { 0x01, 0x80, 0xC2, 0x00, 0x00, 0x30 }; @@ -66,7 +68,6 @@ struct cfm { uint16_t mpid; bool fault; /* Indicates connectivity fault. */ - bool recv_fault; /* Indicates an inability to receive CCMs. */ bool unexpected_recv; /* Received an unexpected CCM. */ uint32_t seq; /* The sequence number of our last CCM. */ @@ -77,7 +78,7 @@ struct cfm { struct timer tx_timer; /* Send CCM when expired. */ struct timer fault_timer; /* Check for faults when expired. */ - struct hmap remote_mps; /* Expected remote MPs. */ + struct hmap remote_mps; /* Remote MPs. */ }; /* Remote MPs represent foreign network entities that are configured to have @@ -87,7 +88,6 @@ struct remote_mp { struct hmap_node node; /* Node in 'remote_mps' map. */ bool recv; /* CCM was received since last fault check. */ - bool fault; /* Indicates a connectivity fault. */ bool rdi; /* Remote Defect Indicator. Indicates remote_mp isn't receiving CCMs that it's expecting to. */ }; @@ -183,11 +183,11 @@ cfm_is_valid_mpid(uint32_t mpid) } static struct remote_mp * -lookup_remote_mp(const struct hmap *hmap, uint16_t mpid) +lookup_remote_mp(const struct cfm *cfm, uint16_t mpid) { struct remote_mp *rmp; - HMAP_FOR_EACH_IN_BUCKET (rmp, node, hash_mpid(mpid), hmap) { + HMAP_FOR_EACH_IN_BUCKET (rmp, node, hash_mpid(mpid), &cfm->remote_mps) { if (rmp->mpid == mpid) { return rmp; } @@ -243,32 +243,37 @@ cfm_run(struct cfm *cfm) { if (timer_expired(&cfm->fault_timer)) { long long int interval = cfm_fault_interval(cfm); - struct remote_mp *rmp; + struct remote_mp *rmp, *rmp_next; cfm->fault = cfm->unexpected_recv; - cfm->recv_fault = false; cfm->unexpected_recv = false; - HMAP_FOR_EACH (rmp, node, &cfm->remote_mps) { - rmp->fault = !rmp->recv; - rmp->recv = false; + HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &cfm->remote_mps) { - if (rmp->fault) { - cfm->recv_fault = true; + if (!rmp->recv) { VLOG_DBG("%s: No CCM from RMP %"PRIu16" in the last %lldms", cfm->name, rmp->mpid, interval); - } else if (rmp->rdi) { - cfm->fault = true; - VLOG_DBG("%s: RDI bit flagged from RMP %"PRIu16, cfm->name, - rmp->mpid); + hmap_remove(&cfm->remote_mps, &rmp->node); + free(rmp); + } else { + rmp->recv = false; + + if (rmp->mpid == cfm->mpid) { + VLOG_WARN_RL(&rl, "%s: Received CCM with local MPID (%d)", + cfm->name, rmp->mpid); + cfm->fault = true; + } + + if (rmp->rdi) { + VLOG_DBG("%s: RDI bit flagged from RMP %"PRIu16, cfm->name, + rmp->mpid); + cfm->fault = true; + } } } - if (cfm->recv_fault) { + if (hmap_is_empty(&cfm->remote_mps)) { cfm->fault = true; - } else { - VLOG_DBG("%s: All RMPs received CCMs in the last %lldms", - cfm->name, interval); } timer_set_duration(&cfm->fault_timer, interval); @@ -304,7 +309,7 @@ cfm_compose_ccm(struct cfm *cfm, struct ofpbuf *packet, memcpy(ccm->maid, cfm->maid, sizeof ccm->maid); memset(ccm->zero, 0, sizeof ccm->zero); - if (cfm->recv_fault) { + if (hmap_is_empty(&cfm->remote_mps)) { ccm->flags |= CCM_RDI_MASK; } } @@ -321,13 +326,9 @@ cfm_wait(struct cfm *cfm) bool cfm_configure(struct cfm *cfm, const struct cfm_settings *s) { - size_t i; uint8_t interval; - struct hmap new_rmps; - struct remote_mp *rmp, *rmp_next; - if (!cfm_is_valid_mpid(s->mpid) || s->interval <= 0 - || s->n_remote_mpids <= 0) { + if (!cfm_is_valid_mpid(s->mpid) || s->interval <= 0) { return false; } @@ -342,32 +343,6 @@ cfm_configure(struct cfm *cfm, const struct cfm_settings *s) timer_set_duration(&cfm->fault_timer, cfm_fault_interval(cfm)); } - hmap_init(&new_rmps); - for (i = 0; i < s->n_remote_mpids; i++) { - uint16_t mpid = s->remote_mpids[i]; - - if (!cfm_is_valid_mpid(mpid) - || lookup_remote_mp(&new_rmps, mpid)) { - continue; - } - - if ((rmp = lookup_remote_mp(&cfm->remote_mps, mpid))) { - hmap_remove(&cfm->remote_mps, &rmp->node); - } else { - rmp = xzalloc(sizeof *rmp); - rmp->mpid = mpid; - } - - hmap_insert(&new_rmps, &rmp->node, hash_mpid(mpid)); - } - - hmap_swap(&new_rmps, &cfm->remote_mps); - HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &new_rmps) { - hmap_remove(&new_rmps, &rmp->node); - free(rmp); - } - hmap_destroy(&new_rmps); - return true; } @@ -425,21 +400,26 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p) ccm_interval = ccm->flags & 0x7; ccm_rdi = ccm->flags & CCM_RDI_MASK; - rmp = lookup_remote_mp(&cfm->remote_mps, ccm_mpid); + if (ccm_interval != cfm->ccm_interval) { + VLOG_WARN_RL(&rl, "%s: received a CCM with an invalid interval" + " (%"PRIu8") from RMP %"PRIu16, cfm->name, + ccm_interval, ccm_mpid); + } + rmp = lookup_remote_mp(cfm, ccm_mpid); if (rmp) { rmp->recv = true; rmp->rdi = ccm_rdi; - - if (ccm_interval != cfm->ccm_interval) { - VLOG_WARN_RL(&rl, "%s: received a CCM with an invalid interval" - " (%"PRIu8") from RMP %"PRIu16, cfm->name, - ccm_interval, rmp->mpid); - } + } else if (hmap_count(&cfm->remote_mps) < CFM_MAX_RMPS) { + rmp = xmalloc(sizeof *rmp); + rmp->mpid = ccm_mpid; + rmp->recv = true; + rmp->rdi = ccm_rdi; + hmap_insert(&cfm->remote_mps, &rmp->node, hash_mpid(rmp->mpid)); } else { cfm->unexpected_recv = true; - VLOG_WARN_RL(&rl, "%s: Received unexpected remote MPID %d from" - " MAC " ETH_ADDR_FMT, cfm->name, ccm_mpid, + VLOG_WARN_RL(&rl, "%s: dropped CCM with MPID %d from MAC " + ETH_ADDR_FMT, cfm->name, ccm_mpid, ETH_ADDR_ARGS(eth->eth_src)); } @@ -484,10 +464,9 @@ cfm_unixctl_show(struct unixctl_conn *conn, return; } - ds_put_format(&ds, "MPID %"PRIu16":%s%s%s\n", cfm->mpid, + ds_put_format(&ds, "MPID %"PRIu16":%s%s\n", cfm->mpid, cfm->fault ? " fault" : "", - cfm->unexpected_recv ? " unexpected_recv" : "", - cfm->recv_fault ? " recv_fault" : ""); + cfm->unexpected_recv ? " unexpected_recv" : ""); ds_put_format(&ds, "\tinterval: %dms\n", cfm->ccm_interval_ms); ds_put_format(&ds, "\tnext CCM tx: %lldms\n", @@ -497,8 +476,8 @@ cfm_unixctl_show(struct unixctl_conn *conn, ds_put_cstr(&ds, "\n"); HMAP_FOR_EACH (rmp, node, &cfm->remote_mps) { - ds_put_format(&ds, "Remote MPID %"PRIu16": %s\n", rmp->mpid, - rmp->fault ? "fault" : ""); + ds_put_format(&ds, "Remote MPID %"PRIu16":%s\n", rmp->mpid, + rmp->rdi ? " rdi" : ""); ds_put_format(&ds, "\trecv since check: %s", rmp->recv ? "true" : "false"); } diff --git a/lib/cfm.h b/lib/cfm.h index 37e158f6..659077b4 100644 --- a/lib/cfm.h +++ b/lib/cfm.h @@ -27,9 +27,6 @@ struct ofpbuf; struct cfm_settings { uint16_t mpid; /* The MPID of this CFM. */ int interval; /* The requested transmission interval. */ - - const uint16_t *remote_mpids; /* Array of remote MPIDs */ - size_t n_remote_mpids; /* Number of MPIDs in 'remote_mpids'. */ }; void cfm_init(void); diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 9783cc96..e903a808 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2607,18 +2607,13 @@ iface_configure_cfm(struct iface *iface) { const struct ovsrec_interface *cfg = iface->cfg; struct cfm_settings s; - uint16_t remote_mpid; - if (!cfg->n_cfm_mpid || !cfg->n_cfm_remote_mpid) { + if (!cfg->n_cfm_mpid) { ofproto_port_clear_cfm(iface->port->bridge->ofproto, iface->ofp_port); return; } s.mpid = *cfg->cfm_mpid; - remote_mpid = *cfg->cfm_remote_mpid; - s.remote_mpids = &remote_mpid; - s.n_remote_mpids = 1; - s.interval = atoi(get_interface_other_config(iface->cfg, "cfm_interval", "0")); if (s.interval <= 0) { diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema index ca61a2cd..43ff95d8 100644 --- a/vswitchd/vswitch.ovsschema +++ b/vswitchd/vswitch.ovsschema @@ -1,6 +1,6 @@ {"name": "Open_vSwitch", - "version": "5.2.0", - "cksum": "434778864 14545", + "version": "6.0.0", + "cksum": "1100213054 14376", "tables": { "Open_vSwitch": { "columns": { @@ -171,11 +171,6 @@ "key": {"type": "integer", "minInteger": 1, "maxInteger": 8191}, "min": 0, "max": 1}}, - "cfm_remote_mpid": { - "type" : { - "key": { "type": "integer", "minInteger": 1, "maxInteger": 8191}, - "min": 0, - "max": 1}}, "cfm_fault": { "type": { "key": { "type": "boolean"}, diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index dbda4efa..b974a3d0 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -1332,6 +1332,15 @@ configurable transmission interval.

+

+ According to the 802.1ag specification, each Maintenance Point should + be configured out-of-band with a list of Remote Maintenance Points it + should have connectivity to. Open vSwitch differs from the + specification in this area. It simply assumes the link is faulted if + no Remote Maintenance Points are reachable, and considers it not + faulted otherwise. +

+ A Maintenance Point ID (MPID) uniquely identifies each endpoint within a Maintenance Association. The MPID is used to identify this endpoint @@ -1340,18 +1349,22 @@ CFM on this . - - The MPID of the remote endpoint being monitored. If this - does not have connectivity to an endpoint - advertising the configured MPID, a fault is signalled. Must be - configured to enable CFM on this - - - Indicates a connectivity fault triggered by an inability to receive - heartbeats from the remote endpoint. When a fault is triggered on - s participating in bonds, they will be - disabled. +

+ Indicates a connectivity fault triggered by an inability to receive + heartbeats from any remote endpoint. When a fault is triggered on + s participating in bonds, they will be + disabled. +

+

+ Faults can be triggered for several reasons. Most importantly they + are triggered when no CCMs are received for a period of 3.5 times the + transmission interval. Faults are also triggered when any CCMs + indicate that a Remote Maintenance Point is not receiving CCMs but + able to send them. Finally, a fault is triggered if a CCM is + received which indicates unexpected configuration. Notably, this + case arises when a CCM is received which advertises the local MPID. +

-- 2.30.2