[ovs-dev] [PATCH 10/11] cfm: Six byte MPIDs in extended mode.

Ethan Jackson ethan at nicira.com
Wed Sep 7 02:32:33 UTC 2011


802.1ag only allows for MPIDs in the range [1, 8191].  This is
restrictive enough to make assignment of MPIDs to instances of OVS
awkward.  This patch allows six byte MPIDs when running in extended
mode.

Bug #7014.
---
 lib/cfm.c                  |  111 +++++++++++++++++++++++++++++++------------
 lib/cfm.h                  |    2 +
 vswitchd/bridge.c          |   38 +++++++++++++--
 vswitchd/vswitch.ovsschema |    4 +-
 4 files changed, 118 insertions(+), 37 deletions(-)

diff --git a/lib/cfm.c b/lib/cfm.c
index bdaddf3..829b862 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -36,6 +36,9 @@
 
 VLOG_DEFINE_THIS_MODULE(cfm);
 
+#define MPID_FMT "%"PRIu16"-"ETH_ADDR_FMT
+#define MPID_ARGS(mp) (mp)->mpid, ETH_ADDR_ARGS((mp)->mpidx)
+
 /* Ethernet destination address of CCM packets. */
 static const uint8_t eth_addr_ccm[6] = { 0x01, 0x80, 0xC2, 0x00, 0x00, 0x30 };
 static const uint8_t eth_addr_ccm_x[6] = {
@@ -62,15 +65,21 @@ struct ccm {
 
     /* Defined by ITU-T Y.1731 should be zero */
     ovs_be16 interval_ms_x;      /* Transmission interval in ms. */
-    uint8_t  zero[14];
+    uint8_t  mpidx[6];           /* MPID in extended mode. */
+    uint8_t  zero[8];
 } __attribute__((packed));
 BUILD_ASSERT_DECL(CCM_LEN == sizeof(struct ccm));
 
+struct mpid {
+    uint16_t mpid;
+    uint8_t mpidx[6];
+};
+
 struct cfm {
     char *name;                 /* Name of this CFM object. */
     struct hmap_node hmap_node; /* Node in all_cfms list. */
 
-    uint16_t mpid;
+    struct mpid mpid;      /* MPID, both standard and extended. */
     bool extended;         /* Extended mode. */
     bool fault;            /* Indicates connectivity fault. */
     bool xrecv;            /* Received an unexpected CCM. */
@@ -89,7 +98,7 @@ struct cfm {
 /* Remote MPs represent foreign network entities that are configured to have
  * the same MAID as this CFM instance. */
 struct remote_mp {
-    uint16_t mpid;         /* The Maintenance Point ID of this 'remote_mp'. */
+    struct mpid mpid;      /* The Maintenance Point ID of this 'remote_mp'. */
     struct hmap_node node; /* Node in 'remote_mps' map. */
 
     bool recv;           /* CCM was received since last fault check. */
@@ -174,10 +183,24 @@ ms_to_ccm_interval(int interval_ms)
     return 1;
 }
 
+static bool
+mpid_eq(const struct cfm *cfm, struct mpid *a, struct mpid *b)
+{
+    if (cfm->extended) {
+        return eth_addr_equals(a->mpidx, b->mpidx);
+    } else {
+        return a->mpid == b->mpid;
+    }
+}
+
 static uint32_t
-hash_mpid(uint8_t mpid)
+hash_mpid(const struct cfm *cfm, struct mpid *mpid)
 {
-    return hash_int(mpid, 0);
+    if (cfm->extended) {
+        return hash_bytes(mpid->mpidx, sizeof mpid->mpidx, 0);
+    } else {
+        return hash_int(mpid->mpid, 0);
+    }
 }
 
 static bool
@@ -188,12 +211,14 @@ cfm_is_valid_mpid(uint32_t mpid)
 }
 
 static struct remote_mp *
-lookup_remote_mp(const struct cfm *cfm, uint16_t mpid)
+lookup_remote_mp(const struct cfm *cfm, struct mpid *mpid)
 {
     struct remote_mp *rmp;
+    uint32_t hash;
 
-    HMAP_FOR_EACH_IN_BUCKET (rmp, node, hash_mpid(mpid), &cfm->remote_mps) {
-        if (rmp->mpid == mpid) {
+    hash = hash_mpid(cfm, mpid);
+    HMAP_FOR_EACH_WITH_HASH (rmp, node, hash, &cfm->remote_mps) {
+        if (mpid_eq(cfm, &rmp->mpid, mpid)) {
             return rmp;
         }
     }
@@ -255,22 +280,22 @@ cfm_run(struct cfm *cfm)
         HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &cfm->remote_mps) {
 
             if (!rmp->recv) {
-                VLOG_DBG("%s: No CCM from RMP %"PRIu16" in the last %lldms",
-                         cfm->name, rmp->mpid, interval);
+                VLOG_DBG("%s: no CCM from RMP "MPID_FMT" in the last %lldms",
+                         cfm->name, MPID_ARGS(&rmp->mpid), interval);
                 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);
+                if (mpid_eq(cfm, &rmp->mpid, &cfm->mpid)) {
+                    VLOG_WARN_RL(&rl,"%s: received CCM with local MPID "
+                                 MPID_FMT, cfm->name, MPID_ARGS(&rmp->mpid));
                     cfm->fault = true;
                 }
 
                 if (rmp->rdi) {
-                    VLOG_DBG("%s: RDI bit flagged from RMP %"PRIu16, cfm->name,
-                             rmp->mpid);
+                    VLOG_DBG("%s: RDI bit flagged from RMP "MPID_FMT,
+                             cfm->name, MPID_ARGS(&rmp->mpid));
                     cfm->fault = true;
                 }
             }
@@ -307,11 +332,17 @@ cfm_compose_ccm(struct cfm *cfm, struct ofpbuf *packet,
     ccm->opcode = CCM_OPCODE;
     ccm->tlv_offset = 70;
     ccm->seq = htonl(++cfm->seq);
-    ccm->mpid = htons(cfm->mpid);
+    ccm->mpid = htons(cfm->mpid.mpid);
     ccm->flags = cfm->ccm_interval;
     memcpy(ccm->maid, cfm->maid, sizeof ccm->maid);
     memset(ccm->zero, 0, sizeof ccm->zero);
 
+    if (cfm->extended) {
+        memcpy(ccm->mpidx, cfm->mpid.mpidx, sizeof ccm->mpidx);
+    } else {
+        memset(ccm->mpidx, 0, sizeof ccm->mpidx);
+    }
+
     if (cfm->ccm_interval == 0) {
         assert(cfm->extended);
         ccm->interval_ms_x = htons(cfm->ccm_interval_ms);
@@ -337,11 +368,23 @@ cfm_configure(struct cfm *cfm, const struct cfm_settings *s)
     uint8_t interval;
     int interval_ms;
 
-    if (!cfm_is_valid_mpid(s->mpid) || s->interval <= 0) {
+    if (s->interval <= 0) {
+        return false;
+    }
+
+    if (s->extended) {
+        if (eth_addr_is_zero(s->mpidx)) {
+            return false;
+        }
+        memcpy(cfm->mpid.mpidx, s->mpidx, sizeof cfm->mpid.mpidx);
+        cfm->mpid.mpid = hash_mpid(cfm, &cfm->mpid);
+    } else if (cfm_is_valid_mpid(s->mpid)) {
+        cfm->mpid.mpid = s->mpid;
+        memset(cfm->mpid.mpidx, 0, sizeof cfm->mpid.mpidx);
+    } else {
         return false;
     }
 
-    cfm->mpid = s->mpid;
     cfm->extended = s->extended;
     interval = ms_to_ccm_interval(s->interval);
     interval_ms = ccm_interval_to_ms(interval);
@@ -410,27 +453,30 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
         VLOG_WARN_RL(&rl, "%s: Received unexpected remote MAID from MAC "
                      ETH_ADDR_FMT, cfm->name, ETH_ADDR_ARGS(eth->eth_src));
     } else {
-        uint16_t ccm_mpid = ntohs(ccm->mpid);
+        struct mpid ccm_mpid;
         uint8_t ccm_interval = ccm->flags & 0x7;
         bool ccm_rdi = ccm->flags & CCM_RDI_MASK;
         uint16_t ccm_interval_ms_x = ntohs(ccm->interval_ms_x);
 
         struct remote_mp *rmp;
 
+        ccm_mpid.mpid = ntohs(ccm->mpid);
+        memcpy(ccm_mpid.mpidx, ccm->mpidx, sizeof ccm_mpid.mpidx);
+
         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);
+                         " (%"PRIu8") from RMP "MPID_FMT, cfm->name,
+                         ccm_interval, MPID_ARGS(&ccm_mpid));
         }
 
         if (cfm->extended && ccm_interval == 0
             && ccm_interval_ms_x != cfm->ccm_interval_ms) {
             VLOG_WARN_RL(&rl, "%s: received a CCM with an invalid extended"
-                         " interval (%"PRIu16"ms) from RMP %"PRIu16, cfm->name,
-                         ccm_interval_ms_x, ccm_mpid);
+                         " interval (%"PRIu16"ms) from RMP "MPID_FMT,
+                         cfm->name, ccm_interval_ms_x, MPID_ARGS(&ccm_mpid));
         }
 
-        rmp = lookup_remote_mp(cfm, ccm_mpid);
+        rmp = lookup_remote_mp(cfm, &ccm_mpid);
         if (rmp) {
             rmp->recv = true;
             rmp->rdi = ccm_rdi;
@@ -439,16 +485,18 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
             rmp->mpid = ccm_mpid;
             rmp->recv = true;
             rmp->rdi = ccm_rdi;
-            hmap_insert(&cfm->remote_mps, &rmp->node, hash_mpid(rmp->mpid));
+            hmap_insert(&cfm->remote_mps, &rmp->node,
+                        hash_mpid(cfm, &rmp->mpid));
         } else {
-            VLOG_WARN_RL(&rl, "%s: Dropped CCM with MPID %d from MAC "
-                         ETH_ADDR_FMT, cfm->name, ccm_mpid,
+            VLOG_WARN_RL(&rl, "%s: dropped CCM with MPID "MPID_FMT" from MAC "
+                         ETH_ADDR_FMT, cfm->name, MPID_ARGS(&ccm_mpid),
                          ETH_ADDR_ARGS(eth->eth_src));
         }
 
-        VLOG_DBG("%s: Received CCM (seq %"PRIu32") (mpid %"PRIu16")"
+        VLOG_DBG("%s: received CCM (seq %"PRIu32") (mpid "MPID_FMT")"
                  " (interval %"PRIu8") (RDI %s)", cfm->name, ntohl(ccm->seq),
-                 ccm_mpid, ccm_interval, ccm_rdi ? "true" : "false");
+                 MPID_ARGS(&ccm_mpid), ccm_interval,
+                 ccm_rdi ? "true" : "false");
     }
 }
 
@@ -487,7 +535,7 @@ cfm_unixctl_show(struct unixctl_conn *conn,
         return;
     }
 
-    ds_put_format(&ds, "MPID %"PRIu16":%s%s\n", cfm->mpid,
+    ds_put_format(&ds, "MPID "MPID_FMT":%s%s\n", MPID_ARGS(&cfm->mpid),
                   cfm->fault ? " fault" : "",
                   cfm->xrecv ? " xrecv" : "");
 
@@ -499,7 +547,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,
+        ds_put_format(&ds, "Remote MPID "MPID_FMT":%s\n",
+                      MPID_ARGS(&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 092e823..c167258 100644
--- a/lib/cfm.h
+++ b/lib/cfm.h
@@ -29,7 +29,9 @@ struct ofpbuf;
 struct cfm_settings {
     uint16_t mpid;              /* The MPID of this CFM. */
     int interval;               /* The requested transmission interval. */
+
     bool extended;              /* Run in extended mode. */
+    uint8_t mpidx[6];           /* The MPID when in extended mode. */
 };
 
 void cfm_init(void);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 18a5cf2..fbcaa2e 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2593,12 +2593,10 @@ iface_configure_cfm(struct iface *iface)
     const char *extended_str;
     struct cfm_settings s;
 
-    if (!cfg->n_cfm_mpid) {
-        ofproto_port_clear_cfm(iface->port->bridge->ofproto, iface->ofp_port);
-        return;
+    if (!cfg->cfm_mpid) {
+        goto error;
     }
 
-    s.mpid = *cfg->cfm_mpid;
     s.interval = atoi(get_interface_other_config(iface->cfg, "cfm_interval",
                                                  "0"));
     if (s.interval <= 0) {
@@ -2609,7 +2607,39 @@ iface_configure_cfm(struct iface *iface)
                                               "false");
     s.extended = !strcasecmp("true", extended_str);
 
+    if (s.extended) {
+        int scan_count;
+
+        scan_count = sscanf(cfg->cfm_mpid, ETH_ADDR_SCAN_FMT,
+                            ETH_ADDR_SCAN_ARGS(s.mpidx));
+
+        if (scan_count != ETH_ADDR_SCAN_COUNT) {
+            goto error;
+        }
+        s.mpid = 0;
+    } else {
+        unsigned long long int mpid;
+
+        /* This uses an oversized destination field (64 bits when 32 bits would do)
+         * because some sscanf() implementations truncate the range of %i
+         * directives, so that e.g. "%"SCNi16 interprets input of "0xfedc" as a
+         * value of 0x7fff.  The other alternatives are to allow only a single
+         * radix (e.g. decimal or hexadecimal) or to write more sophisticated
+         * parsers. */
+        if (sscanf(cfg->cfm_mpid, "%lli", &mpid) == 1 && mpid <= UINT16_MAX) {
+            s.mpid = mpid;
+        } else {
+            goto error;
+        }
+        memset(s.mpidx, 0, sizeof s.mpidx);
+    }
+
     ofproto_port_set_cfm(iface->port->bridge->ofproto, iface->ofp_port, &s);
+    return;
+
+error:
+    ofproto_port_clear_cfm(iface->port->bridge->ofproto, iface->ofp_port);
+
 }
 
 /* Read carrier or miimon status directly from 'iface''s netdev, according to
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 43ff95d..53cd1f0 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
  "version": "6.0.0",
- "cksum": "1100213054 14376",
+ "cksum": "2055559034 14338",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -168,7 +168,7 @@
          "ephemeral": true},
        "cfm_mpid": {
          "type": {
-           "key": {"type": "integer", "minInteger": 1, "maxInteger": 8191},
+           "key": {"type": "string"},
            "min": 0,
            "max": 1}},
        "cfm_fault": {
-- 
1.7.6.1




More information about the dev mailing list