> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao <[email protected]> > wrote: > > 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.
Could you elaborate the problem with this STP port renumbering? > This patch uses the > OpenFlow port numbers instead of sequence numbers to avoid > it. > Using OpenFlow port numbers (32-bit) as STP port numbers (8-bit) seems wrong to me. Besides, you can always use the other-config stp-port-num in ovsdb to specify which STP port number you want, if you do not want them to be numbered automatically. > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
