From 8402c74b186e28c53ad51fc8813aebe64a12cd7c Mon Sep 17 00:00:00 2001 From: Sanjay Sane Date: Tue, 9 Aug 2011 11:08:27 -0700 Subject: [PATCH] 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 --- AUTHORS | 2 ++ ofproto/ofproto-dpif.c | 15 +++++++++++++-- ofproto/ofproto-provider.h | 6 ++++++ ofproto/ofproto.c | 16 ++++++++++++++++ ofproto/ofproto.h | 1 + vswitchd/bridge.c | 16 ++++++++++++++++ vswitchd/vswitch.xml | 10 ++++++++++ 7 files changed, 64 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index cf75c790..8f4237ae 100644 --- 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 8b65becf..f53cc430 100644 --- 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; +} /* 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 8c32e4fc..037dbae5 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 */ + 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 2dd3c9f7..c94549fa 100644 --- 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 4975a8d3..e0c99eac 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_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 94668a73..d99d6a1c 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_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 b8aa813c..f56e670a 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -461,6 +461,16 @@
Values below 100 will be rounded up to 100.
+
forward-bpdu
+
+ 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 true to enable. +
-- 2.30.2