Sorry, I was speaking of changing set_rstp_port() of course. Anyway, my concerns on those additions remain.
Daniele > Il giorno 30/set/2015, alle ore 23:00, Daniele Venturino > <daniele.ventur...@m3s.it> ha scritto: > > 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 <mailto: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 >>> <mailto: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 <mailto: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 >>>> <mailto:jrajaha...@nicira.com>] >>>> Sent: 2015-09-29 23:10 >>>> To: Ben Pfaff >>>> Cc: Daniel Swahn; Daniele Venturino; b...@openvswitch.org >>>> <mailto: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 >>>>> <mailto: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
