Wow, proper TDD! Is it possible to get this backported at least down to 2.10 ?
Acked-by: Miguel Angel Ajo <[email protected]> More context here: https://bugzilla.redhat.com/show_bug.cgi?id=1640045 On Wed, Oct 17, 2018 at 2:59 PM Numan Siddique <[email protected]> wrote: > > > On Wed, Oct 17, 2018 at 6:27 PM Miguel Angel Ajo Pelayo < > [email protected]> wrote: > >> Wow, that was quick Numans, >> >> Did you try the negative of the test? (removing your patch on the C side >> and trying the test >> to make sure it fails?) >> > > Yes. I actually started with the test to make sure it fails before working > on the fix :) > > Thanks > Numan > > >> >> >> >> On Wed, Oct 17, 2018 at 2:08 PM <[email protected]> wrote: >> >>> From: Numan Siddique <[email protected]> >>> >>> We see the below trace when a port is added to a bridge and the >>> configured >>> controller is down >>> >>> 0x00007fb002f8b207 in raise () from /lib64/libc.so.6 >>> 0x00007fb002f8c8f8 in abort () from /lib64/libc.so.6 >>> 0x00007fb004953026 in ofputil_protocol_to_ofp_version () from /lib64/ >>> libopenvswitch-2.10.so.0 >>> 0x00007fb00494e38e in ofputil_encode_port_status () from /lib64/ >>> libopenvswitch-2.10.so.0 >>> 0x00007fb004ef1c5b in connmgr_send_port_status () from >>> /lib64/libofproto-2.10.so.0 >>> 0x00007fb004efa9f4 in ofport_install () from /lib64/libofproto-2.10.so.0 >>> 0x00007fb004efbfb2 in update_port () from /lib64/libofproto-2.10.so.0 >>> 0x00007fb004efc7f9 in ofproto_port_add () from >>> /lib64/libofproto-2.10.so.0 >>> 0x0000556d540a3f95 in bridge_add_ports__ () >>> 0x0000556d540a5a47 in bridge_reconfigure () >>> 0x0000556d540a9199 in bridge_run () >>> 0x0000556d540a02a5 in main () >>> >>> When connmgr detects that the connection to the controller is down, it >>> resets the ofconn's protocol to 'OFPUTIL_P_NONE' and that's why we >>> see the above abort. This patch fixes the issue by also checking the >>> connection status before sending the port status in the >>> connmgr_send_port_status(). >>> >>> The issue can be reproduced by running the test added in this patch >>> without the fix. >>> >>> Signed-off-by: Numan Siddique <[email protected]> >>> --- >>> ofproto/connmgr.c | 3 ++- >>> tests/bridge.at | 21 +++++++++++++++++++++ >>> 2 files changed, 23 insertions(+), 1 deletion(-) >>> >>> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c >>> index f78b4c5ff..02ba75938 100644 >>> --- a/ofproto/connmgr.c >>> +++ b/ofproto/connmgr.c >>> @@ -1624,7 +1624,8 @@ connmgr_send_port_status(struct connmgr *mgr, >>> struct ofconn *source, >>> ps.reason = reason; >>> ps.desc = *pp; >>> LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { >>> - if (ofconn_receives_async_msg(ofconn, OAM_PORT_STATUS, reason)) >>> { >>> + if (ofconn_receives_async_msg(ofconn, OAM_PORT_STATUS, reason) >>> && >>> + rconn_is_connected(ofconn->rconn)) { >>> struct ofpbuf *msg; >>> >>> /* Before 1.5, OpenFlow specified that OFPT_PORT_MOD should >>> not >>> diff --git a/tests/bridge.at b/tests/bridge.at >>> index 1c3618563..ee398bdb1 100644 >>> --- a/tests/bridge.at >>> +++ b/tests/bridge.at >>> @@ -79,3 +79,24 @@ AT_CHECK([ovs-vsctl --columns=status list controller >>> | dnl >>> OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) >>> OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >>> AT_CLEANUP >>> + >>> +AT_SETUP([bridge - add port after stopping controller]) >>> +OVS_VSWITCHD_START >>> + >>> +dnl Start ovs-testcontroller >>> +ovs-testcontroller --detach punix:controller >>> --pidfile=ovs-testcontroller.pid >>> +OVS_WAIT_UNTIL([test -e controller]) >>> + >>> +AT_CHECK([ovs-vsctl set-controller br0 unix:controller]) >>> +AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=internal], >>> [0], [ignore]) >>> +AT_CHECK([ovs-appctl -t ovs-vswitchd version], [0], [ignore]) >>> + >>> +# Now kill the ovs-testcontroller >>> +kill `cat ovs-testcontroller.pid` >>> +OVS_WAIT_UNTIL([! test -e controller]) >>> +AT_CHECK([ovs-vsctl --no-wait add-port br0 p2 -- set Interface p2 >>> type=internal], [0], [ignore]) >>> +AT_CHECK([ovs-appctl -t ovs-vswitchd version], [0], [ignore]) >>> + >>> +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) >>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >>> +AT_CLEANUP >>> -- >>> 2.17.2 >>> >>> _______________________________________________ >>> dev mailing list >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >> >> -- >> Miguel Ángel Ajo >> OSP / Networking DFG, OVN Squad Engineering >> > -- Miguel Ángel Ajo OSP / Networking DFG, OVN Squad Engineering _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
