[ovs-dev] [PATCH 07/11] cfm: Remove cfm_remote_mpid configuration.

Fri Sep 9 02:45:55 UTC 2011

That's very clear.  Thank you.

On Thu, Sep 08, 2011 at 06:10:48PM -0700, Ethan Jackson wrote:
> I added the following paragraph to the documentation.  I'll be
> resending this whole series in a bit.
> 
>         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.
> 
> Ethan
> 
> On Tue, Sep 6, 2011 at 19:32, Ethan Jackson <ethan at nicira.com> wrote:
> > 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 ? ? ? ? ? ? ? ? ?| ? ?5 +-
> > ?vswitchd/bridge.c ? ? ? ? ?| ? ?7 +--
> > ?vswitchd/vswitch.ovsschema | ? ?9 +---
> > ?vswitchd/vswitch.xml ? ? ? | ? ?7 ---
> > ?5 files changed, 48 insertions(+), 91 deletions(-)
> >
> > diff --git a/lib/cfm.c b/lib/cfm.c
> > index fc62079..bee8c61 100644
> > --- a/lib/cfm.c
> > +++ b/lib/cfm.c
> > @@ -66,7 +66,6 @@ struct cfm {
> >
> > ? ? uint16_t mpid;
> > ? ? bool fault; ? ? ? ? ? ?/* Indicates connectivity fault. */
> > - ? ?bool recv_fault; ? ? ? /* Indicates an inability to receive CCMs. */
> > ? ? bool xrecv; ? ? ? ? ? ?/* Received an unexpected CCM. */
> >
> > ? ? uint32_t seq; ? ? ? ? ?/* The sequence number of our last CCM. */
> > @@ -77,7 +76,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 +86,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 +181,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 +241,36 @@ 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->xrecv;
> > - ? ? ? ?cfm->recv_fault = false;
> > ? ? ? ? cfm->xrecv = false;
> > + ? ? ? ?HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &cfm->remote_mps) {
> >
> > - ? ? ? ?HMAP_FOR_EACH (rmp, node, &cfm->remote_mps) {
> > - ? ? ? ? ? ?rmp->fault = !rmp->recv;
> > - ? ? ? ? ? ?rmp->recv = false;
> > -
> > - ? ? ? ? ? ?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 +306,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 +323,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 +340,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 +397,25 @@ 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->xrecv = 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 +460,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->xrecv ? " xrecv" : "",
> > - ? ? ? ? ? ? ? ? ?cfm->recv_fault ? " recv_fault" : "");
> > + ? ? ? ? ? ? ? ? ?cfm->xrecv ? " xrecv" : "");
> >
> > ? ? ds_put_format(&ds, "\tinterval: %dms\n", cfm->ccm_interval_ms);
> > ? ? ds_put_format(&ds, "\tnext CCM tx: %lldms\n",
> > @@ -497,8 +472,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 37e158f..f1ab638 100644
> > --- a/lib/cfm.h
> > +++ b/lib/cfm.h
> > @@ -21,15 +21,14 @@
> > ?#include "hmap.h"
> > ?#include "openvswitch/types.h"
> >
> > +#define CFM_MAX_RMPS 256
> > +
> > ?struct flow;
> > ?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 a348e15..adfc3b6 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -2591,18 +2591,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 ca61a2c..43ff95d 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 c058721..8a74bfa 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -1340,13 +1340,6 @@
> > ? ? ? ? CFM on this <ref table="Interface"/>.
> > ? ? ? </column>
> >
> > - ? ? ?<column name="cfm_remote_mpid">
> > - ? ? ? ?The MPID of the remote endpoint being monitored. ?If this
> > - ? ? ? ?<ref table="Interface"/> does not have connectivity to an endpoint
> > - ? ? ? ?advertising the configured MPID, a fault is signalled. ?Must be
> > - ? ? ? ?configured to enable CFM on this <ref table="Interface"/>
> > - ? ? ?</column>
> > -
> > ? ? ? <column name="cfm_fault">
> > ? ? ? ? Indicates a connectivity fault triggered by an inability to receive
> > ? ? ? ? heartbeats from the remote endpoint. ?When a fault is triggered on
> > --
> > 1.7.6.1
> >
> >
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev