Option to forward BPDU (Ethernet control class) frames

Currently, a NORMAL action bridge drops reserved-multicast-mac addresses;
01-80-c2-00-00-[f0:ff]. A node that does not implement STP should have an
option to forward such frames.

This commit proposes to have a configuration option to allow forwarding of
BPDU class frames.  To ensure backward compatibility, this option is
disabled by default.

This config can be set using bridge's other-config column, for e.g
ovs-vsctl set bridge br0 other-config:forward-bpdu=true

Changing this option can revalidate all flows in a software-OVS
implementation (ofproto-dpif)

--------
unit tests:

------------
make config changes, test runtime behavior

-- test runtime behavior --
continuously send packets to br0 with dest-mac=01:80:c2:00:00:00

ovs-dpctl dump-flows br0
ovs-vsctl set bridge br0 other-config:forward-bpdu=true
ovs-dpctl dump-flows br0
ovs-vsctl set bridge br0 other-config:forward-bpdu=false
ovs-dpctl dump-flows br0
ovs-vsctl set bridge br0 other-config:forward-bpdu=true
ovs-dpctl dump-flows br0
ovs-vsctl remove bridge br0 other-config forward-bpdu=true
ovs-dpctl dump-flows br0

--result--
 ovs-dpctl dump-flows br0
 in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:29550, bytes:1773000, used:0.004s, actions:drop
 ovs-vsctl set bridge br0 other-config:forward-bpdu=true

 ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:8209, bytes:492540, used:0.000s, actions:2,0

 ovs-vsctl set bridge br0 other-config:forward-bpdu=false
 ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:19, bytes:1140, used:0.000s, actions:drop

 ovs-vsctl set bridge br0 other-config:forward-bpdu=true
 ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:29, bytes:1740, used:0.000s, actions:2,0

 ovs-vsctl remove bridge br0 other-config forward-bpdu=true
 ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:0, bytes:0, used:never, actions:drop

Bug #6624
Reported-by: Niklas Andersson <nandersson@nicira.com>

diff --git a/AUTHORS b/AUTHORS
index cf75c79..8f4237a 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -29,6 +29,7 @@ Pravin B Shelar         pshelar@nicira.com
 Reid Price              reid@nicira.com
 Romain Lenglet          romain.lenglet@berabera.info
 Sajjad Lateef           slateef@nicira.com
+Sanjay Sane             ssane@nicira.com
 Shih-Hao Li             shli@nicira.com
 Simon Horman            horms@verge.net.au
 Tetsuo NAKAGAWA         nakagawa@mxc.nes.nec.co.jp
@@ -76,6 +77,7 @@ Krishna Miriyala        krishna@nicira.com
 Luiz Henrique Ozaki     luiz.ozaki@gmail.com
 Michael Mao             mmao@nicira.com
 Mikael Doverhag         mdoverhag@nicira.com
+Niklas Andersson        nandersson@nicira.com
 Pankaj Thakkar          thakkar@nicira.com
 Paulo Cravero           pcravero@as2594.net
 Peter Balland           peter@nicira.com

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8b65bec..f53cc43 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1395,6 +1395,14 @@ is_mirror_output_bundle(struct ofproto *ofproto_, void *aux)
     struct ofbundle *bundle = bundle_lookup(ofproto, aux);
     return bundle && bundle->mirror_out != 0;
 }
+
+static void
+forward_bpdu_changed(struct ofproto *ofproto_) 
+{
+    struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
+    /* Revalidate cached flows whenever forward_bpdu option changes. */
+    ofproto->need_revalidate = true;
+}
 \f
 /* Ports. */
 
@@ -3769,8 +3777,10 @@ is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
         return false;
     }
 
-    /* Drop frames for reserved multicast addresses. */
-    if (eth_addr_is_reserved(flow->dl_dst)) {
+    /* Drop frames for reserved multicast addresses 
+     * only if forward_bpdu option is absent. */
+    if (eth_addr_is_reserved(flow->dl_dst) && 
+        !ofproto->up.forward_bpdu) {
         return false;
     }
 
@@ -4203,4 +4213,5 @@ const struct ofproto_class ofproto_dpif_class = {
     mirror_set,
     set_flood_vlans,
     is_mirror_output_bundle,
+    forward_bpdu_changed,
 };

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 8c32e4f..037dbae 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -41,6 +41,8 @@ struct ofproto {
     unsigned flow_eviction_threshold; /* Threshold at which to begin flow
                                        * table eviction. Only affects the
                                        * ofproto-dpif implementation */
+    bool forward_bpdu;          /* Option to allow forwarding of BPDU frames
+                                 * when NORMAL action is invoked. */
     char *mfr_desc;             /* Manufacturer. */
     char *hw_desc;              /* Hardware. */
     char *sw_desc;              /* Software version. */
@@ -926,6 +928,10 @@ struct ofproto_class {
     /* Returns true if 'aux' is a registered bundle that is currently in use as
      * the output for a mirror. */
     bool (*is_mirror_output_bundle)(struct ofproto *ofproto, void *aux);
+
+    /* When the configuration option of forward_bpdu changes, this function
+     * will be invoked. */
+    void (*forward_bpdu_changed)(struct ofproto *ofproto);
 };
 
 extern const struct ofproto_class ofproto_dpif_class;

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 2dd3c9f..c94549f 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -324,6 +324,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->datapath_id = 0;
     ofproto_set_flow_eviction_threshold(ofproto,
                                         OFPROTO_FLOW_EVICTON_THRESHOLD_DEFAULT);
+    ofproto->forward_bpdu = false;
     ofproto->fallback_dpid = pick_fallback_dpid();
     ofproto->mfr_desc = xstrdup(DEFAULT_MFR_DESC);
     ofproto->hw_desc = xstrdup(DEFAULT_HW_DESC);
@@ -429,6 +430,21 @@ ofproto_set_flow_eviction_threshold(struct ofproto *ofproto, unsigned threshold)
     }
 }
 
+/* If forward_bpdu is true, the NORMAL action will forward frames with 
+ * reserved (e.g. STP) destination Ethernet addresses. if forward_bpdu is false,
+ * the NORMAL action will drop these frames. */
+void
+ofproto_set_forward_bpdu(struct ofproto *ofproto, bool forward_bpdu)
+{
+    bool old_val = ofproto->forward_bpdu;
+    ofproto->forward_bpdu = forward_bpdu;
+    if (old_val != ofproto->forward_bpdu) {
+        if (ofproto->ofproto_class->forward_bpdu_changed) {
+            ofproto->ofproto_class->forward_bpdu_changed(ofproto);
+        }
+    }   
+}
+
 void
 ofproto_set_desc(struct ofproto *p,
                  const char *mfr_desc, const char *hw_desc,

diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 4975a8d..e0c99ea 100644 (file)
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -160,6 +160,7 @@ void ofproto_set_extra_in_band_remotes(struct ofproto *,
                                        const struct sockaddr_in *, size_t n);
 void ofproto_set_in_band_queue(struct ofproto *, int queue_id);
 void ofproto_set_flow_eviction_threshold(struct ofproto *, unsigned threshold);
+void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
 void ofproto_set_desc(struct ofproto *,
                       const char *mfr_desc, const char *hw_desc,
                       const char *sw_desc, const char *serial_desc,

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 94668a7..d99d6a1 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -150,6 +150,7 @@ static void bridge_refresh_ofp_port(struct bridge *);
 static void bridge_configure_datapath_id(struct bridge *);
 static void bridge_configure_flow_eviction_threshold(struct bridge *);
 static void bridge_configure_netflow(struct bridge *);
+static void bridge_configure_forward_bpdu(struct bridge *);
 static void bridge_configure_sflow(struct bridge *, int *sflow_bridge_number);
 static void bridge_configure_remotes(struct bridge *,
                                      const struct sockaddr_in *managers,
@@ -414,6 +415,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
         bridge_configure_mirrors(br);
         bridge_configure_datapath_id(br);
         bridge_configure_flow_eviction_threshold(br);
+        bridge_configure_forward_bpdu(br);
         bridge_configure_remotes(br, managers, n_managers);
         bridge_configure_netflow(br);
         bridge_configure_sflow(br, &sflow_bridge_number);
@@ -982,6 +984,20 @@ bridge_configure_flow_eviction_threshold(struct bridge *br)
     ofproto_set_flow_eviction_threshold(br->ofproto, threshold);
 }
 
+/* Set forward BPDU option. */
+static void
+bridge_configure_forward_bpdu(struct bridge *br)
+{
+    const char *forward_bpdu_str;
+    bool forward_bpdu = false;
+
+    forward_bpdu_str = bridge_get_other_config(br->cfg, "forward-bpdu");
+    if (forward_bpdu_str && !strcmp(forward_bpdu_str, "true")) {
+        forward_bpdu = true;
+    }
+    ofproto_set_forward_bpdu(br->ofproto, forward_bpdu);
+}
+
 static void
 bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
                           struct iface **hw_addr_iface)

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index b8aa813..f56e670 100644 (file)
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -461,6 +461,16 @@
          <dd>
             Values below 100 will be rounded up to 100.
           </dd>
+          <dt><code>forward-bpdu</code></dt>
+          <dd>
+            Option to allow forwarding of BPDU frames when NORMAL 
+            action if invoked. Frames with reserved Ethernet addresses 
+            (e.g. STP BPDU) will be forwarded when this option is enabled. 
+            If the Open vSwitch bridge is used to connect different
+            Ethernet networks, and if Open vSwtich node does not run STP, 
+            then this option should be enabled.    
+            Default is disabled, set to <code>true</code> to enable. 
+          </dd>
         </dl>
       </column>
     </group>