[ovs-dev] [PATCH] Allow passthrough of BPDU (control class) frames

ssane at nicira.com ssane at nicira.com
Mon Aug 8 20:20:52 UTC 2011


From: Sanjay Sane <ssane at nicira.com>

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 mybridge other-config:enable-bpdu-passthrough=true

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

--------
unit tests:

------------
make config changes, test database as well as runtime behavior


# ovs-vsctl list Bridge | grep other_config
other_config        : {}
# ovs-vsctl set bridge mybridge other-config:enable-bpdu-passthrough=true
# ovs-vsctl list Bridge | grep other_config
other_config        : {enable-bpdu-passthrough="true"}
# ovs-vsctl set bridge mybridge other-config:enable-bpdu-passthrough=false
# ovs-vsctl list Bridge | grep other_config
other_config        : {enable-bpdu-passthrough="false"}
# ovs-vsctl set bridge mybridge other-config:enable-bpdu-passthrough=true
# ovs-vsctl list Bridge | grep other_config
other_config        : {enable-bpdu-passthrough="true"}
# ovs-vsctl remove bridge mybridge other-config enable-bpdu-passthrough=true
# ovs-vsctl list Bridge | grep other_config
other_config        : {}
#

continuously send packets to mybridge with dest-mac=01:80:c2:00:00:00

# ovs-dpctl dump-flows mybridge
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:21827, bytes:1309620, used:0.000s, actions:drop
# ovs-vsctl set bridge mybridge other-config:enable-bpdu-passthrough=true
# ovs-dpctl dump-flows mybridge
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:7493, bytes:449580, used:0.004s, actions:0,2
# ovs-vsctl set bridge mybridge other-config:enable-bpdu-passthrough=false
# ovs-dpctl dump-flows mybridge
# ovs-dpctl dump-flows mybridge
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
# ovs-dpctl dump-flows mybridge
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:12449, bytes:746940, used:0.000s, actions:drop
# ovs-dpctl dump-flows mybridge
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:26158, bytes:1569480, used:0.000s, actions:drop
# ovs-vsctl set bridge mybridge other-config:enable-bpdu-passthrough=true
# ovs-dpctl dump-flows mybridge
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:9522, bytes:571320, used:0.000s, actions:0,2
# ovs-vsctl remove bridge mybridge other-config enable-bpdu-passthrough=true
# ovs-dpctl dump-flows mybridge
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:9216, bytes:552960, used:0.000s, actions:drop
#

---------

other tests:

-- test packets that OVS cares about, such as LACP (which fall into same range), and ensure that we consume these packets
when LACP is enabled, even if bpdu-passthrough is enabled
{it seems we always consume LACP frames, even if LACP is not enabled..}


Bug #6624
Reported-by: Niklas Andersson <nandersson at nicira.com>
---
 AUTHORS                    |    2 ++
 ofproto/ofproto-dpif.c     |   18 ++++++++++++++++--
 ofproto/ofproto-provider.h |    6 ++++++
 ofproto/ofproto.c          |   17 +++++++++++++++++
 ofproto/ofproto.h          |    1 +
 vswitchd/bridge.c          |   16 ++++++++++++++++
 vswitchd/vswitch.xml       |    6 ++++++
 7 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index aeb3262..34e4370 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -28,6 +28,7 @@ Pravin B Shelar         pshelar at nicira.com
 Reid Price              reid at nicira.com
 Romain Lenglet          romain.lenglet at berabera.info
 Sajjad Lateef           slateef at nicira.com
+Sanjay Sane             ssane at nicira.com
 Shih-Hao Li             shli at nicira.com
 Simon Horman            horms at verge.net.au
 Tetsuo NAKAGAWA         nakagawa at mxc.nes.nec.co.jp
@@ -75,6 +76,7 @@ Krishna Miriyala        krishna at nicira.com
 Luiz Henrique Ozaki     luiz.ozaki at gmail.com
 Michael Mao             mmao at nicira.com
 Mikael Doverhag         mdoverhag at nicira.com
+Niklas Andersson        nandersson at nicira.com
 Pankaj Thakkar          thakkar at nicira.com
 Paulo Cravero           pcravero at as2594.net
 Peter Balland           peter at nicira.com
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index cdc21bc..edd6ed9 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1393,6 +1393,17 @@ is_mirror_output_bundle(struct ofproto *ofproto_, void *aux)
     struct ofbundle *bundle = bundle_lookup(ofproto, aux);
     return bundle && bundle->mirror_out != 0;
 }
+
+static void
+bpdu_passthru_changed(struct ofproto *ofproto_, uint8_t old_val, 
+                      uint8_t new_val)
+{
+    struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
+    /* revalidate cached flows in either case */
+    if(new_val != old_val){
+        ofproto->need_revalidate = true;
+    }
+}
 
 /* Ports. */
 
@@ -3735,8 +3746,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 enable-bpdu-passthrough option is absent */
+    if (eth_addr_is_reserved(flow->dl_dst) && 
+	!ofproto->up.bpdu_passthru) {
         return false;
     }
 
@@ -4133,4 +4146,5 @@ const struct ofproto_class ofproto_dpif_class = {
     mirror_set,
     set_flood_vlans,
     is_mirror_output_bundle,
+    bpdu_passthru_changed,
 };
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index be1e4de..df86c87 100644
--- 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 */
+    uint8_t bpdu_passthru;      /* option to allow passthrough of BPDU
+                                 * when NORMAL action is invoked */
     char *mfr_desc;             /* Manufacturer. */
     char *hw_desc;              /* Hardware. */
     char *sw_desc;              /* Software version. */
@@ -914,6 +916,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);
+
+    /* Notifies change in configuration option of bpdu passthrough */
+    void (*bpdu_passthru_changed)(struct ofproto *ofproto,
+                                  uint8_t old_val, uint8_t new_val);
 };
 
 extern const struct ofproto_class ofproto_dpif_class;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f40f995..0b98438 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -322,6 +322,8 @@ 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);
+    /* by default, disable bpdu passthrough */
+    ofproto_set_bpdu_passthru(ofproto,0);
     ofproto->fallback_dpid = pick_fallback_dpid();
     ofproto->mfr_desc = xstrdup(DEFAULT_MFR_DESC);
     ofproto->hw_desc = xstrdup(DEFAULT_HW_DESC);
@@ -421,6 +423,21 @@ ofproto_set_flow_eviction_threshold(struct ofproto *ofproto, unsigned threshold)
     }
 }
 
+/* sets the option to enable passthrough of BPDUs when NORMAL action
+ * is invoked. */
+void
+ofproto_set_bpdu_passthru(struct ofproto *ofproto, uint8_t bpdu_passthru)
+{
+    uint8_t old_passthru = ofproto->bpdu_passthru ;
+    ofproto->bpdu_passthru = bpdu_passthru;
+    if (old_passthru != ofproto->bpdu_passthru){
+        if (ofproto->ofproto_class->bpdu_passthru_changed){
+            ofproto->ofproto_class->bpdu_passthru_changed(
+                ofproto, old_passthru, ofproto->bpdu_passthru);
+        }
+    }   
+}
+
 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..5a07a77 100644
--- 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_bpdu_passthru(struct ofproto *, uint8_t bpdu_passthru);
 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 6a4ebe5..b35c68b 100644
--- 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_bpdu_passthru(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_bpdu_passthru(br);
         bridge_configure_remotes(br, managers, n_managers);
         bridge_configure_netflow(br);
         bridge_configure_sflow(br, &sflow_bridge_number);
@@ -980,6 +982,20 @@ bridge_configure_flow_eviction_threshold(struct bridge *br)
     ofproto_set_flow_eviction_threshold(br->ofproto, threshold);
 }
 
+/* Set BPDU passthrough option */
+static void
+bridge_configure_bpdu_passthru(struct bridge *br)
+{
+    const char *bpdu_passthru_str;
+    uint8_t bpdu_passthru = 0;
+
+    bpdu_passthru_str = bridge_get_other_config(br->cfg, "enable-bpdu-passthrough");
+    if(bpdu_passthru_str && !strcmp(bpdu_passthru_str, "true")) {
+        bpdu_passthru = 1;
+    }
+    ofproto_set_bpdu_passthru(br->ofproto, bpdu_passthru);
+}
+
 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 b3029eb..edc90aa 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -453,6 +453,12 @@
 	  <dd>
             Values below 100 will be rounded up to 100.
           </dd>
+          <dt><code>enable-bpdu-passthrough</code></dt>
+          <dd>
+            Option to allow passthrough of BPDU frames when NORMAL 
+            action if invoked. Default is disabled, set to 
+            <code>true</code> to enable. 
+          </dd>
         </dl>
       </column>
     </group>
-- 
1.7.2.5




More information about the dev mailing list