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.
For all cases, call bundle_update after the Spanning Tree changes have been calculated to make sure that final port details are correctly applied (in the case of port state transitions which can occur without subsequent port reconfiguration calls). 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).") Acked-by: Mike Pattrick <[email protected]> Signed-off-by: Aaron Conole <[email protected]> --- ofproto/ofproto-dpif.c | 15 +++++++++++ tests/ofproto-dpif.at | 56 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index a02afe8ef3..2360e2a580 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -2813,12 +2813,20 @@ 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; + + /* Now that ofproto no longer has RSTP configured, bundles need to + * have their 'floodable' states updated. */ + HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) { + bundle_update(bundle); + } } } @@ -2957,6 +2965,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 +2974,12 @@ set_stp(struct ofproto *ofproto_, const struct ofproto_stp_settings *s) stp_unref(ofproto->stp); ofproto->stp = NULL; + + /* Now that ofproto no longer has STP configured, bundles need to + * have their 'floodable' states updated. */ + 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..e2dbd5ffe9 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -13651,3 +13651,59 @@ 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 + +AT_CHECK([ovs-vsctl set bridge br0 stp_enable=true]) +add_of_ports br0 1 2 + +AT_CHECK([ovs-ofctl add-flow br0 action=NORMAL]) + +dnl Verify we are in forwarding state. +OVS_WAIT_UNTIL([ovs-appctl stp/show br0 | grep "p1.*forwarding"]) + +AT_CHECK([ovs-vsctl set bridge br0 stp_enable=false]) +OVS_WAIT_UNTIL([test -z "$(ovs-appctl stp/show)"]) + +dnl Send a broadcast packet on p2, verify it floods to p1. +AT_CHECK([ovs-appctl ofproto/trace br0 \ + 'in_port=p2,eth_src=00:00:00:00:00:01,eth_dst=ff:ff:ff:ff:ff:ff' \ + -generate], + [0], [stdout]) + +AT_CHECK([tail -1 stdout], [0], [dnl +Datapath actions: 100,1 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([ofproto-dpif - bundle floodable flag after RSTP disabled]) +AT_KEYWORDS([rstp bundle floodable]) +OVS_VSWITCHD_START + +AT_CHECK([ovs-vsctl set bridge br0 rstp_enable=true]) +add_of_ports br0 1 2 + +AT_CHECK([ovs-ofctl add-flow br0 action=NORMAL]) + +dnl Verify we are in forwarding state. +OVS_WAIT_UNTIL([ovs-appctl rstp/show br0 | grep "p1.*Designated"]) + +AT_CHECK([ovs-vsctl set bridge br0 rstp_enable=false]) +OVS_WAIT_UNTIL([test -z "$(ovs-appctl rstp/show)"]) + +dnl Send a broadcast packet on p2, verify it floods to p1. +AT_CHECK([ovs-appctl ofproto/trace br0 \ + 'in_port=p2,eth_src=00:00:00:00:00:01,eth_dst=ff:ff:ff:ff:ff:ff' \ + -generate], + [0], [stdout]) + +AT_CHECK([tail -1 stdout], [0], [dnl +Datapath actions: 100,1 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP -- 2.51.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
