ofproto-dpif-xlate: Pull STP xlation into ofproto-dpif-xlate.
authorEthan Jackson <ethan@nicira.com>
Sat, 6 Jul 2013 16:31:35 +0000 (09:31 -0700)
committerEthan Jackson <ethan@nicira.com>
Fri, 2 Aug 2013 23:16:45 +0000 (16:16 -0700)
This patch pulls the STP xlation code into ofproto-dpif-xlate where it
will be easier to guard.

Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
ofproto/ofproto-dpif-xlate.c
ofproto/ofproto-dpif-xlate.h
ofproto/ofproto-dpif.c
ofproto/ofproto-dpif.h

index ae677e7..066de47 100644 (file)
@@ -65,9 +65,9 @@ struct xbridge {
     struct mbridge *mbridge;      /* Mirroring. */
     struct dpif_sflow *sflow;     /* SFlow handle, or null. */
     struct dpif_ipfix *ipfix;     /* Ipfix handle, or null. */
+    struct stp *stp;              /* STP or null if disabled. */
 
     enum ofp_config_flags frag;   /* Fragmentation handling. */
-    bool has_stp;                 /* Bridge runs stp? */
     bool has_netflow;             /* Bridge runs netflow? */
     bool has_in_band;             /* Bridge has in band control? */
     bool forward_bpdu;            /* Bridge forwards STP BPDUs? */
@@ -112,7 +112,7 @@ struct xport {
     struct xport *peer;              /* Patch port peer or null. */
 
     enum ofputil_port_config config; /* OpenFlow port configuration. */
-    enum stp_state stp_state;        /* STP_DISABLED if STP not in use. */
+    int stp_port_no;                 /* STP port number or 0 if not in use. */
 
     bool may_enable;                 /* May be enabled in bonds. */
     bool is_tunnel;                  /* Is a tunnel port. */
@@ -190,11 +190,11 @@ static struct xport *get_ofp_port(const struct xbridge *, ofp_port_t ofp_port);
 
 void
 xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
-                  const struct mac_learning *ml, const struct mbridge *mbridge,
+                  const struct mac_learning *ml, struct stp *stp,
+                  const struct mbridge *mbridge,
                   const struct dpif_sflow *sflow,
                   const struct dpif_ipfix *ipfix, enum ofp_config_flags frag,
-                  bool forward_bpdu, bool has_in_band, bool has_netflow,
-                  bool has_stp)
+                  bool forward_bpdu, bool has_in_band, bool has_netflow)
 {
     struct xbridge *xbridge = xbridge_lookup(ofproto);
 
@@ -227,13 +227,17 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name,
         xbridge->ipfix = dpif_ipfix_ref(ipfix);
     }
 
+    if (xbridge->stp != stp) {
+        stp_unref(xbridge->stp);
+        xbridge->stp = stp_ref(stp);
+    }
+
     free(xbridge->name);
     xbridge->name = xstrdup(name);
 
     xbridge->forward_bpdu = forward_bpdu;
     xbridge->has_in_band = has_in_band;
     xbridge->has_netflow = has_netflow;
-    xbridge->has_stp = has_stp;
     xbridge->frag = frag;
 }
 
@@ -330,8 +334,9 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
                  struct ofport_dpif *ofport, ofp_port_t ofp_port,
                  odp_port_t odp_port, const struct netdev *netdev,
                  const struct cfm *cfm, const struct bfd *bfd,
-                 struct ofport_dpif *peer, enum ofputil_port_config config,
-                 enum stp_state stp_state, bool is_tunnel, bool may_enable)
+                 struct ofport_dpif *peer, int stp_port_no,
+                 enum ofputil_port_config config, bool is_tunnel,
+                 bool may_enable)
 {
     struct xport *xport = xport_lookup(ofport);
 
@@ -349,7 +354,7 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
     ovs_assert(xport->ofp_port == ofp_port);
 
     xport->config = config;
-    xport->stp_state = stp_state;
+    xport->stp_port_no = stp_port_no;
     xport->is_tunnel = is_tunnel;
     xport->may_enable = may_enable;
     xport->odp_port = odp_port;
@@ -455,6 +460,61 @@ xport_lookup(struct ofport_dpif *ofport)
     return NULL;
 }
 
+
+static enum stp_state
+xport_stp_learn_state(const struct xport *xport)
+{
+    enum stp_state stp_state = xport->xbridge->stp && xport->stp_port_no
+        ? stp_port_get_state(stp_get_port(xport->xbridge->stp,
+                                          xport->stp_port_no))
+        : STP_DISABLED;
+    return stp_learn_in_state(stp_state);
+}
+
+static bool
+xport_stp_forward_state(const struct xport *xport)
+{
+    enum stp_state stp_state = xport->xbridge->stp && xport->stp_port_no
+        ? stp_port_get_state(stp_get_port(xport->xbridge->stp,
+                                          xport->stp_port_no))
+        : STP_DISABLED;
+    return stp_forward_in_state(stp_state);
+}
+
+/* Returns true if STP should process 'flow'.  Sets fields in 'wc' that
+ * were used to make the determination.*/
+static bool
+stp_should_process_flow(const struct flow *flow, struct flow_wildcards *wc)
+{
+    memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
+    return eth_addr_equals(flow->dl_dst, eth_addr_stp);
+}
+
+static void
+stp_process_packet(const struct xport *xport, const struct ofpbuf *packet)
+{
+    struct ofpbuf payload = *packet;
+    struct eth_header *eth = payload.data;
+    struct stp_port *sp = xport->xbridge->stp && xport->stp_port_no
+        ? stp_get_port(xport->xbridge->stp, xport->stp_port_no)
+        : NULL;
+
+    /* Sink packets on ports that have STP disabled when the bridge has
+     * STP enabled. */
+    if (!sp || stp_port_get_state(sp) == STP_DISABLED) {
+        return;
+    }
+
+    /* Trim off padding on payload. */
+    if (payload.size > ntohs(eth->eth_type) + ETH_HEADER_LEN) {
+        payload.size = ntohs(eth->eth_type) + ETH_HEADER_LEN;
+    }
+
+    if (ofpbuf_try_pull(&payload, ETH_HEADER_LEN + LLC_HEADER_LEN)) {
+        stp_received_bpdu(sp, payload.data, payload.size);
+    }
+}
+
 static struct xport *
 get_ofp_port(const struct xbridge *xbridge, ofp_port_t ofp_port)
 {
@@ -1213,9 +1273,9 @@ process_special(struct xlate_ctx *ctx, const struct flow *flow,
             lacp_process_packet(xport->xbundle->lacp, xport->ofport, packet);
         }
         return SLOW_LACP;
-    } else if (xbridge->has_stp && stp_should_process_flow(flow, wc)) {
+    } else if (xbridge->stp && stp_should_process_flow(flow, wc)) {
         if (packet) {
-            stp_process_packet(xport->ofport, packet);
+            stp_process_packet(xport, packet);
         }
         return SLOW_STP;
     } else {
@@ -1246,7 +1306,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     } else if (xport->config & OFPUTIL_PC_NO_FWD) {
         xlate_report(ctx, "OFPPC_NO_FWD set, skipping output");
         return;
-    } else if (check_stp && !stp_forward_in_state(xport->stp_state)) {
+    } else if (check_stp && !xport_stp_forward_state(xport)) {
         xlate_report(ctx, "STP not in forwarding state, skipping output");
         return;
     }
@@ -1272,7 +1332,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         if (special) {
             ctx->xout->slow = special;
         } else if (may_receive(peer, ctx)) {
-            if (stp_forward_in_state(peer->stp_state)) {
+            if (xport_stp_forward_state(peer)) {
                 xlate_table_action(ctx, flow->in_port.ofp_port, 0, true);
             } else {
                 /* Forwarding is disabled by STP.  Let OFPP_NORMAL and the
@@ -1880,8 +1940,7 @@ may_receive(const struct xport *xport, struct xlate_ctx *ctx)
      * disabled.  If just learning is enabled, we need to have
      * OFPP_NORMAL and the learning action have a look at the packet
      * before we can drop it. */
-    if (!stp_forward_in_state(xport->stp_state)
-        && !stp_learn_in_state(xport->stp_state)) {
+    if (!xport_stp_forward_state(xport) && !xport_stp_learn_state(xport)) {
         return false;
     }
 
@@ -2372,7 +2431,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
             /* We've let OFPP_NORMAL and the learning action look at the
              * packet, so drop it now if forwarding is disabled. */
-            if (in_port && !stp_forward_in_state(in_port->stp_state)) {
+            if (in_port && !xport_stp_forward_state(in_port)) {
                 ctx.xout->odp_actions.size = sample_actions_len;
             }
         }
index 6c21267..3ce5aa9 100644 (file)
@@ -110,10 +110,10 @@ struct xlate_in {
 };
 
 void xlate_ofproto_set(struct ofproto_dpif *, const char *name,
-                       const struct mac_learning *, const struct mbridge *,
-                       const struct dpif_sflow *, const struct dpif_ipfix *,
-                       enum ofp_config_flags, bool forward_bpdu,
-                       bool has_in_band, bool has_netflow, bool has_stp);
+                       const struct mac_learning *, struct stp *,
+                       const struct mbridge *, const struct dpif_sflow *,
+                       const struct dpif_ipfix *, enum ofp_config_flags,
+                       bool forward_bpdu, bool has_in_band, bool has_netflow);
 void xlate_remove_ofproto(struct ofproto_dpif *);
 
 void xlate_bundle_set(struct ofproto_dpif *, struct ofbundle *,
@@ -127,8 +127,8 @@ void xlate_ofport_set(struct ofproto_dpif *, struct ofbundle *,
                       struct ofport_dpif *, ofp_port_t, odp_port_t,
                       const struct netdev *, const struct cfm *,
                       const struct bfd *, struct ofport_dpif *peer,
-                      enum ofputil_port_config, enum stp_state, bool is_tunnel,
-                      bool may_enable);
+                      int stp_port_no, enum ofputil_port_config,
+                      bool is_tunnel, bool may_enable);
 void xlate_ofport_remove(struct ofport_dpif *);
 
 void xlate_actions(struct xlate_in *, struct xlate_out *);
index 68b455b..56b1796 100644 (file)
@@ -771,12 +771,12 @@ type_run(const char *type)
             }
 
             xlate_ofproto_set(ofproto, ofproto->up.name, ofproto->ml,
-                              ofproto->mbridge, ofproto->sflow,
-                              ofproto->ipfix, ofproto->up.frag_handling,
+                              ofproto->stp, ofproto->mbridge,
+                              ofproto->sflow, ofproto->ipfix,
+                              ofproto->up.frag_handling,
                               ofproto->up.forward_bpdu,
                               connmgr_has_in_band(ofproto->up.connmgr),
-                              ofproto->netflow != NULL,
-                              ofproto->stp != NULL);
+                              ofproto->netflow != NULL);
 
             HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
                 xlate_bundle_set(ofproto, bundle, bundle->name,
@@ -787,12 +787,15 @@ type_run(const char *type)
             }
 
             HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
+                int stp_port = ofport->stp_port
+                    ? stp_port_no(ofport->stp_port)
+                    : 0;
                 xlate_ofport_set(ofproto, ofport->bundle, ofport,
                                  ofport->up.ofp_port, ofport->odp_port,
                                  ofport->up.netdev, ofport->cfm,
-                                 ofport->bfd, ofport->peer,
-                                 ofport->up.pp.config, ofport->stp_state,
-                                 ofport->is_tunnel, ofport->may_enable);
+                                 ofport->bfd, ofport->peer, stp_port,
+                                 ofport->up.pp.config, ofport->is_tunnel,
+                                 ofport->may_enable);
             }
 
             cls_cursor_init(&cursor, &ofproto->facets, NULL);
@@ -2175,39 +2178,6 @@ stp_wait(struct ofproto_dpif *ofproto)
         poll_timer_wait(1000);
     }
 }
-
-/* Returns true if STP should process 'flow'.  Sets fields in 'wc' that
- * were used to make the determination.*/
-bool
-stp_should_process_flow(const struct flow *flow, struct flow_wildcards *wc)
-{
-    memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
-    return eth_addr_equals(flow->dl_dst, eth_addr_stp);
-}
-
-void
-stp_process_packet(const struct ofport_dpif *ofport,
-                   const struct ofpbuf *packet)
-{
-    struct ofpbuf payload = *packet;
-    struct eth_header *eth = payload.data;
-    struct stp_port *sp = ofport->stp_port;
-
-    /* Sink packets on ports that have STP disabled when the bridge has
-     * STP enabled. */
-    if (!sp || stp_port_get_state(sp) == STP_DISABLED) {
-        return;
-    }
-
-    /* Trim off padding on payload. */
-    if (payload.size > ntohs(eth->eth_type) + ETH_HEADER_LEN) {
-        payload.size = ntohs(eth->eth_type) + ETH_HEADER_LEN;
-    }
-
-    if (ofpbuf_try_pull(&payload, ETH_HEADER_LEN + LLC_HEADER_LEN)) {
-        stp_received_bpdu(sp, payload.data, payload.size);
-    }
-}
 \f
 int
 ofproto_dpif_queue_to_priority(const struct ofproto_dpif *ofproto,
index f683281..9e8a107 100644 (file)
@@ -69,10 +69,6 @@ size_t put_userspace_action(const struct ofproto_dpif *,
                             const union user_action_cookie *,
                             const size_t cookie_size);
 
-bool stp_should_process_flow(const struct flow *, struct flow_wildcards *);
-void stp_process_packet(const struct ofport_dpif *,
-                        const struct ofpbuf *packet);
-
 bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
 ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
                                   ofp_port_t realdev_ofp_port,