diff --git a/lib/dpctl.c b/lib/dpctl.c index 4394653ab3a..79b82a1767d 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -673,7 +673,7 @@ show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p) } for (int i = 0; i < n_port_nos; i++) { - if (dpif_port_query_by_number(dpif, port_nos[i], &dpif_port)) { + if (dpif_port_query_by_number(dpif, port_nos[i], &dpif_port, true)) { continue; } diff --git a/lib/dpif.c b/lib/dpif.c index b1cbf39c48d..d07241f1e7c 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -705,13 +705,14 @@ dpif_port_set_config(struct dpif *dpif, odp_port_t port_no, * initializes '*port' appropriately; on failure, returns a positive errno * value. * - * Retuns ENODEV if the port doesn't exist. + * Retuns ENODEV if the port doesn't exist. Will not log a warning in this + * case unless 'warn_if_not_found' is true. * * The caller owns the data in 'port' and must free it with * dpif_port_destroy() when it is no longer needed. */ int dpif_port_query_by_number(const struct dpif *dpif, odp_port_t port_no, - struct dpif_port *port) + struct dpif_port *port, bool warn_if_not_found) { int error = dpif->dpif_class->port_query_by_number(dpif, port_no, port); if (!error) { @@ -719,8 +720,13 @@ dpif_port_query_by_number(const struct dpif *dpif, odp_port_t port_no, dpif_name(dpif), port_no, port->name); } else { memset(port, 0, sizeof *port); - VLOG_WARN_RL(&error_rl, "%s: failed to query port %"PRIu32": %s", - dpif_name(dpif), port_no, ovs_strerror(error)); + if (error == ENODEV && !warn_if_not_found) { + VLOG_DBG_RL(&dpmsg_rl, "%s: failed to query port %"PRIu32": %s", + dpif_name(dpif), port_no, ovs_strerror(error)); + } else { + VLOG_WARN_RL(&error_rl, "%s: failed to query port %"PRIu32": %s", + dpif_name(dpif), port_no, ovs_strerror(error)); + } } return error; } @@ -788,7 +794,7 @@ dpif_port_get_name(struct dpif *dpif, odp_port_t port_no, ovs_assert(name_size > 0); - error = dpif_port_query_by_number(dpif, port_no, &port); + error = dpif_port_query_by_number(dpif, port_no, &port, true); if (!error) { ovs_strlcpy(name, port.name, name_size); dpif_port_destroy(&port); diff --git a/lib/dpif.h b/lib/dpif.h index 9e9d0aa1b0a..0f2dc2ef3c5 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -463,7 +463,7 @@ void dpif_port_clone(struct dpif_port *, const struct dpif_port *); void dpif_port_destroy(struct dpif_port *); bool dpif_port_exists(const struct dpif *dpif, const char *devname); int dpif_port_query_by_number(const struct dpif *, odp_port_t port_no, - struct dpif_port *); + struct dpif_port *, bool warn_if_not_found); int dpif_port_query_by_name(const struct dpif *, const char *devname, struct dpif_port *); int dpif_port_get_name(struct dpif *, odp_port_t port_no, diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index fad7342b0b0..e22ca757ac3 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -2161,8 +2161,7 @@ port_destruct(struct ofport *port_, bool del) struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto); const char *devname = netdev_get_name(port->up.netdev); const char *netdev_type = netdev_get_type(port->up.netdev); - char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; - const char *dp_port_name; + struct dpif_port dpif_port; ofproto->backer->need_revalidate = REV_RECONFIGURE; xlate_txn_start(); @@ -2176,9 +2175,13 @@ port_destruct(struct ofport *port_, bool del) del = dpif_cleanup_required(ofproto->backer->dpif); } - dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf, - sizeof namebuf); - if (del && dpif_port_exists(ofproto->backer->dpif, dp_port_name)) { + /* Don't try to delete ports that are not part of the datapath. */ + if (del && port->odp_port == ODPP_NONE) { + del = false; + } + + if (del && !dpif_port_query_by_number(ofproto->backer->dpif, + port->odp_port, &dpif_port, false)) { /* The underlying device is still there, so delete it. This * happens when the ofproto is being destroyed, since the caller * assumes that removal of attached ports will happen as part of @@ -2186,6 +2189,7 @@ port_destruct(struct ofport *port_, bool del) if (!port->is_tunnel) { dpif_port_del(ofproto->backer->dpif, port->odp_port, false); } + dpif_port_destroy(&dpif_port); } else if (del) { /* The underlying device is already deleted (e.g. tunctl -d). * Calling dpif_port_remove to do local cleanup for the netdev */ diff --git a/tests/system-interface.at b/tests/system-interface.at index 148f011c7ee..d4ee5c46bad 100644 --- a/tests/system-interface.at +++ b/tests/system-interface.at @@ -123,6 +123,63 @@ AT_CHECK([ip link show | grep " genev_sys_[[0-9]]*: .* ovs-system " | diff -u - OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([interface - datapath port rename]) +OVS_TRAFFIC_VSWITCHD_START() + +dnl Not relevant for userspace datapath. +AT_SKIP_IF([! ovs-appctl dpctl/show | grep -q ovs-system]) + +AT_CHECK([ip link add ovs-veth0 type veth peer name ovs-veth1]) +dnl We will rename ovs-veth0, so removing the peer on exit. +on_exit 'ip link del ovs-veth1' + +AT_CHECK([ovs-vsctl add-port br0 ovs-veth0]) + +OVS_WAIT_UNTIL([ip link show | grep -q "ovs-veth0.* ovs-system "]) + +AT_CHECK([ovs-appctl dpctl/show | grep port], [0], [dnl + port 0: ovs-system (internal) + port 1: br0 (internal) + port 2: ovs-veth0 +]) + +dnl Rename the interface while attached to OVS. +AT_CHECK([ip l set ovs-veth0 name ovs-new-port]) + +dnl Wait for the port to be detached from the OVS datapath. +OVS_WAIT_UNTIL([ip link show | grep "ovs-new-port" | grep -v "ovs-system"]) + +dnl Check that database indicates the error. +AT_CHECK([ovs-vsctl get interface ovs-veth0 error], [0], [dnl +"could not open network device ovs-veth0 (No such device)" +]) + +dnl Check that the port is no longer in the datapath. +AT_CHECK([ovs-appctl dpctl/show | grep port], [0], [dnl + port 0: ovs-system (internal) + port 1: br0 (internal) +]) + +dnl Rename the interface back and check that it is in use again. +AT_CHECK([ip l set ovs-new-port name ovs-veth0]) + +OVS_WAIT_UNTIL([ip link show | grep -q "ovs-veth0.* ovs-system "]) + +AT_CHECK([ovs-vsctl get interface ovs-veth0 error], [0], [dnl +[[]] +]) + +AT_CHECK([ovs-appctl dpctl/show | grep port], [0], [dnl + port 0: ovs-system (internal) + port 1: br0 (internal) + port 2: ovs-veth0 +]) + +OVS_TRAFFIC_VSWITCHD_STOP([" + /could not open network device ovs-veth0 (No such device)/d +"]) +AT_CLEANUP + AT_SETUP([interface - current speed]) AT_SKIP_IF([test $HAVE_ETHTOOL = "no"]) OVS_TRAFFIC_VSWITCHD_START()