When STP/RSTP are in a disabled transition on a bridge, the sets will call bundle_update() for each port as it should transition from FORWARDING to DISABLED. However, this happens while the states are still non-NULL, which causes bundle_update() to assume that the STP/RSTP status will remain as forwarding. After this happens, the bundle->floodable flag will remain 'false' and NORMAL action flood attempts will skip over the ports.
In the case of RSTP, the state processing is a bit more complex, so we do a final bundle_update pass to reset the floodable flag. We could choose to do a more simplistic fix in STP case by releasing the ofproto->stp object early; however to keep it consistent, the fix will adopt the same approach of doing a final pass for bundle updates. Reported-at: https://redhat.atlassian.net/browse/FDP-3852 Fixes: 21f7563cef5f ("ovs-vswitchd: Add support for 802.1D STP.") Fixes: 9efd308e957c ("Rapid Spanning Tree Protocol (IEEE 802.1D).") Signed-off-by: Aaron Conole <[email protected]> --- ofproto/ofproto-dpif.c | 17 +++++++++ tests/ofproto-dpif.at | 82 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index a02afe8ef3..26a6da2541 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -2813,12 +2813,21 @@ set_rstp(struct ofproto *ofproto_, const struct ofproto_rstp_settings *s) rstp_set_bridge_transmit_hold_count(ofproto->rstp, s->transmit_hold_count); } else { + struct ofbundle *bundle; struct ofport *ofport; + HMAP_FOR_EACH (ofport, hmap_node, &ofproto->up.ports) { set_rstp_port(ofport, NULL); } rstp_unref(ofproto->rstp); ofproto->rstp = NULL; + + /* During processing above, RSTP state machine may have incorrectly + * recalculated the floodable state, due to a transition. Recalculate + * the bundle floodable flag now that RSTP states are quiet. */ + HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) { + bundle_update(bundle); + } } } @@ -2957,6 +2966,7 @@ set_stp(struct ofproto *ofproto_, const struct ofproto_stp_settings *s) stp_set_max_age(ofproto->stp, s->max_age); stp_set_forward_delay(ofproto->stp, s->fwd_delay); } else { + struct ofbundle *bundle; struct ofport *ofport; HMAP_FOR_EACH (ofport, hmap_node, &ofproto->up.ports) { @@ -2965,6 +2975,13 @@ set_stp(struct ofproto *ofproto_, const struct ofproto_stp_settings *s) stp_unref(ofproto->stp); ofproto->stp = NULL; + + /* During processing above, STP state machine may have incorrectly + * recalculated the floodable state, due to a transition. Recalculate + * the bundle floodable flag now that STP states are quiet. */ + HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) { + bundle_update(bundle); + } } return 0; diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index dcab7bba45..def246e877 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -13651,3 +13651,85 @@ AT_CHECK([ovs-appctl coverage/read-counter revalidate_missing_dp_flow], [0], OVS_VSWITCHD_STOP(["/failed to flow_del (No such file or directory)/d"]) AT_CLEANUP + +AT_SETUP([ofproto-dpif - bundle floodable flag after STP disabled]) +AT_KEYWORDS([stp bundle floodable]) +OVS_VSWITCHD_START + +dnl Setup STP +AT_CHECK([ovs-vsctl set bridge br0 stp_enable=true]) + +dnl Add ports for forwarding. +AT_CHECK([ovs-vsctl add-port br0 p1 -- set interface p1 type=dummy]) +AT_CHECK([ovs-vsctl add-port br0 p2 -- set interface p2 type=dummy]) + +dnl Ensure normal mode. +AT_CHECK([ovs-ofctl del-flows br0], [0], [ignore]) +AT_CHECK([ovs-ofctl add-flow br0 action=NORMAL], [0], [ignore]) + +dnl Verify bundle is non-floodable. +AT_CHECK([ovs-appctl dpif/show | grep -A5 br0], [0], [ignore]) + +dnl Esnure STP reached forwarding state. +OVS_WAIT_UNTIL([ovs-appctl stp/show br0 | grep "p1.*forwarding"]) + +dnl Capture port details in case. +AT_CHECK([ovs-ofctl dump-ports-desc br0], [0], [ignore]) + +dnl Now disable STP. +AT_CHECK([ovs-vsctl set bridge br0 stp_enable=false]) +OVS_WAIT_UNTIL([ovs-vsctl get bridge br0 stp_enable | grep false]) + +dnl Test flood behavior +dnl Send a broadcast packet on p2, verify it floods to p1 +AT_CHECK([ovs-appctl ofproto/trace br0 dnl + 'in_port=p2,eth_src=00:00:00:00:00:01,eth_dst=ff:ff:ff:ff:ff:ff' dnl + -generate], + [0], [stdout]) + +dnl Check that output includes p1 (proves it's floodable) +AT_CHECK([grep 'Datapath actions: 100,1' stdout], [0], [ignore]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([ofproto-dpif - bundle floodable flag after RSTP disabled]) +AT_KEYWORDS([rstp bundle floodable]) +OVS_VSWITCHD_START + +dnl Setup RSTP +AT_CHECK([ovs-vsctl set bridge br0 rstp_enable=true]) + +dnl Add ports for forwarding. +AT_CHECK([ovs-vsctl add-port br0 p1 -- set interface p1 type=dummy]) +AT_CHECK([ovs-vsctl add-port br0 p2 -- set interface p2 type=dummy]) + +dnl Ensure normal mode. +AT_CHECK([ovs-ofctl del-flows br0], [0], [ignore]) +AT_CHECK([ovs-ofctl add-flow br0 action=NORMAL], [0], [ignore]) + +dnl Verify bundle is non-floodable. +AT_CHECK([ovs-appctl dpif/show | grep -A5 br0], [0], [ignore]) + +dnl Esnure STP reached forwarding state. +OVS_WAIT_UNTIL([ovs-appctl rstp/show br0 | grep "p1.*Designated"]) + +dnl Capture port details in case. +AT_CHECK([ovs-ofctl dump-ports-desc br0], [0], [ignore]) + +dnl Now disable RSTP. +AT_CHECK([ovs-vsctl set bridge br0 rstp_enable=false]) +OVS_WAIT_UNTIL([ovs-vsctl get bridge br0 rstp_enable | grep false]) + +dnl Test flood behavior. +dnl Send a broadcast packet on p2, verify it floods to br0,p1 +AT_CHECK([ovs-appctl ofproto/trace br0 dnl + 'in_port=p2,eth_src=00:00:00:00:00:01,eth_dst=ff:ff:ff:ff:ff:ff' dnl + -generate], + [0], [stdout]) + +dnl Check that output includes p1 (proves it's floodable) +AT_CHECK([grep 'Datapath actions: 100,1' stdout], [0], [ignore]) + +OVS_VSWITCHD_STOP +AT_CLEANUP -- 2.51.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
