On 9/16/22 06:24, Mike Pattrick wrote: > On Tue, Sep 13, 2022 at 3:10 PM Ilya Maximets <[email protected]> wrote: >> >> If the PACKET_OUT from controller ends up with sending packet to >> a bond interface, the main thread will take locks in the following >> order: >> handle_openflow >> --> take ofproto_mutex >> handle_packet_out >> packet_xlate >> output_normal >> bond_update_post_recirc_rules >> --> take rwlock in bond.c >> >> If at the same time revalidator thread is processing other packet >> with the output to the same bond: >> xlate_actions >> output_normal >> bond_update_post_recirc_rules >> --> take rwlock in bond.c >> update_recirc_rules >> ofproto_dpif_add_internal_flow >> ofproto_flow_mod >> --> take ofproto_mutex >> >> So, it is possible for these 2 threads to lock each other by >> taking one lock and waiting for another thread to release the >> second lock. >> >> It is also possible for the main thread to lock itself up by trying >> to acquire ofproto_mutex for the second time, if it will actually >> proceed with update_recirc_rules() after taking the bond rwlock. >> >> The problem appears to be that bond_update_post_recirc_rules() >> is called during the flow translation even if side effects are >> prohibited, which is the case for openflow PACKET_OUT handling. >> >> Skipping actual flow updates during the flow translation if >> side effects are disabled to avoid the deadlock. >> >> Since flows are not installed now when actions translated for >> very first packet, installing initial flows in bond_reconfigure(). >> This will cover the case of allocating a new recirc_id. >> >> Also checking if we need to update flows in bond_run() to cover >> link state changes. >> >> Regression test is added to catch the double lock case. >> >> Reported-at: https://github.com/openvswitch/ovs-issues/issues/259 >> Reported-by: Daniel Ding <[email protected]> >> Fixes: adcf00ba35a0 ("ofproto/bond: Implement bond megaflow using >> recirculation") >> Signed-off-by: Ilya Maximets <[email protected]> >> --- > > New test looks good! > > Acked-by: Mike Pattrick <[email protected]> >
Thanks! Applied and backported down to 2.13. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
