Hi Jarno, I’ve seen your patch and although your suggestions look reasonable, I fear changing rstp_unref() like that. The reason is that I no longer have access to the IXIA validation suite and all these calls: rstp_port_set_aux(rp, NULL); rstp_port_set_state(rp, RSTP_DISABLED); rstp_port_set_mac_operational(rp, false); update_rstp_port_state(of port); might have a strong effect on the behavior of the protocol that I can’t test as I’d like to.
For this reason I, like you, would like to know if Daniel tried my patch alone, since it introduces only an initialization and a check on old_root_aux and new_root_aux. Daniele > Il giorno 30/set/2015, alle ore 22:50, Jarno Rajahalme > <jrajaha...@nicira.com> ha scritto: > > Daniele, > > Could you review my patch (in combination with yours), i.e., reply with an > Acked-by: if you think it is necessary, or reason why it is not needed or is > not sufficient. > > Regards, > > Jarno > >> On Sep 30, 2015, at 1:54 AM, Daniele Venturino <daniele.ventur...@m3s.it> >> wrote: >> >> I tried the commands you listed but I was not able to reproduce this in a >> simpler setup only with RSTP enabled on the bridge. >> >> I’m attaching a patch I’d like you to try. I’m not really expecting much >> from it, but can you give it a try since I can’t reproduce this? >> >> Daniele >> >> <0001-ofproto-dpif-Add-check-in-rstp_run.patch> >> >>> Il giorno 30/set/2015, alle ore 10:12, Daniel Swahn >>> <daniel.sw...@clavister.com> ha scritto: >>> >>> I tested with the patch but unfortunately the same crash occurs. >>> >>> -----Original Message----- >>> From: Jarno Rajahalme [mailto:jrajaha...@nicira.com] >>> Sent: 2015-09-29 23:10 >>> To: Ben Pfaff >>> Cc: Daniel Swahn; Daniele Venturino; b...@openvswitch.org >>> Subject: Re: [ovs-discuss] RSTP crash in Open vSwitch >>> >>> >>>> On Sep 28, 2015, at 9:13 PM, Ben Pfaff <b...@nicira.com> wrote: >>>> >>>> On Wed, Sep 23, 2015 at 11:31:31AM +0000, Daniel Swahn wrote: >>>>> (gdb) frame 0 >>>>> #0 0x00000000004291ca in rstp_run (ofproto=<optimized out>, >>>>> ofproto=<optimized out>) at ofproto/ofproto-dpif.c:2278 >>>>> 2278 bundle_move(((struct ofport_dpif >>>>> *)rstp_get_old_root_aux(ofproto->rstp))->bundle, >>>> >>>> Thanks for the report. >>>> >>>> Jarno and Daniele, the code around line 2278 in v2.4.0 is this: >>>> >>>> if (rstp_shift_root_learned_address(ofproto->rstp)) { >>>> bundle_move(((struct ofport_dpif >>>> *)rstp_get_old_root_aux(ofproto->rstp))->bundle, >>>> ((struct ofport_dpif >>>> *)rstp_get_new_root_aux(ofproto->rstp))->bundle); >>>> rstp_reset_root_changed(ofproto->rstp); >>>> } >>>> >>>> Looking at lib/rstp*.[ch], I see some code that sets 'old_root_aux' >>>> and 'new_root_aux'. I don't see any code that clears them or updates >>>> them if something happens to invalidate them (that is, if a port gets >>>> deleted or if its aux gets updated). I suspect that this may be the >>>> problem here. >>>> >>>> Jarno or Daniele, can you confirm this? >>> >>> I looked into this, and it may be that we fail to move the RSTP state >>> machine when removing a port. In rstp_port_unref(), we are setting the port >>> state to RSTP_DISABLED, but are then removing and freeing the port without >>> triggering any change in the RSTP state machine, so if this port was the >>> root, the root aux would still point to the deleted ofport. >>> >>> Maybe this would help: >>> >> >
_______________________________________________ discuss mailing list discuss@openvswitch.org http://openvswitch.org/mailman/listinfo/discuss
