| Message ID | 20170627181310.2911-2-joe@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
On 6/27/17, 11:13 AM, "ovs-dev-bounces@openvswitch.org on behalf of Joe Stringer" <ovs-dev-bounces@openvswitch.org on behalf of joe@ovn.org> wrote:
Commit 32b77c316d9982("dpif: Save added ports in a port map.")
introduced tracking of all dpif ports by taking a reference on each
available netdev when the dpif is opened, but it failed to clear out and
release references to these netdevs when the dpif is closed.
Balance the referencing of netdevs by introducing netdev_ports_flush()
and clearing these during dpif_close().
Fixes: 32b77c316d9982("dpif: Save added ports in a port map.")
Signed-off-by: Joe Stringer <joe@ovn.org>
---
CC: Paul Blakey <paulb@mellanox.com>
CC: Darrell Ball <dlu998@gmail.com>
---
lib/dpif.c | 1 +
lib/netdev.c | 15 +++++++++++++++
lib/netdev.h | 1 +
3 files changed, 17 insertions(+)
diff --git a/lib/dpif.c b/lib/dpif.c
index 2ed0ba02f2ce..134388b996d5 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -427,6 +427,7 @@ dpif_close(struct dpif *dpif)
struct registered_dpif_class *rc;
rc = shash_find_data(&dpif_classes, dpif->dpif_class->type);
+ netdev_ports_flush(DPIF_HMAP_KEY(dpif));
I traced this call chain on datapath close:
What I noticed is that when dpif_close() gets called (triggered from close_dpif_backer())
the netdev_ports_flush() call has an empty set of ports, at this point. So netdev_port_data_destroy()
never gets called in this call chain.
I think this makes sense because the new datastructure port_to_netdev_data, on which
netdev_port_data_destroy() is intended to operate, is meant to handle
system ports and dpif_close() is designed to close a datapath “port” which is represented by
either netdev@ovs-netdev or system@ovs-system, which are tap or internal ports.
So, I am not sure why we attempt to clean port_to_netdev_data during dpif_close().
dpif_uninit(dpif, true);
dp_class_unref(rc);
}
diff --git a/lib/netdev.c b/lib/netdev.c
index eb7aef7376f1..52c132f3ad22 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2247,6 +2247,21 @@ netdev_ports_remove(odp_port_t port_no, const void *obj)
return ret;
}
+void
+netdev_ports_flush(const void *obj)
+{
+ struct port_to_netdev_data *data, *next;
+
+ ovs_mutex_lock(&netdev_hmap_mutex);
+ HMAP_FOR_EACH_SAFE(data, next, node, &port_to_netdev) {
+ if (data->obj == obj) {
+ netdev_port_data_destroy(data);
+ }
+ }
+
+ ovs_mutex_unlock(&netdev_hmap_mutex);
+}
+
odp_port_t
netdev_ifindex_to_odp_port(int ifindex)
{
diff --git a/lib/netdev.h b/lib/netdev.h
index 31846fabf9af..158c16bcb6ca 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -186,6 +186,7 @@ struct dpif_port;
int netdev_ports_insert(struct netdev *, const void *obj, struct dpif_port *);
struct netdev *netdev_ports_get(odp_port_t port, const void *obj);
int netdev_ports_remove(odp_port_t port, const void *obj);
+void netdev_ports_flush(const void *obj);
odp_port_t netdev_ifindex_to_odp_port(int ifindex);
struct netdev_flow_dump **netdev_ports_flow_dump_create(const void *obj,
int *ports);
--
2.11.1
_______________________________________________
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=kutlTwJJJEdC8nzf6dRZUP0Un8cd31791aAQP-ps2Mg&s=GMVmoQp7FFaa8ZEjGteerBURiVszoRZMb9b9B0hXgsI&e=
On Tue, Jun 27, 2017 at 11:13:10AM -0700, Joe Stringer wrote: > Commit 32b77c316d9982("dpif: Save added ports in a port map.") > introduced tracking of all dpif ports by taking a reference on each > available netdev when the dpif is opened, but it failed to clear out and > release references to these netdevs when the dpif is closed. > > Balance the referencing of netdevs by introducing netdev_ports_flush() > and clearing these during dpif_close(). > > Fixes: 32b77c316d9982("dpif: Save added ports in a port map.") > Signed-off-by: Joe Stringer <joe@ovn.org> > --- > CC: Paul Blakey <paulb@mellanox.com> > CC: Darrell Ball <dlu998@gmail.com> Hi Joe. I'd like to take a look at this series but it no longer applies because it uses a macro DPIF_HMAP_KEY that no longer exists. Would you mind rebasing and reposting? Thanks, Ben.
On 7 August 2017 at 17:00, Ben Pfaff <blp@ovn.org> wrote: > On Tue, Jun 27, 2017 at 11:13:10AM -0700, Joe Stringer wrote: >> Commit 32b77c316d9982("dpif: Save added ports in a port map.") >> introduced tracking of all dpif ports by taking a reference on each >> available netdev when the dpif is opened, but it failed to clear out and >> release references to these netdevs when the dpif is closed. >> >> Balance the referencing of netdevs by introducing netdev_ports_flush() >> and clearing these during dpif_close(). >> >> Fixes: 32b77c316d9982("dpif: Save added ports in a port map.") >> Signed-off-by: Joe Stringer <joe@ovn.org> >> --- >> CC: Paul Blakey <paulb@mellanox.com> >> CC: Darrell Ball <dlu998@gmail.com> > > Hi Joe. I'd like to take a look at this series but it no longer applies > because it uses a macro DPIF_HMAP_KEY that no longer exists. Would you > mind rebasing and reposting? Sure, I'll follow up with a rebase.
diff --git a/lib/dpif.c b/lib/dpif.c index 2ed0ba02f2ce..134388b996d5 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -427,6 +427,7 @@ dpif_close(struct dpif *dpif) struct registered_dpif_class *rc; rc = shash_find_data(&dpif_classes, dpif->dpif_class->type); + netdev_ports_flush(DPIF_HMAP_KEY(dpif)); dpif_uninit(dpif, true); dp_class_unref(rc); } diff --git a/lib/netdev.c b/lib/netdev.c index eb7aef7376f1..52c132f3ad22 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -2247,6 +2247,21 @@ netdev_ports_remove(odp_port_t port_no, const void *obj) return ret; } +void +netdev_ports_flush(const void *obj) +{ + struct port_to_netdev_data *data, *next; + + ovs_mutex_lock(&netdev_hmap_mutex); + HMAP_FOR_EACH_SAFE(data, next, node, &port_to_netdev) { + if (data->obj == obj) { + netdev_port_data_destroy(data); + } + } + + ovs_mutex_unlock(&netdev_hmap_mutex); +} + odp_port_t netdev_ifindex_to_odp_port(int ifindex) { diff --git a/lib/netdev.h b/lib/netdev.h index 31846fabf9af..158c16bcb6ca 100644 --- a/lib/netdev.h +++ b/lib/netdev.h @@ -186,6 +186,7 @@ struct dpif_port; int netdev_ports_insert(struct netdev *, const void *obj, struct dpif_port *); struct netdev *netdev_ports_get(odp_port_t port, const void *obj); int netdev_ports_remove(odp_port_t port, const void *obj); +void netdev_ports_flush(const void *obj); odp_port_t netdev_ifindex_to_odp_port(int ifindex); struct netdev_flow_dump **netdev_ports_flow_dump_create(const void *obj, int *ports);
Commit 32b77c316d9982("dpif: Save added ports in a port map.") introduced tracking of all dpif ports by taking a reference on each available netdev when the dpif is opened, but it failed to clear out and release references to these netdevs when the dpif is closed. Balance the referencing of netdevs by introducing netdev_ports_flush() and clearing these during dpif_close(). Fixes: 32b77c316d9982("dpif: Save added ports in a port map.") Signed-off-by: Joe Stringer <joe@ovn.org> --- CC: Paul Blakey <paulb@mellanox.com> CC: Darrell Ball <dlu998@gmail.com> --- lib/dpif.c | 1 + lib/netdev.c | 15 +++++++++++++++ lib/netdev.h | 1 + 3 files changed, 17 insertions(+)