[ovs-discuss] RSTP crash in Open vSwitch

[Jarno Rajahalme]

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.venturino at 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.swahn at clavister.com> ha scritto:
>> 
>> I tested with the patch but unfortunately the same crash occurs.
>> 
>> -----Original Message-----
>> From: Jarno Rajahalme [mailto:jrajahalme at nicira.com] 
>> Sent: 2015-09-29 23:10
>> To: Ben Pfaff
>> Cc: Daniel Swahn; Daniele Venturino; bugs at openvswitch.org
>> Subject: Re: [ovs-discuss] RSTP crash in Open vSwitch
>> 
>> 
>>> On Sep 28, 2015, at 9:13 PM, Ben Pfaff <blp at 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:
>> 
>