The test added in this commit would have caught the bug fixed by commit 96be8de595150 (bridge: When ports disappear from a datapath, add them back.). With that commit reverted, the new test fails.
Signed-off-by: Ben Pfaff <b...@nicira.com> Acked-by: Gurucharan Shetty <gshe...@nicira.com> --- v1->v2: Rebase, fixing a lot of conflicts. lib/dpif-netdev.c | 63 +++++++++++++++++++++++++++++++++++++++------------- tests/automake.mk | 1 + tests/bridge.at | 38 +++++++++++++++++++++++++++++++ tests/testsuite.at | 3 ++- 4 files changed, 89 insertions(+), 16 deletions(-) create mode 100644 tests/bridge.at diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index df62912..91c83d6 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -325,7 +325,7 @@ static void dp_netdev_flow_flush(struct dp_netdev *); static int do_add_port(struct dp_netdev *dp, const char *devname, const char *type, odp_port_t port_no) OVS_REQUIRES(dp->port_mutex); -static int do_del_port(struct dp_netdev *dp, odp_port_t port_no) +static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *) OVS_REQUIRES(dp->port_mutex); static void dp_netdev_destroy_all_queues(struct dp_netdev *dp) OVS_REQ_WRLOCK(dp->queue_rwlock); @@ -548,7 +548,7 @@ dp_netdev_free(struct dp_netdev *dp) dp_netdev_flow_flush(dp); ovs_mutex_lock(&dp->port_mutex); CMAP_FOR_EACH (port, node, &cursor, &dp->ports) { - do_del_port(dp, port->port_no); + do_del_port(dp, port); } ovs_mutex_unlock(&dp->port_mutex); @@ -762,7 +762,16 @@ dpif_netdev_port_del(struct dpif *dpif, odp_port_t port_no) int error; ovs_mutex_lock(&dp->port_mutex); - error = port_no == ODPP_LOCAL ? EINVAL : do_del_port(dp, port_no); + if (port_no == ODPP_LOCAL) { + error = EINVAL; + } else { + struct dp_netdev_port *port; + + error = get_port_by_number(dp, port_no, &port); + if (!error) { + do_del_port(dp, port); + } + } ovs_mutex_unlock(&dp->port_mutex); return error; @@ -851,26 +860,17 @@ get_port_by_name(struct dp_netdev *dp, return ENOENT; } -static int -do_del_port(struct dp_netdev *dp, odp_port_t port_no) +static void +do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port) OVS_REQUIRES(dp->port_mutex) { - struct dp_netdev_port *port; - int error; - - error = get_port_by_number(dp, port_no, &port); - if (error) { - return error; - } - - cmap_remove(&dp->ports, &port->node, hash_odp_port(port_no)); + cmap_remove(&dp->ports, &port->node, hash_odp_port(port->port_no)); seq_change(dp->port_seq); if (netdev_is_pmd(port->netdev)) { dp_netdev_reload_pmd_threads(dp); } port_unref(port); - return 0; } static void @@ -2292,6 +2292,37 @@ exit: } static void +dpif_dummy_delete_port(struct unixctl_conn *conn, int argc OVS_UNUSED, + const char *argv[], void *aux OVS_UNUSED) +{ + struct dp_netdev_port *port; + struct dp_netdev *dp; + + ovs_mutex_lock(&dp_netdev_mutex); + dp = shash_find_data(&dp_netdevs, argv[1]); + if (!dp || !dpif_netdev_class_is_dummy(dp->class)) { + ovs_mutex_unlock(&dp_netdev_mutex); + unixctl_command_reply_error(conn, "unknown datapath or not a dummy"); + return; + } + ovs_refcount_ref(&dp->ref_cnt); + ovs_mutex_unlock(&dp_netdev_mutex); + + ovs_mutex_lock(&dp->port_mutex); + if (get_port_by_name(dp, argv[2], &port)) { + unixctl_command_reply_error(conn, "unknown port"); + } else if (port->port_no == ODPP_LOCAL) { + unixctl_command_reply_error(conn, "can't delete local port"); + } else { + do_del_port(dp, port); + unixctl_command_reply(conn, NULL); + } + ovs_mutex_unlock(&dp->port_mutex); + + dp_netdev_unref(dp); +} + +static void dpif_dummy_register__(const char *type) { struct dpif_class *class; @@ -2324,4 +2355,6 @@ dpif_dummy_register(bool override) unixctl_command_register("dpif-dummy/change-port-number", "DP PORT NEW-NUMBER", 3, 3, dpif_dummy_change_port_number, NULL); + unixctl_command_register("dpif-dummy/delete-port", "DP PORT", + 2, 2, dpif_dummy_delete_port, NULL); } diff --git a/tests/automake.mk b/tests/automake.mk index a858148..9354fad 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -39,6 +39,7 @@ TESTSUITE_AT = \ tests/ovs-vswitchd.at \ tests/dpif-netdev.at \ tests/ofproto-dpif.at \ + tests/bridge.at \ tests/vlan-splinters.at \ tests/ofproto-macros.at \ tests/ofproto.at \ diff --git a/tests/bridge.at b/tests/bridge.at new file mode 100644 index 0000000..817931f --- /dev/null +++ b/tests/bridge.at @@ -0,0 +1,38 @@ +AT_BANNER([bridge]) + +dnl When a port disappears from a datapath, e.g. because an admin used +dnl "ovs-dpctl del-port", the bridge code should be resilient enough to +dnl notice and add it back the next time we reconfigure. A prior version +dnl of the code failed to do this, so this test guards against regression. +AT_SETUP([bridge - ports that disappear get added back]) +OVS_VSWITCHD_START + +# Add some ports and make sure that they show up in the datapath. +ADD_OF_PORTS([br0], 1, 2) +AT_CHECK([ovs-appctl dpif/show], [0], [dnl +dummy@ovs-dummy: hit:0 missed:0 + br0: + br0 65534/100: (dummy) + p1 1/1: (dummy) + p2 2/2: (dummy) +]) + +# Delete p1 from the datapath as if by "ovs-dpctl del-port" +# and check that it disappeared. +AT_CHECK([ovs-appctl dpif-dummy/delete-port ovs-dummy p1]) +AT_CHECK([ovs-appctl dpif/show], [0], [dnl +dummy@ovs-dummy: hit:0 missed:0 + br0: + br0 65534/100: (dummy) + p2 2/2: (dummy) +]) + +# Force reconfiguration and make sure that p1 got added back. +AT_CHECK([ovs-vsctl del-port p2]) +AT_CHECK([ovs-appctl dpif/show], [0], [dnl +dummy@ovs-dummy: hit:0 missed:0 + br0: + br0 65534/100: (dummy) + p1 1/1: (dummy) +]) +AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index 1911ac6..d7e5a7d 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -1,6 +1,6 @@ AT_INIT -AT_COPYRIGHT([Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc. +AT_COPYRIGHT([Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -147,6 +147,7 @@ m4_include([tests/ovs-vswitchd.at]) m4_include([tests/ofproto.at]) m4_include([tests/dpif-netdev.at]) m4_include([tests/ofproto-dpif.at]) +m4_include([tests/bridge.at]) m4_include([tests/vlan-splinters.at]) m4_include([tests/ovsdb.at]) m4_include([tests/ovs-vsctl.at]) -- 1.7.10.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev