When a bridge stp enabled, we assign sequentially element of stp_port array (in stp struct) to bridge ports. That is ok when no ports are added to bridge. When adding a port to bridge which stp enabled, the ovs-vswitchd will assign stp_port sequentially again. Then the stp_port belonging to one port may belong to other one. This patch uses the OpenFlow port numbers instead of sequence numbers to avoid it.
Signed-off-by: nickcooper-zhangtonghao <[email protected]> --- tests/stp.at | 90 ++++++++++++++++++++++++++++++++++++++++++------------- vswitchd/bridge.c | 10 +++++-- 2 files changed, 77 insertions(+), 23 deletions(-) diff --git a/tests/stp.at b/tests/stp.at index bd5d208..98632a8 100644 --- a/tests/stp.at +++ b/tests/stp.at @@ -570,11 +570,6 @@ for i in $(seq 0 35); do done # root bridge sends query packet -# we don't want to lose that message, so send it twice -AT_CHECK([ovs-appctl netdev-dummy/receive br0 \ - '01005E010101000C29A027D18100000108004500001C000100004002CBCBAC102201E00101011114EEEB00000000']) - -ovs-appctl time/warp 1000 AT_CHECK([ovs-appctl netdev-dummy/receive br0 \ '01005E010101000C29A027D18100000108004500001C000100004002CBCBAC102201E00101011114EEEB00000000']) @@ -589,21 +584,14 @@ OVS_WAIT_UNTIL([ovs-appctl mdb/show br2 | grep 'querier']) # del p2 on the br0, the topology will be changed AT_CHECK([ovs-vsctl del-port br0 p2]) -# give time for STP to synchronize -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 - -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 - -ovs-appctl time/warp 3000 -ovs-appctl time/warp 3000 +# We give time for STP to synchronize. The message age of +# p6 port will time out after 20s. The p5 will left blocked +# state to forwarding after 30s and then br2 bridge will +# detect the topology change, flush the fdb and sent tcn BPDU. +# br1 and br0 will also flush fdb when receiving tcn BPDU. +for i in $(seq 0 52); do + ovs-appctl time/warp 1000 +done # check fdb and mdb AT_CHECK([ovs-appctl fdb/show br0], [0], [dnl @@ -626,5 +614,67 @@ AT_CHECK([ovs-appctl mdb/show br2], [0], [dnl port VLAN GROUP Age ]) +AT_CLEANUP + +AT_SETUP([STP - check the stp ports num]) +OVS_VSWITCHD_START([]) + +AT_CHECK([ + ovs-vsctl -- \ + set port br0 other_config:stp-enable=false -- \ + set bridge br0 datapath-type=dummy stp_enable=true \ + other-config:hwaddr=aa:66:aa:66:00:00 +], [0]) + +AT_CHECK([ + ovs-vsctl add-port br0 p1 -- \ + set interface p1 type=dummy ofport_request=1 + ovs-vsctl add-port br0 p2 -- \ + set interface p2 type=dummy ofport_request=2 +], [0]) + +ovs-appctl time/stop + +# give time for STP to move initially +for i in $(seq 0 30); do + ovs-appctl time/warp 1000 +done + +AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl + p1 designated forwarding 19 128.1 +]) +AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl + p2 designated forwarding 19 128.2 +]) + +# add a stp port +AT_CHECK([ + ovs-vsctl add-port br0 p3 -- \ + set interface p3 type=dummy ofport_request=3 +], [0]) + +# The new stp port should be a listening state and other +# stp ports keep forwarding. +AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl + p1 designated forwarding 19 128.1 +]) +AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl + p2 designated forwarding 19 128.2 +]) +AT_CHECK([ovs-appctl stp/show br0 | grep p3], [0], [dnl + p3 designated listening 19 128.3 +]) + +# delete p1 stp port +AT_CHECK([ovs-vsctl del-port br0 p1]) + +# The other stp ports keep original state. +AT_CHECK([ovs-appctl stp/show br0 | grep p2], [0], [dnl + p2 designated forwarding 19 128.2 +]) +AT_CHECK([ovs-appctl stp/show br0 | grep p3], [0], [dnl + p3 designated listening 19 128.3 +]) + OVS_VSWITCHD_STOP AT_CLEANUP diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 867a26d..d683000 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -1396,13 +1396,17 @@ port_configure_stp(const struct ofproto *ofproto, struct port *port, bitmap_set1(port_num_bitmap, port_idx); port_s->port_num = port_idx; } else { - if (*port_num_counter >= STP_MAX_PORTS) { - VLOG_ERR("port %s: too many STP ports, disabling", port->name); + if (iface->ofp_port > STP_MAX_PORTS) { + VLOG_ERR("port %s: too many STP ports or OpenFlow port number " + "(%d) > STP_MAX_PORTS (%d), disabling", port->name, + iface->ofp_port, STP_MAX_PORTS); + port_s->enable = false; return; } - port_s->port_num = (*port_num_counter)++; + port_s->port_num = iface->ofp_port -1; + (*port_num_counter)++; } config_str = smap_get(&port->cfg->other_config, "stp-path-cost"); -- 1.8.3.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
